[libvirt] [PULL 00/45] Machine queue, 2018-10-18
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100) are available in the Git repository at: git://github.com/ehabkost/qemu.git tags/machine-next-pull-request for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc: numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300) ---------------------------------------------------------------- Machine queue, 2018-10-18 * sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang) ---------------------------------------------------------------- Queue for Machine Core patches Cédric Le Goater (11): net: etraxfs_eth: convert SysBus init method to a realize method net: etraxfs_eth: add a reset method net: lan9118: convert SysBus init method to a realize method net: lance: convert SysBus init method to a realize method net: milkymist_minimac2: convert SysBus init method to a realize method net: mipsnet: convert SysBus init method to a realize method net: opencores_eth: convert SysBus init method to a realize method net: smc91c111: convert SysBus init method to a realize method net: stellaris_enet: convert SysBus init method to a realize method net: stellaris_enet: add a reset method net: xgmac: convert SysBus init method to a realize method David Hildenbrand (16): memory-device: fix alignment error message memory-device: fix error message when hinted address is too small memory-device: improve "range conflicts" error message pc-dimm: pass PCDIMMDevice to pc_dimm_.*plug memory-device: use memory device terminology in error messages memory-device: introduce separate config option memory-device: forward errors in get_region_size()/get_plugged_size() memory-device: document MemoryDeviceClass memory-device: add and use memory_device_get_region_size() memory-device: factor out get_memory_region() from pc-dimm memory-device: drop get_region_size() memory-device: add device class function set_addr() memory-device: complete factoring out pre_plug handling memory-device: complete factoring out plug handling memory-device: complete factoring out unplug handling memory-device: trace when pre_plugging/plugging/unplugging Igor Mammedov (2): vl.c deprecate incorrect CPUs topology vl:c: make sure that sockets are calculated correctly in '-smp X' case Li Qiang (1): machine: fix a typo Markus Armbruster (2): numa: Fix QMP command set-numa-node error handling numa: Clean up error reporting in parse_numa() Philippe Mathieu-Daudé (12): trace-events: Fix copy/paste typo hw/timer/sun4v-rtc: Convert from DPRINTF() macro to trace events hw/timer/sun4v-rtc: Use DeviceState::realize rather than SysBusDevice::init hw/ssi/xilinx_spi: Use DeviceState::realize rather than SysBusDevice::init hw/sh4/sh_pci: Use DeviceState::realize rather than SysBusDevice::init hw/pci-host/bonito: Use DeviceState::realize rather than SysBusDevice::init hw/mips/gt64xxx_pci: Convert gt64120_reset() function into Device reset method hw/mips/gt64xxx_pci: Mark as bridge device hw/sparc64/niagara: Model the I/O Bridge with the 'unimplemented_device' hw/alpha/typhoon: Remove unuseful code hw/hppa/dino: Remove unuseful code hw/mips/malta: Remove unuseful code Zhang Yi (1): hostmem-file: fixed the memory leak while get pmem path. default-configs/i386-softmmu.mak | 3 +- default-configs/ppc64-softmmu.mak | 3 +- default-configs/sparc64-softmmu.mak | 1 - qapi/misc.json | 2 +- include/hw/mem/memory-device.h | 74 +++++++++++++++++--- include/hw/mem/pc-dimm.h | 10 +-- include/sysemu/numa.h | 1 - backends/hostmem-file.c | 10 ++- hw/alpha/typhoon.c | 13 ---- hw/core/machine.c | 2 +- hw/hppa/dino.c | 7 -- hw/i386/pc.c | 6 +- hw/mem/memory-device.c | 103 ++++++++++++++++++++++------ hw/mem/nvdimm.c | 9 ++- hw/mem/pc-dimm.c | 84 ++++++++--------------- hw/mips/gt64xxx_pci.c | 18 ++--- hw/mips/mips_malta.c | 13 ---- hw/net/etraxfs_eth.c | 44 ++++++++---- hw/net/lan9118.c | 9 +-- hw/net/lance.c | 8 +-- hw/net/milkymist-minimac2.c | 9 +-- hw/net/mipsnet.c | 9 +-- hw/net/opencores_eth.c | 8 +-- hw/net/smc91c111.c | 8 +-- hw/net/stellaris_enet.c | 15 ++-- hw/net/xgmac.c | 9 +-- hw/pci-host/bonito.c | 9 +-- hw/ppc/spapr.c | 29 +++----- hw/sh4/sh_pci.c | 20 +++--- hw/sparc64/niagara.c | 4 +- hw/ssi/xilinx_spi.c | 9 +-- hw/timer/sun4v-rtc.c | 23 +++---- numa.c | 21 +++--- vl.c | 12 +++- hw/Makefile.objs | 2 +- hw/mem/Makefile.objs | 4 +- hw/mem/trace-events | 5 +- hw/timer/trace-events | 6 +- qemu-deprecated.texi | 12 ++++ 39 files changed, 347 insertions(+), 287 deletions(-) -- 2.18.0.rc1.1.g3f1ff2140
From: Zhang Yi <yi.z.zhang@linux.intel.com> object_get_canonical_path_component() returns a string which must be freed using g_free(). Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com> Message-Id: <7328fb16c394eaf5d65437d11c2a9343647b6d3d.1535471899.git.yi.z.zhang@linux.intel.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- backends/hostmem-file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index e64074954f..639c8d4307 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -145,20 +145,26 @@ static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp) HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o); if (host_memory_backend_mr_inited(backend)) { + char *path = object_get_canonical_path_component(o); + error_setg(errp, "cannot change property 'pmem' of %s '%s'", object_get_typename(o), - object_get_canonical_path_component(o)); + path); + g_free(path); return; } #ifndef CONFIG_LIBPMEM if (value) { Error *local_err = NULL; + char *path = object_get_canonical_path_component(o); + error_setg(&local_err, "Lack of libpmem support while setting the 'pmem=on'" " of %s '%s'. We can't ensure data persistence.", object_get_typename(o), - object_get_canonical_path_component(o)); + path); + g_free(path); error_propagate(errp, local_err); return; } -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:38PM -0300, Eduardo Habkost wrote:
From: Zhang Yi <yi.z.zhang@linux.intel.com>
object_get_canonical_path_component() returns a string which must be freed using g_free().
Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Zhang Yi <yi.z.zhang@linux.intel.com> Message-Id: <7328fb16c394eaf5d65437d11c2a9343647b6d3d.1535471899.git.yi.z.zhang@linux.intel.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- backends/hostmem-file.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index e64074954f..639c8d4307 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -145,20 +145,26 @@ static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp) HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
if (host_memory_backend_mr_inited(backend)) { + char *path = object_get_canonical_path_component(o); + error_setg(errp, "cannot change property 'pmem' of %s '%s'", object_get_typename(o), - object_get_canonical_path_component(o)); + path); + g_free(path); return; }
#ifndef CONFIG_LIBPMEM if (value) { Error *local_err = NULL; + char *path = object_get_canonical_path_component(o); + error_setg(&local_err, "Lack of libpmem support while setting the 'pmem=on'" " of %s '%s'. We can't ensure data persistence.", object_get_typename(o), - object_get_canonical_path_component(o)); + path); + g_free(path); error_propagate(errp, local_err); return; }
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Igor Mammedov <imammedo@redhat.com> -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology so that total number of logical CPUs [sockets * cores * threads] would be equal to [maxcpus], however historically we didn't have such check in QEMU and it is possible to start VM with an invalid topology. Deprecate invalid options combination so we can make sure that the topology VM started with is always correct in the future. Users with an invalid sockets/cores/threads/maxcpus values should fix their CLI to make sure that [sockets * cores * threads] == [maxcpus] Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <1536836762-273036-2-git-send-email-imammedo@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- vl.c | 7 +++++++ qemu-deprecated.texi | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/vl.c b/vl.c index 4e25c78bff..027d04c7ca 100644 --- a/vl.c +++ b/vl.c @@ -1235,6 +1235,13 @@ static void smp_parse(QemuOpts *opts) exit(1); } + if (sockets * cores * threads != max_cpus) { + warn_report("Invalid CPU topology deprecated: " + "sockets (%u) * cores (%u) * threads (%u) " + "!= maxcpus (%u)", + sockets, cores, threads, max_cpus); + } + smp_cpus = cpus; smp_cores = cores; smp_threads = threads; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 11b870c5c1..5d2d7a3588 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -86,6 +86,18 @@ for these file types is 'host_cdrom' or 'host_device' as appropriate. The @option{name} parameter of the @option{-net} option is a synonym for the @option{id} parameter, which should now be used instead. +@subsection -smp (invalid topologies) (since 3.1) + +CPU topology properties should describe whole machine topology including +possible CPUs. + +However, historically it was possible to start QEMU with an incorrect topology +where @math{@var{n} <= @var{sockets} * @var{cores} * @var{threads} < @var{maxcpus}}, +which could lead to an incorrect topology enumeration by the guest. +Support for invalid topologies will be removed, the user must ensure +topologies described with -smp include all possible cpus, i.e. + @math{@var{sockets} * @var{cores} * @var{threads} = @var{maxcpus}}. + @section QEMU Machine Protocol (QMP) commands @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0) -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:39PM -0300, Eduardo Habkost wrote:
From: Igor Mammedov <imammedo@redhat.com>
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology so that total number of logical CPUs [sockets * cores * threads] would be equal to [maxcpus], however historically we didn't have such check in QEMU and it is possible to start VM with an invalid topology. Deprecate invalid options combination so we can make sure that the topology VM started with is always correct in the future. Users with an invalid sockets/cores/threads/maxcpus values should fix their CLI to make sure that [sockets * cores * threads] == [maxcpus]
Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <1536836762-273036-2-git-send-email-imammedo@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- vl.c | 7 +++++++ qemu-deprecated.texi | 12 ++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/vl.c b/vl.c index 4e25c78bff..027d04c7ca 100644 --- a/vl.c +++ b/vl.c @@ -1235,6 +1235,13 @@ static void smp_parse(QemuOpts *opts) exit(1); }
+ if (sockets * cores * threads != max_cpus) { + warn_report("Invalid CPU topology deprecated: " + "sockets (%u) * cores (%u) * threads (%u) " + "!= maxcpus (%u)", + sockets, cores, threads, max_cpus); + } + smp_cpus = cpus; smp_cores = cores; smp_threads = threads; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 11b870c5c1..5d2d7a3588 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -86,6 +86,18 @@ for these file types is 'host_cdrom' or 'host_device' as appropriate. The @option{name} parameter of the @option{-net} option is a synonym for the @option{id} parameter, which should now be used instead.
+@subsection -smp (invalid topologies) (since 3.1) + +CPU topology properties should describe whole machine topology including +possible CPUs. + +However, historically it was possible to start QEMU with an incorrect topology +where @math{@var{n} <= @var{sockets} * @var{cores} * @var{threads} < @var{maxcpus}}, +which could lead to an incorrect topology enumeration by the guest. +Support for invalid topologies will be removed, the user must ensure +topologies described with -smp include all possible cpus, i.e. + @math{@var{sockets} * @var{cores} * @var{threads} = @var{maxcpus}}. + @section QEMU Machine Protocol (QMP) commands
@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Igor Mammedov <imammedo@redhat.com> commit (5cdc9b76e3 vl.c: Remove dead assignment) removed sockets calculation when 'sockets' weren't provided on CLI since there wasn't any users for it back then. Exiting checks are neither reachable } else if (sockets * cores * threads < cpus) { or nor triggerable if (sockets * cores * threads > max_cpus) so we weren't noticing wrong topology since then, since users recalculate sockets adhoc on their own. However with deprecation check it becomes noticable, for example -smp 2 will start printing warning: "warning: Invalid CPU topology deprecated: sockets (1) * cores (1) * threads (1) != maxcpus (2)" calculating sockets if they weren't specified. Fix it by returning back sockets calculation if it's omitted on CLI. Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-Id: <1536836762-273036-3-git-send-email-imammedo@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- vl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 027d04c7ca..2324321d91 100644 --- a/vl.c +++ b/vl.c @@ -1199,11 +1199,14 @@ static void smp_parse(QemuOpts *opts) /* compute missing values, prefer sockets over cores over threads */ if (cpus == 0 || sockets == 0) { - sockets = sockets > 0 ? sockets : 1; cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; if (cpus == 0) { + sockets = sockets > 0 ? sockets : 1; cpus = cores * threads * sockets; + } else { + max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus); + sockets = max_cpus / (cores * threads); } } else if (cores == 0) { threads = threads > 0 ? threads : 1; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:40PM -0300, Eduardo Habkost wrote:
From: Igor Mammedov <imammedo@redhat.com>
commit (5cdc9b76e3 vl.c: Remove dead assignment) removed sockets calculation when 'sockets' weren't provided on CLI since there wasn't any users for it back then. Exiting checks are neither reachable } else if (sockets * cores * threads < cpus) { or nor triggerable if (sockets * cores * threads > max_cpus) so we weren't noticing wrong topology since then, since users recalculate sockets adhoc on their own.
However with deprecation check it becomes noticable, for example -smp 2 will start printing warning: "warning: Invalid CPU topology deprecated: sockets (1) * cores (1) * threads (1) != maxcpus (2)" calculating sockets if they weren't specified.
Fix it by returning back sockets calculation if it's omitted on CLI.
Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Message-Id: <1536836762-273036-3-git-send-email-imammedo@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- vl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c index 027d04c7ca..2324321d91 100644 --- a/vl.c +++ b/vl.c @@ -1199,11 +1199,14 @@ static void smp_parse(QemuOpts *opts)
/* compute missing values, prefer sockets over cores over threads */ if (cpus == 0 || sockets == 0) { - sockets = sockets > 0 ? sockets : 1; cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; if (cpus == 0) { + sockets = sockets > 0 ? sockets : 1; cpus = cores * threads * sockets; + } else { + max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus); + sockets = max_cpus / (cores * threads); } } else if (cores == 0) { threads = threads > 0 ? threads : 1;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Markus Armbruster <armbru@redhat.com> Calling error_report() in a function that takes an Error ** argument is suspicious. parse_numa_node() does that, and then exit()s. It also passes &error_fatal to machine_set_cpu_numa_node(). Both wrong. Attempting to configure numa when the machine doesn't support it kills the VM: $ qemu-system-x86_64 -nodefaults -S -display none -M none -preconfig -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3}, "package": "v3.0.0-837-gc5e4e49258"}, "capabilities": []}} {"execute": "qmp_capabilities"} {"return": {}} {"execute": "set-numa-node", "arguments": {"type": "node"}} NUMA is not supported by this machine-type $ echo $? 1 Messed up when commit 64c2a8f6d3f and 7c88e65d9e9 (v2.10.0) added incorrect error handling right next to correct examples. Latent bug until commit f3be67812c2 (v3.0.0) made it accessible via QMP. Fairly harmless in practice, because it's limited to RUN_STATE_PRECONFIG. The fix is obvious: replace error_report(); exit() by error_setg(); return. This affects parse_numa_node()'s other caller numa_complete_configuration(): since it ignores errors, the "NUMA is not supported by this machine-type" is now ignored, too. But that error is as unexpected there as any other. Change it to abort on error instead. Fixes: f3be67812c226162f86ce92634bd913714445420 Cc: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20181008173125.19678-15-armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- numa.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/numa.c b/numa.c index 81542d4ebb..1d7c49ad43 100644 --- a/numa.c +++ b/numa.c @@ -60,6 +60,7 @@ NodeInfo numa_info[MAX_NODES]; static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, Error **errp) { + Error *err = NULL; uint16_t nodenr; uint16List *cpus = NULL; MachineClass *mc = MACHINE_GET_CLASS(ms); @@ -82,8 +83,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, } if (!mc->cpu_index_to_instance_props || !mc->get_default_cpu_node_id) { - error_report("NUMA is not supported by this machine-type"); - exit(1); + error_setg(errp, "NUMA is not supported by this machine-type"); + return; } for (cpus = node->cpus; cpus; cpus = cpus->next) { CpuInstanceProperties props; @@ -97,7 +98,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, props = mc->cpu_index_to_instance_props(ms, cpus->value); props.node_id = nodenr; props.has_node_id = true; - machine_set_cpu_numa_node(ms, &props, &error_fatal); + machine_set_cpu_numa_node(ms, &props, &err); + if (err) { + error_propagate(errp, err); + return; + } } if (node->has_mem && node->has_memdev) { @@ -367,7 +372,7 @@ void numa_complete_configuration(MachineState *ms) if (ms->ram_slots > 0 && nb_numa_nodes == 0 && mc->auto_enable_numa_with_memhp) { NumaNodeOptions node = { }; - parse_numa_node(ms, &node, NULL); + parse_numa_node(ms, &node, &error_abort); } assert(max_numa_nodeid <= MAX_NODES); -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:41PM -0300, Eduardo Habkost wrote:
From: Markus Armbruster <armbru@redhat.com>
Calling error_report() in a function that takes an Error ** argument is suspicious. parse_numa_node() does that, and then exit()s. It also passes &error_fatal to machine_set_cpu_numa_node(). Both wrong. Attempting to configure numa when the machine doesn't support it kills the VM:
$ qemu-system-x86_64 -nodefaults -S -display none -M none -preconfig -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3}, "package": "v3.0.0-837-gc5e4e49258"}, "capabilities": []}} {"execute": "qmp_capabilities"} {"return": {}} {"execute": "set-numa-node", "arguments": {"type": "node"}} NUMA is not supported by this machine-type $ echo $? 1
Messed up when commit 64c2a8f6d3f and 7c88e65d9e9 (v2.10.0) added incorrect error handling right next to correct examples. Latent bug until commit f3be67812c2 (v3.0.0) made it accessible via QMP. Fairly harmless in practice, because it's limited to RUN_STATE_PRECONFIG. The fix is obvious: replace error_report(); exit() by error_setg(); return.
This affects parse_numa_node()'s other caller numa_complete_configuration(): since it ignores errors, the "NUMA is not supported by this machine-type" is now ignored, too. But that error is as unexpected there as any other. Change it to abort on error instead.
Fixes: f3be67812c226162f86ce92634bd913714445420 Cc: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20181008173125.19678-15-armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- numa.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/numa.c b/numa.c index 81542d4ebb..1d7c49ad43 100644 --- a/numa.c +++ b/numa.c @@ -60,6 +60,7 @@ NodeInfo numa_info[MAX_NODES]; static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, Error **errp) { + Error *err = NULL; uint16_t nodenr; uint16List *cpus = NULL; MachineClass *mc = MACHINE_GET_CLASS(ms); @@ -82,8 +83,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, }
if (!mc->cpu_index_to_instance_props || !mc->get_default_cpu_node_id) { - error_report("NUMA is not supported by this machine-type"); - exit(1); + error_setg(errp, "NUMA is not supported by this machine-type"); + return; } for (cpus = node->cpus; cpus; cpus = cpus->next) { CpuInstanceProperties props; @@ -97,7 +98,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, props = mc->cpu_index_to_instance_props(ms, cpus->value); props.node_id = nodenr; props.has_node_id = true; - machine_set_cpu_numa_node(ms, &props, &error_fatal); + machine_set_cpu_numa_node(ms, &props, &err); + if (err) { + error_propagate(errp, err); + return; + } }
if (node->has_mem && node->has_memdev) { @@ -367,7 +372,7 @@ void numa_complete_configuration(MachineState *ms) if (ms->ram_slots > 0 && nb_numa_nodes == 0 && mc->auto_enable_numa_with_memhp) { NumaNodeOptions node = { }; - parse_numa_node(ms, &node, NULL); + parse_numa_node(ms, &node, &error_abort); }
assert(max_numa_nodeid <= MAX_NODES);
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Missed while reviewing 5dd85b4b486. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-2-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/timer/trace-events | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/timer/trace-events b/hw/timer/trace-events index fa4213df5b..ca9ad6321a 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -56,7 +56,7 @@ systick_timer_tick(void) "systick reload" systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u" systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u" -# hw/char/cmsdk_apb_timer.c +# hw/timer/cmsdk_apb_timer.c cmsdk_apb_timer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" cmsdk_apb_timer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" cmsdk_apb_timer_reset(void) "CMSDK APB timer: reset" -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:42PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Missed while reviewing 5dd85b4b486.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-2-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/timer/trace-events | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/timer/trace-events b/hw/timer/trace-events index fa4213df5b..ca9ad6321a 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -56,7 +56,7 @@ systick_timer_tick(void) "systick reload" systick_read(uint64_t addr, uint32_t value, unsigned size) "systick read addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u" systick_write(uint64_t addr, uint32_t value, unsigned size) "systick write addr 0x%" PRIx64 " data 0x%" PRIx32 " size %u"
-# hw/char/cmsdk_apb_timer.c +# hw/timer/cmsdk_apb_timer.c cmsdk_apb_timer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" cmsdk_apb_timer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" cmsdk_apb_timer_reset(void) "CMSDK APB timer: reset"
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-3-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/timer/sun4v-rtc.c | 13 +++---------- hw/timer/trace-events | 4 ++++ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c index 310523225f..13be94f8da 100644 --- a/hw/timer/sun4v-rtc.c +++ b/hw/timer/sun4v-rtc.c @@ -14,15 +14,8 @@ #include "hw/sysbus.h" #include "qemu/timer.h" #include "hw/timer/sun4v-rtc.h" +#include "trace.h" -//#define DEBUG_SUN4V_RTC - -#ifdef DEBUG_SUN4V_RTC -#define DPRINTF(fmt, ...) \ - do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) do {} while (0) -#endif #define TYPE_SUN4V_RTC "sun4v_rtc" #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC) @@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr addr, /* accessing the high 32 bits */ val >>= 32; } - DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val); + trace_sun4v_rtc_read(addr, val); return val; } static void sun4v_rtc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { - DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr); + trace_sun4v_rtc_read(addr, val); } static const MemoryRegionOps sun4v_rtc_ops = { diff --git a/hw/timer/trace-events b/hw/timer/trace-events index ca9ad6321a..75bd3b1042 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -66,5 +66,9 @@ cmsdk_apb_dualtimer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK A cmsdk_apb_dualtimer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB dualtimer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset" +# hw/timer/sun4v-rtc.c +sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64 " value 0x%" PRIx64 +sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%" PRIx64 " value 0x%" PRIx64 + # hw/timer/xlnx-zynqmp-rtc.c xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d" -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:43PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-3-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/timer/sun4v-rtc.c | 13 +++---------- hw/timer/trace-events | 4 ++++ 2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c index 310523225f..13be94f8da 100644 --- a/hw/timer/sun4v-rtc.c +++ b/hw/timer/sun4v-rtc.c @@ -14,15 +14,8 @@ #include "hw/sysbus.h" #include "qemu/timer.h" #include "hw/timer/sun4v-rtc.h" +#include "trace.h"
-//#define DEBUG_SUN4V_RTC - -#ifdef DEBUG_SUN4V_RTC -#define DPRINTF(fmt, ...) \ - do { printf("sun4v_rtc: " fmt , ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) do {} while (0) -#endif
#define TYPE_SUN4V_RTC "sun4v_rtc" #define SUN4V_RTC(obj) OBJECT_CHECK(Sun4vRtc, (obj), TYPE_SUN4V_RTC) @@ -41,14 +34,14 @@ static uint64_t sun4v_rtc_read(void *opaque, hwaddr addr, /* accessing the high 32 bits */ val >>= 32; } - DPRINTF("read from " TARGET_FMT_plx " val %lx\n", addr, val); + trace_sun4v_rtc_read(addr, val); return val; }
static void sun4v_rtc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { - DPRINTF("write 0x%x to " TARGET_FMT_plx "\n", (unsigned)val, addr); + trace_sun4v_rtc_read(addr, val);
This should be trace_sun4v_rtc_write(), no?
}
static const MemoryRegionOps sun4v_rtc_ops = { diff --git a/hw/timer/trace-events b/hw/timer/trace-events index ca9ad6321a..75bd3b1042 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -66,5 +66,9 @@ cmsdk_apb_dualtimer_read(uint64_t offset, uint64_t data, unsigned size) "CMSDK A cmsdk_apb_dualtimer_write(uint64_t offset, uint64_t data, unsigned size) "CMSDK APB dualtimer write: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" cmsdk_apb_dualtimer_reset(void) "CMSDK APB dualtimer: reset"
+# hw/timer/sun4v-rtc.c +sun4v_rtc_read(uint64_t addr, uint64_t value) "read: addr 0x%" PRIx64 " value 0x%" PRIx64 +sun4v_rtc_write(uint64_t addr, uint64_t value) "write: addr 0x%" PRIx64 " value 0x%" PRIx64 + # hw/timer/xlnx-zynqmp-rtc.c xlnx_zynqmp_rtc_gettime(int year, int month, int day, int hour, int min, int sec) "Get time from host: %d-%d-%d %2d:%02d:%02d"
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Move from the legacy SysBusDevice::init method to using DeviceState::realize. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-4-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/timer/sun4v-rtc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c index 13be94f8da..4e7f6a1eff 100644 --- a/hw/timer/sun4v-rtc.c +++ b/hw/timer/sun4v-rtc.c @@ -63,21 +63,21 @@ void sun4v_rtc_init(hwaddr addr) sysbus_mmio_map(s, 0, addr); } -static int sun4v_rtc_init1(SysBusDevice *dev) +static void sun4v_rtc_realize(DeviceState *dev, Error **errp) { + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); Sun4vRtc *s = SUN4V_RTC(dev); memory_region_init_io(&s->iomem, OBJECT(s), &sun4v_rtc_ops, s, "sun4v-rtc", 0x08ULL); - sysbus_init_mmio(dev, &s->iomem); - return 0; + sysbus_init_mmio(sbd, &s->iomem); } static void sun4v_rtc_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass); - k->init = sun4v_rtc_init1; + dc->realize = sun4v_rtc_realize; } static const TypeInfo sun4v_rtc_info = { -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:44PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Move from the legacy SysBusDevice::init method to using DeviceState::realize.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-4-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/timer/sun4v-rtc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/timer/sun4v-rtc.c b/hw/timer/sun4v-rtc.c index 13be94f8da..4e7f6a1eff 100644 --- a/hw/timer/sun4v-rtc.c +++ b/hw/timer/sun4v-rtc.c @@ -63,21 +63,21 @@ void sun4v_rtc_init(hwaddr addr) sysbus_mmio_map(s, 0, addr); }
-static int sun4v_rtc_init1(SysBusDevice *dev) +static void sun4v_rtc_realize(DeviceState *dev, Error **errp) { + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); Sun4vRtc *s = SUN4V_RTC(dev);
memory_region_init_io(&s->iomem, OBJECT(s), &sun4v_rtc_ops, s, "sun4v-rtc", 0x08ULL); - sysbus_init_mmio(dev, &s->iomem); - return 0; + sysbus_init_mmio(sbd, &s->iomem); }
static void sun4v_rtc_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass);
- k->init = sun4v_rtc_init1; + dc->realize = sun4v_rtc_realize; }
static const TypeInfo sun4v_rtc_info = {
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Move from the legacy SysBusDevice::init method to using DeviceState::realize. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-5-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/ssi/xilinx_spi.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c index 83585bc8b2..3dae303d5b 100644 --- a/hw/ssi/xilinx_spi.c +++ b/hw/ssi/xilinx_spi.c @@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = { } }; -static int xilinx_spi_init(SysBusDevice *sbd) +static void xilinx_spi_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); XilinxSPI *s = XILINX_SPI(dev); int i; @@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd) fifo8_create(&s->tx_fifo, FIFO_CAPACITY); fifo8_create(&s->rx_fifo, FIFO_CAPACITY); - - return 0; } static const VMStateDescription vmstate_xilinx_spi = { @@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = { static void xilinx_spi_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = xilinx_spi_init; + dc->realize = xilinx_spi_realize; dc->reset = xlx_spi_reset; dc->props = xilinx_spi_properties; dc->vmsd = &vmstate_xilinx_spi; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:45PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Move from the legacy SysBusDevice::init method to using DeviceState::realize.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-5-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/ssi/xilinx_spi.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c index 83585bc8b2..3dae303d5b 100644 --- a/hw/ssi/xilinx_spi.c +++ b/hw/ssi/xilinx_spi.c @@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = { } };
-static int xilinx_spi_init(SysBusDevice *sbd) +static void xilinx_spi_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); XilinxSPI *s = XILINX_SPI(dev); int i;
@@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd)
fifo8_create(&s->tx_fifo, FIFO_CAPACITY); fifo8_create(&s->rx_fifo, FIFO_CAPACITY); - - return 0; }
static const VMStateDescription vmstate_xilinx_spi = { @@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = { static void xilinx_spi_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = xilinx_spi_init; + dc->realize = xilinx_spi_realize; dc->reset = xlx_spi_reset; dc->props = xilinx_spi_properties; dc->vmsd = &vmstate_xilinx_spi;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Move from the legacy SysBusDevice::init method to using DeviceState::realize. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20181002212522.23303-6-f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/sh4/sh_pci.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c index 4ec2e35500..379d0685ed 100644 --- a/hw/sh4/sh_pci.c +++ b/hw/sh4/sh_pci.c @@ -120,16 +120,15 @@ static void sh_pci_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pic[irq_num], level); } -static int sh_pci_device_init(SysBusDevice *dev) +static void sh_pci_device_realize(DeviceState *dev, Error **errp) { - PCIHostState *phb; - SHPCIState *s; + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + SHPCIState *s = SH_PCI_HOST_BRIDGE(dev); + PCIHostState *phb = PCI_HOST_BRIDGE(s); int i; - s = SH_PCI_HOST_BRIDGE(dev); - phb = PCI_HOST_BRIDGE(s); for (i = 0; i < 4; i++) { - sysbus_init_irq(dev, &s->irq[i]); + sysbus_init_irq(sbd, &s->irq[i]); } phb->bus = pci_register_root_bus(DEVICE(dev), "pci", sh_pci_set_irq, sh_pci_map_irq, @@ -143,13 +142,12 @@ static int sh_pci_device_init(SysBusDevice *dev) &s->memconfig_p4, 0, 0x224); memory_region_init_alias(&s->isa, OBJECT(s), "sh_pci.isa", get_system_io(), 0, 0x40000); - sysbus_init_mmio(dev, &s->memconfig_p4); - sysbus_init_mmio(dev, &s->memconfig_a7); + sysbus_init_mmio(sbd, &s->memconfig_p4); + sysbus_init_mmio(sbd, &s->memconfig_a7); s->iobr = 0xfe240000; memory_region_add_subregion(get_system_memory(), s->iobr, &s->isa); s->dev = pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "sh_pci_host"); - return 0; } static void sh_pci_host_realize(PCIDevice *d, Error **errp) @@ -187,9 +185,9 @@ static const TypeInfo sh_pci_host_info = { static void sh_pci_device_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass); - sdc->init = sh_pci_device_init; + dc->realize = sh_pci_device_realize; } static const TypeInfo sh_pci_device_info = { -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:46PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Move from the legacy SysBusDevice::init method to using DeviceState::realize.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20181002212522.23303-6-f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/sh4/sh_pci.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c index 4ec2e35500..379d0685ed 100644 --- a/hw/sh4/sh_pci.c +++ b/hw/sh4/sh_pci.c @@ -120,16 +120,15 @@ static void sh_pci_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pic[irq_num], level); }
-static int sh_pci_device_init(SysBusDevice *dev) +static void sh_pci_device_realize(DeviceState *dev, Error **errp) { - PCIHostState *phb; - SHPCIState *s; + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + SHPCIState *s = SH_PCI_HOST_BRIDGE(dev); + PCIHostState *phb = PCI_HOST_BRIDGE(s); int i;
- s = SH_PCI_HOST_BRIDGE(dev); - phb = PCI_HOST_BRIDGE(s); for (i = 0; i < 4; i++) { - sysbus_init_irq(dev, &s->irq[i]); + sysbus_init_irq(sbd, &s->irq[i]); } phb->bus = pci_register_root_bus(DEVICE(dev), "pci", sh_pci_set_irq, sh_pci_map_irq, @@ -143,13 +142,12 @@ static int sh_pci_device_init(SysBusDevice *dev) &s->memconfig_p4, 0, 0x224); memory_region_init_alias(&s->isa, OBJECT(s), "sh_pci.isa", get_system_io(), 0, 0x40000); - sysbus_init_mmio(dev, &s->memconfig_p4); - sysbus_init_mmio(dev, &s->memconfig_a7); + sysbus_init_mmio(sbd, &s->memconfig_p4); + sysbus_init_mmio(sbd, &s->memconfig_a7); s->iobr = 0xfe240000; memory_region_add_subregion(get_system_memory(), s->iobr, &s->isa);
s->dev = pci_create_simple(phb->bus, PCI_DEVFN(0, 0), "sh_pci_host"); - return 0; }
static void sh_pci_host_realize(PCIDevice *d, Error **errp) @@ -187,9 +185,9 @@ static const TypeInfo sh_pci_host_info = {
static void sh_pci_device_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass);
- sdc->init = sh_pci_device_init; + dc->realize = sh_pci_device_realize; }
static const TypeInfo sh_pci_device_info = {
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Move from the legacy SysBusDevice::init method to using DeviceState::realize. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20181002212522.23303-7-f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/pci-host/bonito.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index 9868e2eccc..9f33582706 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -595,7 +595,7 @@ static const VMStateDescription vmstate_bonito = { } }; -static int bonito_pcihost_initfn(SysBusDevice *dev) +static void bonito_pcihost_realize(DeviceState *dev, Error **errp) { PCIHostState *phb = PCI_HOST_BRIDGE(dev); @@ -603,8 +603,6 @@ static int bonito_pcihost_initfn(SysBusDevice *dev) pci_bonito_set_irq, pci_bonito_map_irq, dev, get_system_memory(), get_system_io(), 0x28, 32, TYPE_PCI_BUS); - - return 0; } static void bonito_realize(PCIDevice *dev, Error **errp) @@ -684,7 +682,6 @@ PCIBus *bonito_init(qemu_irq *pic) pcihost->pic = pic; qdev_init_nofail(dev); - /* set the pcihost pointer before bonito_initfn is called */ d = pci_create(phb->bus, PCI_DEVFN(0, 0), TYPE_PCI_BONITO); s = PCI_BONITO(d); s->pcihost = pcihost; @@ -726,9 +723,9 @@ static const TypeInfo bonito_info = { static void bonito_pcihost_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass); - k->init = bonito_pcihost_initfn; + dc->realize = bonito_pcihost_realize; } static const TypeInfo bonito_pcihost_info = { -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:47PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Move from the legacy SysBusDevice::init method to using DeviceState::realize.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20181002212522.23303-7-f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/pci-host/bonito.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index 9868e2eccc..9f33582706 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -595,7 +595,7 @@ static const VMStateDescription vmstate_bonito = { } };
-static int bonito_pcihost_initfn(SysBusDevice *dev) +static void bonito_pcihost_realize(DeviceState *dev, Error **errp) { PCIHostState *phb = PCI_HOST_BRIDGE(dev);
@@ -603,8 +603,6 @@ static int bonito_pcihost_initfn(SysBusDevice *dev) pci_bonito_set_irq, pci_bonito_map_irq, dev, get_system_memory(), get_system_io(), 0x28, 32, TYPE_PCI_BUS); - - return 0; }
static void bonito_realize(PCIDevice *dev, Error **errp) @@ -684,7 +682,6 @@ PCIBus *bonito_init(qemu_irq *pic) pcihost->pic = pic; qdev_init_nofail(dev);
- /* set the pcihost pointer before bonito_initfn is called */ d = pci_create(phb->bus, PCI_DEVFN(0, 0), TYPE_PCI_BONITO); s = PCI_BONITO(d); s->pcihost = pcihost; @@ -726,9 +723,9 @@ static const TypeInfo bonito_info = {
static void bonito_pcihost_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + DeviceClass *dc = DEVICE_CLASS(klass);
- k->init = bonito_pcihost_initfn; + dc->realize = bonito_pcihost_realize; }
static const TypeInfo bonito_pcihost_info = {
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Convert the gt64120_reset() function into a proper Device reset method. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-8-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mips/gt64xxx_pci.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index 24ad0ad024..dcd1a66329 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -992,9 +992,9 @@ static void gt64120_pci_set_irq(void *opaque, int irq_num, int level) } -static void gt64120_reset(void *opaque) +static void gt64120_reset(DeviceState *dev) { - GT64120State *s = opaque; + GT64120State *s = GT64120_PCI_HOST_BRIDGE(dev); /* FIXME: Malta specific hw assumptions ahead */ @@ -1184,16 +1184,6 @@ PCIBus *gt64120_register(qemu_irq *pic) return phb->bus; } -static int gt64120_init(SysBusDevice *dev) -{ - GT64120State *s; - - s = GT64120_PCI_HOST_BRIDGE(dev); - - qemu_register_reset(gt64120_reset, s); - return 0; -} - static void gt64120_pci_realize(PCIDevice *d, Error **errp) { /* FIXME: Malta specific hw assumptions ahead */ @@ -1241,9 +1231,8 @@ static const TypeInfo gt64120_pci_info = { static void gt64120_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass); - sdc->init = gt64120_init; + dc->reset = gt64120_reset; dc->vmsd = &vmstate_gt64120; } -- 2.18.0.rc1.1.g3f1ff2140
From: Philippe Mathieu-Daudé <f4bug@amsat.org> The gt64120 is currently listed as uncategorized device. Mark it as bridge device. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-9-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mips/gt64xxx_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index dcd1a66329..1cd8aac658 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -1232,6 +1232,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); dc->reset = gt64120_reset; dc->vmsd = &vmstate_gt64120; } -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:49PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
The gt64120 is currently listed as uncategorized device. Mark it as bridge device.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181002212522.23303-9-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/mips/gt64xxx_pci.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index dcd1a66329..1cd8aac658 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -1232,6 +1232,7 @@ static void gt64120_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass);
+ set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); dc->reset = gt64120_reset; dc->vmsd = &vmstate_gt64120; }
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Since the I/O Bridge device is not implemented, Use the TYPE_UNIMPLEMENTED_DEVICE which suits better: if the user asks for 'unimp' warnings via the -d option then all accesses will generate logging. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com> Message-Id: <20181002212522.23303-10-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- default-configs/sparc64-softmmu.mak | 1 - hw/sparc64/niagara.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/default-configs/sparc64-softmmu.mak b/default-configs/sparc64-softmmu.mak index 52edafe547..ce63d47046 100644 --- a/default-configs/sparc64-softmmu.mak +++ b/default-configs/sparc64-softmmu.mak @@ -16,5 +16,4 @@ CONFIG_SIMBA=y CONFIG_SUNHME=y CONFIG_MC146818RTC=y CONFIG_ISA_TESTDEV=y -CONFIG_EMPTY_SLOT=y CONFIG_SUN4V_RTC=y diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c index 4fa8cb2904..f8a856f611 100644 --- a/hw/sparc64/niagara.c +++ b/hw/sparc64/niagara.c @@ -29,7 +29,7 @@ #include "hw/hw.h" #include "hw/boards.h" #include "hw/char/serial.h" -#include "hw/empty_slot.h" +#include "hw/misc/unimp.h" #include "hw/loader.h" #include "hw/sparc/sparc64.h" #include "hw/timer/sun4v-rtc.h" @@ -161,7 +161,7 @@ static void niagara_init(MachineState *machine) serial_mm_init(sysmem, NIAGARA_UART_BASE, 0, NULL, 115200, serial_hd(0), DEVICE_BIG_ENDIAN); } - empty_slot_init(NIAGARA_IOBBASE, NIAGARA_IOBSIZE); + create_unimplemented_device("sun4v-iob", NIAGARA_IOBBASE, NIAGARA_IOBSIZE); sun4v_rtc_init(NIAGARA_RTC_BASE); } -- 2.18.0.rc1.1.g3f1ff2140
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20181002212522.23303-11-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/alpha/typhoon.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index d74b5b55e1..8004afe45b 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -932,23 +932,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus, return b; } -static int typhoon_pcihost_init(SysBusDevice *dev) -{ - return 0; -} - -static void typhoon_pcihost_class_init(ObjectClass *klass, void *data) -{ - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - - k->init = typhoon_pcihost_init; -} - static const TypeInfo typhoon_pcihost_info = { .name = TYPE_TYPHOON_PCI_HOST_BRIDGE, .parent = TYPE_PCI_HOST_BRIDGE, .instance_size = sizeof(TyphoonState), - .class_init = typhoon_pcihost_class_init, }; static void typhoon_iommu_memory_region_class_init(ObjectClass *klass, -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:51PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20181002212522.23303-11-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/alpha/typhoon.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index d74b5b55e1..8004afe45b 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -932,23 +932,10 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus, return b; }
-static int typhoon_pcihost_init(SysBusDevice *dev) -{ - return 0; -} - -static void typhoon_pcihost_class_init(ObjectClass *klass, void *data) -{ - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - - k->init = typhoon_pcihost_init; -} - static const TypeInfo typhoon_pcihost_info = { .name = TYPE_TYPHOON_PCI_HOST_BRIDGE, .parent = TYPE_PCI_HOST_BRIDGE, .instance_size = sizeof(TyphoonState), - .class_init = typhoon_pcihost_class_init, };
static void typhoon_iommu_memory_region_class_init(ObjectClass *klass,
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20181002212522.23303-12-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/hppa/dino.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c index 564b938e3a..31e09942b5 100644 --- a/hw/hppa/dino.c +++ b/hw/hppa/dino.c @@ -488,17 +488,10 @@ PCIBus *dino_init(MemoryRegion *addr_space, return b; } -static int dino_pcihost_init(SysBusDevice *dev) -{ - return 0; -} - static void dino_pcihost_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); - k->init = dino_pcihost_init; dc->vmsd = &vmstate_dino; } -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:52PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20181002212522.23303-12-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/hppa/dino.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c index 564b938e3a..31e09942b5 100644 --- a/hw/hppa/dino.c +++ b/hw/hppa/dino.c @@ -488,17 +488,10 @@ PCIBus *dino_init(MemoryRegion *addr_space, return b; }
-static int dino_pcihost_init(SysBusDevice *dev) -{ - return 0; -} - static void dino_pcihost_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass);
- k->init = dino_pcihost_init; dc->vmsd = &vmstate_dino; }
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20181002212522.23303-13-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mips/mips_malta.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 29b90bacf3..c1cf0fe12e 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1422,23 +1422,10 @@ void mips_malta_init(MachineState *machine) pci_vga_init(pci_bus); } -static int mips_malta_sysbus_device_init(SysBusDevice *sysbusdev) -{ - return 0; -} - -static void mips_malta_class_init(ObjectClass *klass, void *data) -{ - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - - k->init = mips_malta_sysbus_device_init; -} - static const TypeInfo mips_malta_device = { .name = TYPE_MIPS_MALTA, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(MaltaState), - .class_init = mips_malta_class_init, }; static void mips_malta_machine_init(MachineClass *mc) -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:53PM -0300, Eduardo Habkost wrote:
From: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20181002212522.23303-13-f4bug@amsat.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/mips/mips_malta.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 29b90bacf3..c1cf0fe12e 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1422,23 +1422,10 @@ void mips_malta_init(MachineState *machine) pci_vga_init(pci_bus); }
-static int mips_malta_sysbus_device_init(SysBusDevice *sysbusdev) -{ - return 0; -} - -static void mips_malta_class_init(ObjectClass *klass, void *data) -{ - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - - k->init = mips_malta_sysbus_device_init; -} - static const TypeInfo mips_malta_device = { .name = TYPE_MIPS_MALTA, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(MaltaState), - .class_init = mips_malta_class_init, };
static void mips_malta_machine_init(MachineClass *mc)
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Li Qiang <liq3ea@163.com> Cc: qemu-trivial@nongnu.org Signed-off-by: Li Qiang <liq3ea@163.com> Message-Id: <20181010161025.34796-1-liq3ea@163.com> Reviewed-by: Stefano Garzarella <stefanogarzarella@gmail.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/core/machine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 1987557833..da50ad6de7 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -636,7 +636,7 @@ static void machine_class_init(ObjectClass *oc, void *data) machine_get_memory_encryption, machine_set_memory_encryption, &error_abort); object_class_property_set_description(oc, "memory-encryption", - "Set memory encyption object to use", &error_abort); + "Set memory encryption object to use", &error_abort); } static void machine_class_base_init(ObjectClass *oc, void *data) -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:54PM -0300, Eduardo Habkost wrote:
From: Li Qiang <liq3ea@163.com>
Cc: qemu-trivial@nongnu.org Signed-off-by: Li Qiang <liq3ea@163.com> Message-Id: <20181010161025.34796-1-liq3ea@163.com> Reviewed-by: Stefano Garzarella <stefanogarzarella@gmail.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/core/machine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c index 1987557833..da50ad6de7 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -636,7 +636,7 @@ static void machine_class_init(ObjectClass *oc, void *data) machine_get_memory_encryption, machine_set_memory_encryption, &error_abort); object_class_property_set_description(oc, "memory-encryption", - "Set memory encyption object to use", &error_abort); + "Set memory encryption object to use", &error_abort); }
static void machine_class_base_init(ObjectClass *oc, void *data)
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: David Hildenbrand <david@redhat.com> We're missing "x" after the leading 0. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-2-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mem/memory-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 6de4f70bb4..0b52fe2c5e 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -120,7 +120,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, /* address_space_start indicates the maximum alignment we expect */ if (QEMU_ALIGN_UP(address_space_start, align) != address_space_start) { - error_setg(errp, "the alignment (0%" PRIx64 ") is not supported", + error_setg(errp, "the alignment (0x%" PRIx64 ") is not supported", align); return 0; } -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:03:55PM -0300, Eduardo Habkost wrote:
From: David Hildenbrand <david@redhat.com>
We're missing "x" after the leading 0.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-2-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/mem/memory-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 6de4f70bb4..0b52fe2c5e 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -120,7 +120,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint,
/* address_space_start indicates the maximum alignment we expect */ if (QEMU_ALIGN_UP(address_space_start, align) != address_space_start) { - error_setg(errp, "the alignment (0%" PRIx64 ") is not supported", + error_setg(errp, "the alignment (0x%" PRIx64 ") is not supported", align); return 0; }
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: David Hildenbrand <david@redhat.com> The "at" should actually be a "before". if (new_addr < address_space_start) -> "can't add memory ... before... $address_space_start" So it looks similar to the other check } else if ((new_addr + size) > address_space_end) -> "can't add memory ... beyond..." Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-3-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mem/memory-device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 0b52fe2c5e..7c706fadfc 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -146,7 +146,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, new_addr = *hint; if (new_addr < address_space_start) { error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64 - "] at 0x%" PRIx64, new_addr, size, address_space_start); + "] before 0x%" PRIx64, new_addr, size, + address_space_start); return 0; } else if ((new_addr + size) > address_space_end) { error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64 -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> Handle id==NULL better and indicate that we are dealing with memory devices. Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-4-david@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mem/memory-device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 7c706fadfc..0624184c40 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -175,7 +175,8 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, if (ranges_overlap(md_addr, md_size, new_addr, size)) { if (hint) { const DeviceState *d = DEVICE(md); - error_setg(errp, "address range conflicts with '%s'", d->id); + error_setg(errp, "address range conflicts with memory device" + " id='%s'", d->id ? d->id : "(unnamed)"); goto out; } new_addr = QEMU_ALIGN_UP(md_addr + md_size, align); -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> We're plugging/unplugging a PCDIMMDevice, so directly pass this type instead of a more generic DeviceState. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Acked-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-5-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/pc-dimm.h | 6 +++--- hw/i386/pc.c | 6 +++--- hw/mem/pc-dimm.c | 24 +++++++++++------------- hw/ppc/spapr.c | 8 ++++---- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index b382eb4303..99cbd54de7 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -79,8 +79,8 @@ typedef struct PCDIMMDeviceClass { Error **errp); } PCDIMMDeviceClass; -void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine, +void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, const uint64_t *legacy_align, Error **errp); -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp); -void pc_dimm_unplug(DeviceState *dev, MachineState *machine); +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp); +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine); #endif diff --git a/hw/i386/pc.c b/hw/i386/pc.c index cd5029c149..19ace9dadd 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1704,7 +1704,7 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } - pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), + pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), pcmc->enforce_aligned_dimm ? NULL : &legacy_align, errp); } @@ -1716,7 +1716,7 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev, PCMachineState *pcms = PC_MACHINE(hotplug_dev); bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); - pc_dimm_plug(dev, MACHINE(pcms), &local_err); + pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err); if (local_err) { goto out; } @@ -1776,7 +1776,7 @@ static void pc_memory_unplug(HotplugHandler *hotplug_dev, goto out; } - pc_dimm_unplug(dev, MACHINE(pcms)); + pc_dimm_unplug(PC_DIMM(dev), MACHINE(pcms)); object_unparent(OBJECT(dev)); out: diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index fb6bcaedc4..f46fb7ada2 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -29,24 +29,24 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp); -void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine, +void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, const uint64_t *legacy_align, Error **errp) { - PCDIMMDevice *dimm = PC_DIMM(dev); PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); Error *local_err = NULL; MemoryRegion *mr; uint64_t addr, align; int slot; - slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP, + slot = object_property_get_int(OBJECT(dimm), PC_DIMM_SLOT_PROP, &error_abort); slot = pc_dimm_get_free_slot(slot == PC_DIMM_UNASSIGNED_SLOT ? NULL : &slot, machine->ram_slots, &local_err); if (local_err) { goto out; } - object_property_set_int(OBJECT(dev), slot, PC_DIMM_SLOT_PROP, &error_abort); + object_property_set_int(OBJECT(dimm), slot, PC_DIMM_SLOT_PROP, + &error_abort); trace_mhp_pc_dimm_assigned_slot(slot); mr = ddc->get_memory_region(dimm, &local_err); @@ -55,7 +55,7 @@ void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine, } align = legacy_align ? *legacy_align : memory_region_get_alignment(mr); - addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP, + addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, &error_abort); addr = memory_device_get_free_addr(machine, !addr ? NULL : &addr, align, memory_region_size(mr), &local_err); @@ -63,38 +63,36 @@ void pc_dimm_pre_plug(DeviceState *dev, MachineState *machine, goto out; } trace_mhp_pc_dimm_assigned_address(addr); - object_property_set_uint(OBJECT(dev), addr, PC_DIMM_ADDR_PROP, + object_property_set_uint(OBJECT(dimm), addr, PC_DIMM_ADDR_PROP, &error_abort); out: error_propagate(errp, local_err); } -void pc_dimm_plug(DeviceState *dev, MachineState *machine, Error **errp) +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp) { - PCDIMMDevice *dimm = PC_DIMM(dev); PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, &error_abort); MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); uint64_t addr; - addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP, + addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, &error_abort); memory_device_plug_region(machine, mr, addr); - vmstate_register_ram(vmstate_mr, dev); + vmstate_register_ram(vmstate_mr, DEVICE(dimm)); } -void pc_dimm_unplug(DeviceState *dev, MachineState *machine) +void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) { - PCDIMMDevice *dimm = PC_DIMM(dev); PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, &error_abort); MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); memory_device_unplug_region(machine, mr); - vmstate_unregister_ram(vmstate_mr, dev); + vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); } static int pc_dimm_slot2bitmap(Object *obj, void *opaque) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 98868d893a..c078347b66 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3135,7 +3135,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, size = memory_region_size(mr); - pc_dimm_plug(dev, MACHINE(ms), &local_err); + pc_dimm_plug(dimm, MACHINE(ms), &local_err); if (local_err) { goto out; } @@ -3158,7 +3158,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; out_unplug: - pc_dimm_unplug(dev, MACHINE(ms)); + pc_dimm_unplug(dimm, MACHINE(ms)); out: error_propagate(errp, local_err); } @@ -3202,7 +3202,7 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } - pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), NULL, errp); + pc_dimm_pre_plug(dimm, MACHINE(hotplug_dev), NULL, errp); } struct sPAPRDIMMState { @@ -3314,7 +3314,7 @@ static void spapr_memory_unplug(HotplugHandler *hotplug_dev, DeviceState *dev) sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev); sPAPRDIMMState *ds = spapr_pending_dimm_unplugs_find(spapr, PC_DIMM(dev)); - pc_dimm_unplug(dev, MACHINE(hotplug_dev)); + pc_dimm_unplug(PC_DIMM(dev), MACHINE(hotplug_dev)); object_unparent(OBJECT(dev)); spapr_pending_dimm_unplugs_remove(spapr, ds); } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> While we rephrased most error messages, we missed these. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-6-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mem/memory-device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 0624184c40..87369ef2e1 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -87,7 +87,7 @@ static void memory_device_check_addable(MachineState *ms, uint64_t size, memory_device_used_region_size(OBJECT(ms), &used_region_size); if (used_region_size + size > ms->maxram_size - ms->ram_size) { error_setg(errp, "not enough space, currently 0x%" PRIx64 - " in use of total hot pluggable 0x" RAM_ADDR_FMT, + " in use of total space for memory devices 0x" RAM_ADDR_FMT, used_region_size, ms->maxram_size - ms->ram_size); return; } @@ -145,12 +145,12 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, if (hint) { new_addr = *hint; if (new_addr < address_space_start) { - error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64 + error_setg(errp, "can't add memory device [0x%" PRIx64 ":0x%" PRIx64 "] before 0x%" PRIx64, new_addr, size, address_space_start); return 0; } else if ((new_addr + size) > address_space_end) { - error_setg(errp, "can't add memory [0x%" PRIx64 ":0x%" PRIx64 + error_setg(errp, "can't add memory device [0x%" PRIx64 ":0x%" PRIx64 "] beyond 0x%" PRIx64, new_addr, size, address_space_end); return 0; -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> Some architectures might support memory devices, while they don't support DIMM/NVDIMM. So let's - Rename CONFIG_MEM_HOTPLUG to CONFIG_MEM_DEVICE - Introduce CONFIG_DIMM and use it similarly to CONFIG NVDIMM CONFIG_DIMM and CONFIG_NVDIMM require CONFIG_MEM_DEVICE. Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Acked-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-7-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- default-configs/i386-softmmu.mak | 3 ++- default-configs/ppc64-softmmu.mak | 3 ++- qapi/misc.json | 2 +- hw/Makefile.objs | 2 +- hw/mem/Makefile.objs | 4 ++-- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 8c7d4a0fa0..4c1637338b 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -50,7 +50,8 @@ CONFIG_PCI_Q35=y CONFIG_APIC=y CONFIG_IOAPIC=y CONFIG_PVPANIC=y -CONFIG_MEM_HOTPLUG=y +CONFIG_MEM_DEVICE=y +CONFIG_DIMM=y CONFIG_NVDIMM=y CONFIG_ACPI_NVDIMM=y CONFIG_PCIE_PORT=y diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak index b94af6c7c6..f550573782 100644 --- a/default-configs/ppc64-softmmu.mak +++ b/default-configs/ppc64-softmmu.mak @@ -16,4 +16,5 @@ CONFIG_VIRTIO_VGA=y CONFIG_XICS=$(CONFIG_PSERIES) CONFIG_XICS_SPAPR=$(CONFIG_PSERIES) CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM)) -CONFIG_MEM_HOTPLUG=y +CONFIG_MEM_DEVICE=y +CONFIG_DIMM=y diff --git a/qapi/misc.json b/qapi/misc.json index 3a68af9ca3..9b964644aa 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -2066,7 +2066,7 @@ # # @plugged-memory: size of memory that can be hot-unplugged. This field # is omitted if target doesn't support memory hotplug -# (i.e. CONFIG_MEM_HOTPLUG not defined on build time). +# (i.e. CONFIG_MEM_DEVICE not defined at build time). # # Since: 2.11.0 ## diff --git a/hw/Makefile.objs b/hw/Makefile.objs index a19c1417ed..58872e27e0 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -33,7 +33,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += vfio/ devices-dirs-$(CONFIG_SOFTMMU) += virtio/ devices-dirs-$(CONFIG_SOFTMMU) += watchdog/ devices-dirs-$(CONFIG_SOFTMMU) += xen/ -devices-dirs-$(CONFIG_MEM_HOTPLUG) += mem/ +devices-dirs-$(CONFIG_MEM_DEVICE) += mem/ devices-dirs-$(CONFIG_SOFTMMU) += smbios/ devices-dirs-y += core/ common-obj-y += $(devices-dirs-y) diff --git a/hw/mem/Makefile.objs b/hw/mem/Makefile.objs index 10be4df2a2..3e2f7c5ca2 100644 --- a/hw/mem/Makefile.objs +++ b/hw/mem/Makefile.objs @@ -1,3 +1,3 @@ -common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o -common-obj-$(CONFIG_MEM_HOTPLUG) += memory-device.o +common-obj-$(CONFIG_DIMM) += pc-dimm.o +common-obj-$(CONFIG_MEM_DEVICE) += memory-device.o common-obj-$(CONFIG_NVDIMM) += nvdimm.o -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> Let's properly forward the errors, so errors from get_region_size() / get_plugged_size() can be handled. Users right now call both functions after the device has been realized, which is will never fail, so it is fine to continue using error_abort. While at it, remove a leftover error check (suggested by Igor). Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-8-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 4 ++-- hw/mem/memory-device.c | 9 +++------ hw/mem/pc-dimm.c | 5 +++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 2853b084b5..f02b229837 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -33,8 +33,8 @@ typedef struct MemoryDeviceClass { InterfaceClass parent_class; uint64_t (*get_addr)(const MemoryDeviceState *md); - uint64_t (*get_plugged_size)(const MemoryDeviceState *md); - uint64_t (*get_region_size)(const MemoryDeviceState *md); + uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); + uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); void (*fill_device_info)(const MemoryDeviceState *md, MemoryDeviceInfo *info); } MemoryDeviceClass; diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 87369ef2e1..c1e1b81cf2 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -60,7 +60,7 @@ static int memory_device_used_region_size(Object *obj, void *opaque) const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); if (dev->realized) { - *size += mdc->get_region_size(md); + *size += mdc->get_region_size(md, &error_abort); } } @@ -167,10 +167,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, uint64_t md_size, md_addr; md_addr = mdc->get_addr(md); - md_size = mdc->get_region_size(md); - if (*errp) { - goto out; - } + md_size = mdc->get_region_size(md, &error_abort); if (ranges_overlap(md_addr, md_size, new_addr, size)) { if (hint) { @@ -234,7 +231,7 @@ static int memory_device_plugged_size(Object *obj, void *opaque) const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); if (dev->realized) { - *size += mdc->get_plugged_size(md); + *size += mdc->get_plugged_size(md, &error_abort); } } diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index f46fb7ada2..130f78d0de 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -234,14 +234,15 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) return dimm->addr; } -static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md) +static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, + Error **errp) { /* dropping const here is fine as we don't touch the memory region */ PCDIMMDevice *dimm = PC_DIMM(md); const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md); MemoryRegion *mr; - mr = ddc->get_memory_region(dimm, &error_abort); + mr = ddc->get_memory_region(dimm, errp); if (!mr) { return 0; } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> Document the functions. Don't document get_region_size(), as we will be dropping/replacing that one soon. Use same documentation style as in include/exec/memory.h, but don't document the parameters, as they are self-explanatory. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-9-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index f02b229837..0474a3dd11 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -29,12 +29,48 @@ typedef struct MemoryDeviceState { Object parent_obj; } MemoryDeviceState; +/** + * MemoryDeviceClass: + * + * All memory devices need to implement TYPE_MEMORY_DEVICE as an interface. + * + * A memory device is a device that owns a memory region which is + * mapped into guest physical address space at a certain address. The + * address in guest physical memory can either be specified explicitly + * or get assigned automatically. + */ typedef struct MemoryDeviceClass { + /* private */ InterfaceClass parent_class; + /* + * Return the address of the memory device in guest physical memory. + * + * Called when (un)plugging a memory device or when iterating over + * all memory devices mapped into guest physical address space. + * + * If "0" is returned, no address has been specified by the user and + * no address has been assigned to this memory device yet. + */ uint64_t (*get_addr)(const MemoryDeviceState *md); + + /* + * Return the amount of memory provided by the memory device currently + * usable ("plugged") by the VM. + * + * Called when calculating the total amount of ram available to the + * VM (e.g. to report memory stats to the user). + * + * This is helpful for devices that dynamically manage the amount of + * memory accessible by the guest via the reserved memory region. For + * most devices, this corresponds to the size of the memory region. + */ uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); + + /* + * Translate the memory device into #MemoryDeviceInfo. + */ void (*fill_device_info)(const MemoryDeviceState *md, MemoryDeviceInfo *info); } MemoryDeviceClass; -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> We will factor out get_memory_region() from pc-dimm to memory device code soon. Once that is done, get_region_size() can be implemented generically and essentially be replaced by memory_device_get_region_size (and work only on get_memory_region()). We have some users of get_memory_region() (spapr and pc-dimm code) that are only interested in the size. So let's rework them to use memory_device_get_region_size() first, then we can factor out get_memory_region() and eventually remove get_region_size() without touching the same code multiple times. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-10-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 2 ++ hw/mem/memory-device.c | 13 ++++++++++--- hw/mem/pc-dimm.c | 10 ++++------ hw/ppc/spapr.c | 21 +++++++-------------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 0474a3dd11..898df3057d 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -83,5 +83,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, void memory_device_plug_region(MachineState *ms, MemoryRegion *mr, uint64_t addr); void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr); +uint64_t memory_device_get_region_size(const MemoryDeviceState *md, + Error **errp); #endif diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index c1e1b81cf2..425830c65c 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -57,10 +57,9 @@ static int memory_device_used_region_size(Object *obj, void *opaque) if (object_dynamic_cast(obj, TYPE_MEMORY_DEVICE)) { const DeviceState *dev = DEVICE(obj); const MemoryDeviceState *md = MEMORY_DEVICE(obj); - const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj); if (dev->realized) { - *size += mdc->get_region_size(md, &error_abort); + *size += memory_device_get_region_size(md, &error_abort); } } @@ -167,7 +166,7 @@ uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, uint64_t md_size, md_addr; md_addr = mdc->get_addr(md); - md_size = mdc->get_region_size(md, &error_abort); + md_size = memory_device_get_region_size(md, &error_abort); if (ranges_overlap(md_addr, md_size, new_addr, size)) { if (hint) { @@ -266,6 +265,14 @@ void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr) memory_region_del_subregion(&ms->device_memory->mr, mr); } +uint64_t memory_device_get_region_size(const MemoryDeviceState *md, + Error **errp) +{ + MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); + + return mdc->get_region_size(md, errp); +} + static const TypeInfo memory_device_info = { .name = TYPE_MEMORY_DEVICE, .parent = TYPE_INTERFACE, diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 130f78d0de..3e43ec8742 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -161,16 +161,14 @@ static Property pc_dimm_properties[] = { static void pc_dimm_get_size(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { + Error *local_err = NULL; uint64_t value; - MemoryRegion *mr; - PCDIMMDevice *dimm = PC_DIMM(obj); - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj); - mr = ddc->get_memory_region(dimm, errp); - if (!mr) { + value = memory_device_get_region_size(MEMORY_DEVICE(obj), &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } - value = memory_region_size(mr); visit_type_uint64(v, name, &value, errp); } diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index c078347b66..c08130facb 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3128,12 +3128,10 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error *local_err = NULL; sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev); PCDIMMDevice *dimm = PC_DIMM(dev); - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); uint64_t size, addr; uint32_t node; - size = memory_region_size(mr); + size = memory_device_get_region_size(MEMORY_DEVICE(dev), &error_abort); pc_dimm_plug(dimm, MACHINE(ms), &local_err); if (local_err) { @@ -3169,9 +3167,7 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, const sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(hotplug_dev); sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev); PCDIMMDevice *dimm = PC_DIMM(dev); - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); Error *local_err = NULL; - MemoryRegion *mr; uint64_t size; Object *memdev; hwaddr pagesize; @@ -3181,11 +3177,11 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } - mr = ddc->get_memory_region(dimm, errp); - if (!mr) { + size = memory_device_get_region_size(MEMORY_DEVICE(dimm), &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } - size = memory_region_size(mr); if (size % SPAPR_MEMORY_BLOCK_SIZE) { error_setg(errp, "Hotplugged memory size must be a multiple of " @@ -3257,9 +3253,8 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms, PCDIMMDevice *dimm) { sPAPRDRConnector *drc; - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); - uint64_t size = memory_region_size(mr); + uint64_t size = memory_device_get_region_size(MEMORY_DEVICE(dimm), + &error_abort); uint32_t nr_lmbs = size / SPAPR_MEMORY_BLOCK_SIZE; uint32_t avail_lmbs = 0; uint64_t addr_start, addr; @@ -3325,14 +3320,12 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, sPAPRMachineState *spapr = SPAPR_MACHINE(hotplug_dev); Error *local_err = NULL; PCDIMMDevice *dimm = PC_DIMM(dev); - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); uint32_t nr_lmbs; uint64_t size, addr_start, addr; int i; sPAPRDRConnector *drc; - size = memory_region_size(mr); + size = memory_device_get_region_size(MEMORY_DEVICE(dimm), &error_abort); nr_lmbs = size / SPAPR_MEMORY_BLOCK_SIZE; addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> The memory region is necessary for plugging/unplugging a memory device. The region size (via get_region_size()) is no longer sufficient, as besides the alignment, also the region itself is required in order to add it to the device memory region of the machine via - memory_region_add_subregion - memory_region_del_subregion So, to factor out plugging/unplugging of memory devices from pc-dimm code, we have to factor out access to the memory region first. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-11-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 15 +++++++++++++++ include/hw/mem/pc-dimm.h | 4 ---- hw/mem/nvdimm.c | 9 ++++++--- hw/mem/pc-dimm.c | 27 ++++++++++++++++++--------- 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 898df3057d..659f38385c 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -38,6 +38,11 @@ typedef struct MemoryDeviceState { * mapped into guest physical address space at a certain address. The * address in guest physical memory can either be specified explicitly * or get assigned automatically. + * + * Conceptually, memory devices only span one memory region. If multiple + * successive memory regions are used, a covering memory region has to + * be provided. Scattered memory regions are not supported for single + * devices. */ typedef struct MemoryDeviceClass { /* private */ @@ -68,6 +73,16 @@ typedef struct MemoryDeviceClass { uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); + /* + * Return the memory region of the memory device. + * + * Called when (un)plugging the memory device, to (un)map the + * memory region in guest physical memory, but also to detect the + * required alignment during address assignment or when the size of the + * memory region is required. + */ + MemoryRegion *(*get_memory_region)(MemoryDeviceState *md, Error **errp); + /* * Translate the memory device into #MemoryDeviceInfo. */ diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index 99cbd54de7..01436b9f50 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -61,9 +61,6 @@ typedef struct PCDIMMDevice { * PCDIMMDeviceClass: * @realize: called after common dimm is realized so that the dimm based * devices get the chance to do specified operations. - * @get_memory_region: returns #MemoryRegion associated with @dimm which - * is directly mapped into the physical address space of guest. Will not - * fail after the device was realized. * @get_vmstate_memory_region: returns #MemoryRegion which indicates the * memory of @dimm should be kept during live migration. Will not fail * after the device was realized. @@ -74,7 +71,6 @@ typedef struct PCDIMMDeviceClass { /* public */ void (*realize)(PCDIMMDevice *dimm, Error **errp); - MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm, Error **errp); MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm, Error **errp); } PCDIMMDeviceClass; diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c index 1c6674c4ed..49324f3fae 100644 --- a/hw/mem/nvdimm.c +++ b/hw/mem/nvdimm.c @@ -27,6 +27,7 @@ #include "qapi/error.h" #include "qapi/visitor.h" #include "hw/mem/nvdimm.h" +#include "hw/mem/memory-device.h" static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) @@ -118,9 +119,10 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp) nvdimm->nvdimm_mr->align = align; } -static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm, Error **errp) +static MemoryRegion *nvdimm_md_get_memory_region(MemoryDeviceState *md, + Error **errp) { - NVDIMMDevice *nvdimm = NVDIMM(dimm); + NVDIMMDevice *nvdimm = NVDIMM(md); Error *local_err = NULL; if (!nvdimm->nvdimm_mr) { @@ -190,11 +192,12 @@ static Property nvdimm_properties[] = { static void nvdimm_class_init(ObjectClass *oc, void *data) { PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc); + MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc); NVDIMMClass *nvc = NVDIMM_CLASS(oc); DeviceClass *dc = DEVICE_CLASS(oc); ddc->realize = nvdimm_realize; - ddc->get_memory_region = nvdimm_get_memory_region; + mdc->get_memory_region = nvdimm_md_get_memory_region; dc->props = nvdimm_properties; nvc->read_label_data = nvdimm_read_label_data; diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 3e43ec8742..c9f6ad589e 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -32,7 +32,7 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp); void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, const uint64_t *legacy_align, Error **errp) { - PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); + MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dimm); Error *local_err = NULL; MemoryRegion *mr; uint64_t addr, align; @@ -49,7 +49,7 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, &error_abort); trace_mhp_pc_dimm_assigned_slot(slot); - mr = ddc->get_memory_region(dimm, &local_err); + mr = mdc->get_memory_region(MEMORY_DEVICE(dimm), &local_err); if (local_err) { goto out; } @@ -72,9 +72,11 @@ out: void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp) { PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); + MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, &error_abort); - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); + MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dimm), + &error_abort); uint64_t addr; addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, @@ -87,9 +89,11 @@ void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp) void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) { PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); + MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, &error_abort); - MemoryRegion *mr = ddc->get_memory_region(dimm, &error_abort); + MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dimm), + &error_abort); memory_device_unplug_region(machine, mr); vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); @@ -235,12 +239,11 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, Error **errp) { - /* dropping const here is fine as we don't touch the memory region */ - PCDIMMDevice *dimm = PC_DIMM(md); - const PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(md); + MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); MemoryRegion *mr; - mr = ddc->get_memory_region(dimm, errp); + /* dropping const here is fine as we don't touch the memory region */ + mr = mdc->get_memory_region((MemoryDeviceState *)md, errp); if (!mr) { return 0; } @@ -248,6 +251,12 @@ static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, return memory_region_size(mr); } +static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md, + Error **errp) +{ + return pc_dimm_get_memory_region(PC_DIMM(md), errp); +} + static void pc_dimm_md_fill_device_info(const MemoryDeviceState *md, MemoryDeviceInfo *info) { @@ -289,13 +298,13 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) dc->props = pc_dimm_properties; dc->desc = "DIMM memory module"; - ddc->get_memory_region = pc_dimm_get_memory_region; ddc->get_vmstate_memory_region = pc_dimm_get_memory_region; mdc->get_addr = pc_dimm_md_get_addr; /* for a dimm plugged_size == region_size */ mdc->get_plugged_size = pc_dimm_md_get_region_size; mdc->get_region_size = pc_dimm_md_get_region_size; + mdc->get_memory_region = pc_dimm_md_get_memory_region; mdc->fill_device_info = pc_dimm_md_fill_device_info; } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> There are no remaining users of get_region_size() except memory_device_get_region_size() itself. We can make memory_device_get_region_size() work directly on get_memory_region() instead and drop get_region_size(). In addition, we can now use memory_device_get_region_size() in pc-dimm code to implement get_plugged_size()" Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-12-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 1 - hw/mem/memory-device.c | 11 +++++++++-- hw/mem/pc-dimm.c | 18 +----------------- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 659f38385c..9c3398bc4a 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -71,7 +71,6 @@ typedef struct MemoryDeviceClass { * most devices, this corresponds to the size of the memory region. */ uint64_t (*get_plugged_size)(const MemoryDeviceState *md, Error **errp); - uint64_t (*get_region_size)(const MemoryDeviceState *md, Error **errp); /* * Return the memory region of the memory device. diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 425830c65c..07749bab69 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -268,9 +268,16 @@ void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr) uint64_t memory_device_get_region_size(const MemoryDeviceState *md, Error **errp) { - MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); + const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); + MemoryRegion *mr; - return mdc->get_region_size(md, errp); + /* dropping const here is fine as we don't touch the memory region */ + mr = mdc->get_memory_region((MemoryDeviceState *)md, errp); + if (!mr) { + return 0; + } + + return memory_region_size(mr); } static const TypeInfo memory_device_info = { diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index c9f6ad589e..47b2e83389 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -236,21 +236,6 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) return dimm->addr; } -static uint64_t pc_dimm_md_get_region_size(const MemoryDeviceState *md, - Error **errp) -{ - MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); - MemoryRegion *mr; - - /* dropping const here is fine as we don't touch the memory region */ - mr = mdc->get_memory_region((MemoryDeviceState *)md, errp); - if (!mr) { - return 0; - } - - return memory_region_size(mr); -} - static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md, Error **errp) { @@ -302,8 +287,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) mdc->get_addr = pc_dimm_md_get_addr; /* for a dimm plugged_size == region_size */ - mdc->get_plugged_size = pc_dimm_md_get_region_size; - mdc->get_region_size = pc_dimm_md_get_region_size; + mdc->get_plugged_size = memory_device_get_region_size; mdc->get_memory_region = pc_dimm_md_get_memory_region; mdc->fill_device_info = pc_dimm_md_fill_device_info; } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> To be able to factor out address assignment of memory devices, we will have to read (get_addr()) and write (set_addr()) the address. We can't use properties for this purpose, as properties are device specific. E.g. while the address property for a DIMM is called "addr", it might be called differently (e.g. "memaddr") for other devices. Especially virtio based memory devices cannot use "addr" as that is already reserved and used for the address on the bus (for the proxy device). Also, it might be possible to have memory devices without address properties (e.g. internal DIMM-like thingies). In contrast to get_addr(), we expect that set_addr() can fail. Keep it simple for now for pc-dimm and simply set the static property, that will fail once realized. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-13-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 8 ++++++++ hw/mem/pc-dimm.c | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 9c3398bc4a..ed7cf5bf4b 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -59,6 +59,14 @@ typedef struct MemoryDeviceClass { */ uint64_t (*get_addr)(const MemoryDeviceState *md); + /* + * Set the address of the memory device in guest physical memory. + * + * Called when plugging the memory device to configure the determined + * address in guest physical memory. + */ + void (*set_addr)(MemoryDeviceState *md, uint64_t addr, Error **errp); + /* * Return the amount of memory provided by the memory device currently * usable ("plugged") by the VM. diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 47b2e83389..4a15091734 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -236,6 +236,12 @@ static uint64_t pc_dimm_md_get_addr(const MemoryDeviceState *md) return dimm->addr; } +static void pc_dimm_md_set_addr(MemoryDeviceState *md, uint64_t addr, + Error **errp) +{ + object_property_set_uint(OBJECT(md), addr, PC_DIMM_ADDR_PROP, errp); +} + static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md, Error **errp) { @@ -286,6 +292,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) ddc->get_vmstate_memory_region = pc_dimm_get_memory_region; mdc->get_addr = pc_dimm_md_get_addr; + mdc->set_addr = pc_dimm_md_set_addr; /* for a dimm plugged_size == region_size */ mdc->get_plugged_size = memory_device_get_region_size; mdc->get_memory_region = pc_dimm_md_get_memory_region; -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> With all required memory device class functions in place, we can factor out pre_plug handling of memory devices. Take proper care of errors. We still have to carry along legacy_align required for pc compatibility handling. We will factor out tracing of the address separately in a follow-up patch. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-14-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 5 ++--- hw/mem/memory-device.c | 32 +++++++++++++++++++++++++++++--- hw/mem/pc-dimm.c | 15 +++------------ 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index ed7cf5bf4b..d92acd2425 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -99,9 +99,8 @@ typedef struct MemoryDeviceClass { MemoryDeviceInfoList *qmp_memory_device_list(void); uint64_t get_plugged_memory_size(void); -uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, - uint64_t align, uint64_t size, - Error **errp); +void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms, + const uint64_t *legacy_align, Error **errp); void memory_device_plug_region(MachineState *ms, MemoryRegion *mr, uint64_t addr); void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr); diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 07749bab69..aa4ddd5918 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -93,9 +93,10 @@ static void memory_device_check_addable(MachineState *ms, uint64_t size, } -uint64_t memory_device_get_free_addr(MachineState *ms, const uint64_t *hint, - uint64_t align, uint64_t size, - Error **errp) +static uint64_t memory_device_get_free_addr(MachineState *ms, + const uint64_t *hint, + uint64_t align, uint64_t size, + Error **errp) { uint64_t address_space_start, address_space_end; GSList *list = NULL, *item; @@ -247,6 +248,31 @@ uint64_t get_plugged_memory_size(void) return size; } +void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms, + const uint64_t *legacy_align, Error **errp) +{ + const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); + Error *local_err = NULL; + uint64_t addr, align; + MemoryRegion *mr; + + mr = mdc->get_memory_region(md, &local_err); + if (local_err) { + goto out; + } + + align = legacy_align ? *legacy_align : memory_region_get_alignment(mr); + addr = mdc->get_addr(md); + addr = memory_device_get_free_addr(ms, !addr ? NULL : &addr, align, + memory_region_size(mr), &local_err); + if (local_err) { + goto out; + } + mdc->set_addr(md, addr, &local_err); +out: + error_propagate(errp, local_err); +} + void memory_device_plug_region(MachineState *ms, MemoryRegion *mr, uint64_t addr) { diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 4a15091734..b2959bcf0e 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -32,10 +32,8 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp); void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, const uint64_t *legacy_align, Error **errp) { - MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dimm); Error *local_err = NULL; - MemoryRegion *mr; - uint64_t addr, align; + uint64_t addr; int slot; slot = object_property_get_int(OBJECT(dimm), PC_DIMM_SLOT_PROP, @@ -49,22 +47,15 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, &error_abort); trace_mhp_pc_dimm_assigned_slot(slot); - mr = mdc->get_memory_region(MEMORY_DEVICE(dimm), &local_err); + memory_device_pre_plug(MEMORY_DEVICE(dimm), machine, legacy_align, + &local_err); if (local_err) { goto out; } - align = legacy_align ? *legacy_align : memory_region_get_alignment(mr); addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, &error_abort); - addr = memory_device_get_free_addr(machine, !addr ? NULL : &addr, align, - memory_region_size(mr), &local_err); - if (local_err) { - goto out; - } trace_mhp_pc_dimm_assigned_address(addr); - object_property_set_uint(OBJECT(dimm), addr, PC_DIMM_ADDR_PROP, - &error_abort); out: error_propagate(errp, local_err); } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> With the new memory device functions in place, we can factor out plugging of memory devices completely. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-15-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 3 +-- hw/mem/memory-device.c | 13 ++++++++++--- hw/mem/pc-dimm.c | 9 +-------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index d92acd2425..53d89dac89 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -101,8 +101,7 @@ MemoryDeviceInfoList *qmp_memory_device_list(void); uint64_t get_plugged_memory_size(void); void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms, const uint64_t *legacy_align, Error **errp); -void memory_device_plug_region(MachineState *ms, MemoryRegion *mr, - uint64_t addr); +void memory_device_plug(MemoryDeviceState *md, MachineState *ms); void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr); uint64_t memory_device_get_region_size(const MemoryDeviceState *md, Error **errp); diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index aa4ddd5918..6569896b7e 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -273,10 +273,17 @@ out: error_propagate(errp, local_err); } -void memory_device_plug_region(MachineState *ms, MemoryRegion *mr, - uint64_t addr) +void memory_device_plug(MemoryDeviceState *md, MachineState *ms) { - /* we expect a previous call to memory_device_get_free_addr() */ + const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); + const uint64_t addr = mdc->get_addr(md); + MemoryRegion *mr; + + /* + * We expect that a previous call to memory_device_pre_plug() succeeded, so + * it can't fail at this point. + */ + mr = mdc->get_memory_region(md, &error_abort); g_assert(ms->device_memory); memory_region_add_subregion(&ms->device_memory->mr, diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index b2959bcf0e..647841bcd4 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -63,17 +63,10 @@ out: void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp) { PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); - MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, &error_abort); - MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dimm), - &error_abort); - uint64_t addr; - - addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, - &error_abort); - memory_device_plug_region(machine, mr, addr); + memory_device_plug(MEMORY_DEVICE(dimm), machine); vmstate_register_ram(vmstate_mr, DEVICE(dimm)); } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> With the new memory device functions in place, we can factor out unplugging of memory devices completely. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-16-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/hw/mem/memory-device.h | 2 +- hw/mem/memory-device.c | 11 +++++++++-- hw/mem/pc-dimm.c | 5 +---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h index 53d89dac89..e904e194d5 100644 --- a/include/hw/mem/memory-device.h +++ b/include/hw/mem/memory-device.h @@ -102,7 +102,7 @@ uint64_t get_plugged_memory_size(void); void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms, const uint64_t *legacy_align, Error **errp); void memory_device_plug(MemoryDeviceState *md, MachineState *ms); -void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr); +void memory_device_unplug(MemoryDeviceState *md, MachineState *ms); uint64_t memory_device_get_region_size(const MemoryDeviceState *md, Error **errp); diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 6569896b7e..30585d6148 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -290,9 +290,16 @@ void memory_device_plug(MemoryDeviceState *md, MachineState *ms) addr - ms->device_memory->base, mr); } -void memory_device_unplug_region(MachineState *ms, MemoryRegion *mr) +void memory_device_unplug(MemoryDeviceState *md, MachineState *ms) { - /* we expect a previous call to memory_device_get_free_addr() */ + const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(md); + MemoryRegion *mr; + + /* + * We expect that a previous call to memory_device_pre_plug() succeeded, so + * it can't fail at this point. + */ + mr = mdc->get_memory_region(md, &error_abort); g_assert(ms->device_memory); memory_region_del_subregion(&ms->device_memory->mr, mr); diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 647841bcd4..6c854139a7 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -73,13 +73,10 @@ void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp) void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine) { PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); - MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, &error_abort); - MemoryRegion *mr = mdc->get_memory_region(MEMORY_DEVICE(dimm), - &error_abort); - memory_device_unplug_region(machine, mr); + memory_device_unplug(MEMORY_DEVICE(dimm), machine); vmstate_unregister_ram(vmstate_mr, DEVICE(dimm)); } -- 2.18.0.rc1.1.g3f1ff2140
From: David Hildenbrand <david@redhat.com> Let's trace the address and the id of a memory device when pre_plugging/plugging/unplugging succeeded. Trace it when pre_plugging as well as when plugging, so we really know when a specific address is actually used. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20181005092024.14344-17-david@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/mem/memory-device.c | 8 ++++++++ hw/mem/pc-dimm.c | 8 -------- hw/mem/trace-events | 5 ++++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c index 30585d6148..7de1ccd497 100644 --- a/hw/mem/memory-device.c +++ b/hw/mem/memory-device.c @@ -17,6 +17,7 @@ #include "qemu/range.h" #include "hw/virtio/vhost.h" #include "sysemu/kvm.h" +#include "trace.h" static gint memory_device_addr_sort(gconstpointer a, gconstpointer b) { @@ -269,6 +270,10 @@ void memory_device_pre_plug(MemoryDeviceState *md, MachineState *ms, goto out; } mdc->set_addr(md, addr, &local_err); + if (!local_err) { + trace_memory_device_pre_plug(DEVICE(md)->id ? DEVICE(md)->id : "", + addr); + } out: error_propagate(errp, local_err); } @@ -288,6 +293,7 @@ void memory_device_plug(MemoryDeviceState *md, MachineState *ms) memory_region_add_subregion(&ms->device_memory->mr, addr - ms->device_memory->base, mr); + trace_memory_device_plug(DEVICE(md)->id ? DEVICE(md)->id : "", addr); } void memory_device_unplug(MemoryDeviceState *md, MachineState *ms) @@ -303,6 +309,8 @@ void memory_device_unplug(MemoryDeviceState *md, MachineState *ms) g_assert(ms->device_memory); memory_region_del_subregion(&ms->device_memory->mr, mr); + trace_memory_device_unplug(DEVICE(md)->id ? DEVICE(md)->id : "", + mdc->get_addr(md)); } uint64_t memory_device_get_region_size(const MemoryDeviceState *md, diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 6c854139a7..0c9b9e8292 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -33,7 +33,6 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, const uint64_t *legacy_align, Error **errp) { Error *local_err = NULL; - uint64_t addr; int slot; slot = object_property_get_int(OBJECT(dimm), PC_DIMM_SLOT_PROP, @@ -49,13 +48,6 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, memory_device_pre_plug(MEMORY_DEVICE(dimm), machine, legacy_align, &local_err); - if (local_err) { - goto out; - } - - addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, - &error_abort); - trace_mhp_pc_dimm_assigned_address(addr); out: error_propagate(errp, local_err); } diff --git a/hw/mem/trace-events b/hw/mem/trace-events index e150dcc497..0f2f278ff2 100644 --- a/hw/mem/trace-events +++ b/hw/mem/trace-events @@ -2,4 +2,7 @@ # hw/mem/pc-dimm.c mhp_pc_dimm_assigned_slot(int slot) "%d" -mhp_pc_dimm_assigned_address(uint64_t addr) "0x%"PRIx64 +# hw/mem/memory-device.c +memory_device_pre_plug(const char *id, uint64_t addr) "id=%s addr=0x%"PRIx64 +memory_device_plug(const char *id, uint64_t addr) "id=%s addr=0x%"PRIx64 +memory_device_unplug(const char *id, uint64_t addr) "id=%s addr=0x%"PRIx64 -- 2.18.0.rc1.1.g3f1ff2140
From: Cédric Le Goater <clg@kaod.org> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20181001063803.22330-2-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/etraxfs_eth.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c index a6932432b1..64ab3df1ae 100644 --- a/hw/net/etraxfs_eth.c +++ b/hw/net/etraxfs_eth.c @@ -23,6 +23,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/sysbus.h" #include "net/net.h" #include "hw/cris/etraxfs.h" @@ -584,14 +585,14 @@ static NetClientInfo net_etraxfs_info = { .link_status_changed = eth_set_link, }; -static int fs_eth_init(SysBusDevice *sbd) +static void etraxfs_eth_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); ETRAXFSEthState *s = ETRAX_FS_ETH(dev); if (!s->dma_out || !s->dma_in) { - error_report("Unconnected ETRAX-FS Ethernet MAC"); - return -1; + error_setg(errp, "Unconnected ETRAX-FS Ethernet MAC"); + return; } s->dma_out->client.push = eth_tx_push; @@ -611,7 +612,6 @@ static int fs_eth_init(SysBusDevice *sbd) tdk_init(&s->phy); mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr); - return 0; } static Property etraxfs_eth_properties[] = { @@ -625,9 +625,8 @@ static Property etraxfs_eth_properties[] = { static void etraxfs_eth_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = fs_eth_init; + dc->realize = etraxfs_eth_realize; dc->props = etraxfs_eth_properties; /* Reason: pointer properties "dma_out", "dma_in" */ dc->user_creatable = false; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:11PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20181001063803.22330-2-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/etraxfs_eth.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c index a6932432b1..64ab3df1ae 100644 --- a/hw/net/etraxfs_eth.c +++ b/hw/net/etraxfs_eth.c @@ -23,6 +23,7 @@ */
#include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/sysbus.h" #include "net/net.h" #include "hw/cris/etraxfs.h" @@ -584,14 +585,14 @@ static NetClientInfo net_etraxfs_info = { .link_status_changed = eth_set_link, };
-static int fs_eth_init(SysBusDevice *sbd) +static void etraxfs_eth_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); ETRAXFSEthState *s = ETRAX_FS_ETH(dev);
if (!s->dma_out || !s->dma_in) { - error_report("Unconnected ETRAX-FS Ethernet MAC"); - return -1; + error_setg(errp, "Unconnected ETRAX-FS Ethernet MAC"); + return; }
s->dma_out->client.push = eth_tx_push; @@ -611,7 +612,6 @@ static int fs_eth_init(SysBusDevice *sbd)
tdk_init(&s->phy); mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr); - return 0; }
static Property etraxfs_eth_properties[] = { @@ -625,9 +625,8 @@ static Property etraxfs_eth_properties[] = { static void etraxfs_eth_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = fs_eth_init; + dc->realize = etraxfs_eth_realize; dc->props = etraxfs_eth_properties; /* Reason: pointer properties "dma_out", "dma_in" */ dc->user_creatable = false;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181001063803.22330-3-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/etraxfs_eth.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c index 64ab3df1ae..36855804db 100644 --- a/hw/net/etraxfs_eth.c +++ b/hw/net/etraxfs_eth.c @@ -127,7 +127,7 @@ tdk_write(struct qemu_phy *phy, unsigned int req, unsigned int data) } static void -tdk_init(struct qemu_phy *phy) +tdk_reset(struct qemu_phy *phy) { phy->regs[0] = 0x3100; /* PHY Id. */ @@ -136,9 +136,6 @@ tdk_init(struct qemu_phy *phy) /* Autonegotiation advertisement reg. */ phy->regs[4] = 0x01E1; phy->link = 1; - - phy->read = tdk_read; - phy->write = tdk_write; } struct qemu_mdio @@ -585,6 +582,27 @@ static NetClientInfo net_etraxfs_info = { .link_status_changed = eth_set_link, }; +static void etraxfs_eth_reset(DeviceState *dev) +{ + ETRAXFSEthState *s = ETRAX_FS_ETH(dev); + + memset(s->regs, 0, sizeof(s->regs)); + memset(s->macaddr, 0, sizeof(s->macaddr)); + s->duplex_mismatch = 0; + + s->mdio_bus.mdc = 0; + s->mdio_bus.mdio = 0; + s->mdio_bus.state = 0; + s->mdio_bus.drive = 0; + s->mdio_bus.cnt = 0; + s->mdio_bus.addr = 0; + s->mdio_bus.opc = 0; + s->mdio_bus.req = 0; + s->mdio_bus.data = 0; + + tdk_reset(&s->phy); +} + static void etraxfs_eth_realize(DeviceState *dev, Error **errp) { SysBusDevice *sbd = SYS_BUS_DEVICE(dev); @@ -609,8 +627,8 @@ static void etraxfs_eth_realize(DeviceState *dev, Error **errp) object_get_typename(OBJECT(s)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); - - tdk_init(&s->phy); + s->phy.read = tdk_read; + s->phy.write = tdk_write; mdio_attach(&s->mdio_bus, &s->phy, s->phyaddr); } @@ -627,6 +645,7 @@ static void etraxfs_eth_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc->realize = etraxfs_eth_realize; + dc->reset = etraxfs_eth_reset; dc->props = etraxfs_eth_properties; /* Reason: pointer properties "dma_out", "dma_in" */ dc->user_creatable = false; -- 2.18.0.rc1.1.g3f1ff2140
From: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181001063803.22330-4-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/lan9118.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c index b9032dac59..a6269d9463 100644 --- a/hw/net/lan9118.c +++ b/hw/net/lan9118.c @@ -1320,9 +1320,9 @@ static NetClientInfo net_lan9118_info = { .link_status_changed = lan9118_set_link, }; -static int lan9118_init1(SysBusDevice *sbd) +static void lan9118_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); lan9118_state *s = LAN9118(dev); QEMUBH *bh; int i; @@ -1349,8 +1349,6 @@ static int lan9118_init1(SysBusDevice *sbd) s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT); ptimer_set_freq(s->timer, 10000); ptimer_set_limit(s->timer, 0xffff, 1); - - return 0; } static Property lan9118_properties[] = { @@ -1362,12 +1360,11 @@ static Property lan9118_properties[] = { static void lan9118_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = lan9118_init1; dc->reset = lan9118_reset; dc->props = lan9118_properties; dc->vmsd = &vmstate_lan9118; + dc->realize = lan9118_realize; } static const TypeInfo lan9118_info = { -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:13PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181001063803.22330-4-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/lan9118.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c index b9032dac59..a6269d9463 100644 --- a/hw/net/lan9118.c +++ b/hw/net/lan9118.c @@ -1320,9 +1320,9 @@ static NetClientInfo net_lan9118_info = { .link_status_changed = lan9118_set_link, };
-static int lan9118_init1(SysBusDevice *sbd) +static void lan9118_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); lan9118_state *s = LAN9118(dev); QEMUBH *bh; int i; @@ -1349,8 +1349,6 @@ static int lan9118_init1(SysBusDevice *sbd) s->timer = ptimer_init(bh, PTIMER_POLICY_DEFAULT); ptimer_set_freq(s->timer, 10000); ptimer_set_limit(s->timer, 0xffff, 1); - - return 0; }
static Property lan9118_properties[] = { @@ -1362,12 +1360,11 @@ static Property lan9118_properties[] = { static void lan9118_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = lan9118_init1; dc->reset = lan9118_reset; dc->props = lan9118_properties; dc->vmsd = &vmstate_lan9118; + dc->realize = lan9118_realize; }
static const TypeInfo lan9118_info = {
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-5-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/lance.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/net/lance.c b/hw/net/lance.c index a08d5ac6a8..f987b2fd18 100644 --- a/hw/net/lance.c +++ b/hw/net/lance.c @@ -97,9 +97,9 @@ static const VMStateDescription vmstate_lance = { } }; -static int lance_init(SysBusDevice *sbd) +static void lance_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); SysBusPCNetState *d = SYSBUS_PCNET(dev); PCNetState *s = &d->state; @@ -115,7 +115,6 @@ static int lance_init(SysBusDevice *sbd) s->phys_mem_read = ledma_memory_read; s->phys_mem_write = ledma_memory_write; pcnet_common_init(dev, s, &net_lance_info); - return 0; } static void lance_reset(DeviceState *dev) @@ -144,9 +143,8 @@ static Property lance_properties[] = { static void lance_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = lance_init; + dc->realize = lance_realize; set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); dc->fw_name = "ethernet"; dc->reset = lance_reset; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:14PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-5-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/lance.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/net/lance.c b/hw/net/lance.c index a08d5ac6a8..f987b2fd18 100644 --- a/hw/net/lance.c +++ b/hw/net/lance.c @@ -97,9 +97,9 @@ static const VMStateDescription vmstate_lance = { } };
-static int lance_init(SysBusDevice *sbd) +static void lance_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); SysBusPCNetState *d = SYSBUS_PCNET(dev); PCNetState *s = &d->state;
@@ -115,7 +115,6 @@ static int lance_init(SysBusDevice *sbd) s->phys_mem_read = ledma_memory_read; s->phys_mem_write = ledma_memory_write; pcnet_common_init(dev, s, &net_lance_info); - return 0; }
static void lance_reset(DeviceState *dev) @@ -144,9 +143,8 @@ static Property lance_properties[] = { static void lance_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = lance_init; + dc->realize = lance_realize; set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); dc->fw_name = "ethernet"; dc->reset = lance_reset;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Cc: Michael Walle <michael@walle.cc> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-6-clg@kaod.org> Acked-by: Michael Walle <michael@walle.cc> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/milkymist-minimac2.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/net/milkymist-minimac2.c b/hw/net/milkymist-minimac2.c index 3eaa19dfde..7ef1daee41 100644 --- a/hw/net/milkymist-minimac2.c +++ b/hw/net/milkymist-minimac2.c @@ -452,9 +452,9 @@ static NetClientInfo net_milkymist_minimac2_info = { .receive = minimac2_rx, }; -static int milkymist_minimac2_init(SysBusDevice *sbd) +static void milkymist_minimac2_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); MilkymistMinimac2State *s = MILKYMIST_MINIMAC2(dev); size_t buffers_size = TARGET_PAGE_ALIGN(3 * MINIMAC2_BUFFER_SIZE); @@ -479,8 +479,6 @@ static int milkymist_minimac2_init(SysBusDevice *sbd) s->nic = qemu_new_nic(&net_milkymist_minimac2_info, &s->conf, object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); - - return 0; } static const VMStateDescription vmstate_milkymist_minimac2_mdio = { @@ -521,9 +519,8 @@ static Property milkymist_minimac2_properties[] = { static void milkymist_minimac2_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = milkymist_minimac2_init; + dc->realize = milkymist_minimac2_realize; dc->reset = milkymist_minimac2_reset; dc->vmsd = &vmstate_milkymist_minimac2; dc->props = milkymist_minimac2_properties; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:15PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Cc: Michael Walle <michael@walle.cc> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-6-clg@kaod.org> Acked-by: Michael Walle <michael@walle.cc> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/milkymist-minimac2.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/net/milkymist-minimac2.c b/hw/net/milkymist-minimac2.c index 3eaa19dfde..7ef1daee41 100644 --- a/hw/net/milkymist-minimac2.c +++ b/hw/net/milkymist-minimac2.c @@ -452,9 +452,9 @@ static NetClientInfo net_milkymist_minimac2_info = { .receive = minimac2_rx, };
-static int milkymist_minimac2_init(SysBusDevice *sbd) +static void milkymist_minimac2_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); MilkymistMinimac2State *s = MILKYMIST_MINIMAC2(dev); size_t buffers_size = TARGET_PAGE_ALIGN(3 * MINIMAC2_BUFFER_SIZE);
@@ -479,8 +479,6 @@ static int milkymist_minimac2_init(SysBusDevice *sbd) s->nic = qemu_new_nic(&net_milkymist_minimac2_info, &s->conf, object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); - - return 0; }
static const VMStateDescription vmstate_milkymist_minimac2_mdio = { @@ -521,9 +519,8 @@ static Property milkymist_minimac2_properties[] = { static void milkymist_minimac2_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = milkymist_minimac2_init; + dc->realize = milkymist_minimac2_realize; dc->reset = milkymist_minimac2_reset; dc->vmsd = &vmstate_milkymist_minimac2; dc->props = milkymist_minimac2_properties;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Cc: Aleksandar Markovic <amarkovic@wavecomp.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-7-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/mipsnet.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c index 5a63df7ccb..03b3104278 100644 --- a/hw/net/mipsnet.c +++ b/hw/net/mipsnet.c @@ -236,9 +236,9 @@ static const MemoryRegionOps mipsnet_ioport_ops = { .impl.max_access_size = 4, }; -static int mipsnet_sysbus_init(SysBusDevice *sbd) +static void mipsnet_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); MIPSnetState *s = MIPS_NET(dev); memory_region_init_io(&s->io, OBJECT(dev), &mipsnet_ioport_ops, s, @@ -249,8 +249,6 @@ static int mipsnet_sysbus_init(SysBusDevice *sbd) s->nic = qemu_new_nic(&net_mipsnet_info, &s->conf, object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); - - return 0; } static void mipsnet_sysbus_reset(DeviceState *dev) @@ -267,9 +265,8 @@ static Property mipsnet_properties[] = { static void mipsnet_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = mipsnet_sysbus_init; + dc->realize = mipsnet_realize; set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); dc->desc = "MIPS Simulator network device"; dc->reset = mipsnet_sysbus_reset; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:16PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Cc: Aleksandar Markovic <amarkovic@wavecomp.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-7-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/mipsnet.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c index 5a63df7ccb..03b3104278 100644 --- a/hw/net/mipsnet.c +++ b/hw/net/mipsnet.c @@ -236,9 +236,9 @@ static const MemoryRegionOps mipsnet_ioport_ops = { .impl.max_access_size = 4, };
-static int mipsnet_sysbus_init(SysBusDevice *sbd) +static void mipsnet_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); MIPSnetState *s = MIPS_NET(dev);
memory_region_init_io(&s->io, OBJECT(dev), &mipsnet_ioport_ops, s, @@ -249,8 +249,6 @@ static int mipsnet_sysbus_init(SysBusDevice *sbd) s->nic = qemu_new_nic(&net_mipsnet_info, &s->conf, object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); - - return 0; }
static void mipsnet_sysbus_reset(DeviceState *dev) @@ -267,9 +265,8 @@ static Property mipsnet_properties[] = { static void mipsnet_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = mipsnet_sysbus_init; + dc->realize = mipsnet_realize; set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); dc->desc = "MIPS Simulator network device"; dc->reset = mipsnet_sysbus_reset;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Cc: Max Filippov <jcmvbkbc@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-8-clg@kaod.org> Acked-by: Max Filippov <jcmvbkbc@gmail.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/opencores_eth.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c index d42b79c08c..d6f54f8d82 100644 --- a/hw/net/opencores_eth.c +++ b/hw/net/opencores_eth.c @@ -715,9 +715,9 @@ static const MemoryRegionOps open_eth_desc_ops = { .write = open_eth_desc_write, }; -static int sysbus_open_eth_init(SysBusDevice *sbd) +static void sysbus_open_eth_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); OpenEthState *s = OPEN_ETH(dev); memory_region_init_io(&s->reg_io, OBJECT(dev), &open_eth_reg_ops, s, @@ -732,7 +732,6 @@ static int sysbus_open_eth_init(SysBusDevice *sbd) s->nic = qemu_new_nic(&net_open_eth_info, &s->conf, object_get_typename(OBJECT(s)), dev->id, s); - return 0; } static void qdev_open_eth_reset(DeviceState *dev) @@ -750,9 +749,8 @@ static Property open_eth_properties[] = { static void open_eth_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = sysbus_open_eth_init; + dc->realize = sysbus_open_eth_realize; set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); dc->desc = "Opencores 10/100 Mbit Ethernet"; dc->reset = qdev_open_eth_reset; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:17PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Cc: Max Filippov <jcmvbkbc@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-8-clg@kaod.org> Acked-by: Max Filippov <jcmvbkbc@gmail.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/opencores_eth.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c index d42b79c08c..d6f54f8d82 100644 --- a/hw/net/opencores_eth.c +++ b/hw/net/opencores_eth.c @@ -715,9 +715,9 @@ static const MemoryRegionOps open_eth_desc_ops = { .write = open_eth_desc_write, };
-static int sysbus_open_eth_init(SysBusDevice *sbd) +static void sysbus_open_eth_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); OpenEthState *s = OPEN_ETH(dev);
memory_region_init_io(&s->reg_io, OBJECT(dev), &open_eth_reg_ops, s, @@ -732,7 +732,6 @@ static int sysbus_open_eth_init(SysBusDevice *sbd)
s->nic = qemu_new_nic(&net_open_eth_info, &s->conf, object_get_typename(OBJECT(s)), dev->id, s); - return 0; }
static void qdev_open_eth_reset(DeviceState *dev) @@ -750,9 +749,8 @@ static Property open_eth_properties[] = { static void open_eth_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = sysbus_open_eth_init; + dc->realize = sysbus_open_eth_realize; set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); dc->desc = "Opencores 10/100 Mbit Ethernet"; dc->reset = qdev_open_eth_reset;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181001063803.22330-9-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/smc91c111.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c index d2fd2040e8..99da2d9297 100644 --- a/hw/net/smc91c111.c +++ b/hw/net/smc91c111.c @@ -766,9 +766,9 @@ static NetClientInfo net_smc91c111_info = { .receive = smc91c111_receive, }; -static int smc91c111_init1(SysBusDevice *sbd) +static void smc91c111_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); smc91c111_state *s = SMC91C111(dev); memory_region_init_io(&s->mmio, OBJECT(s), &smc91c111_mem_ops, s, @@ -780,7 +780,6 @@ static int smc91c111_init1(SysBusDevice *sbd) object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); /* ??? Save/restore. */ - return 0; } static Property smc91c111_properties[] = { @@ -791,9 +790,8 @@ static Property smc91c111_properties[] = { static void smc91c111_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = smc91c111_init1; + dc->realize = smc91c111_realize; dc->reset = smc91c111_reset; dc->vmsd = &vmstate_smc91c111; dc->props = smc91c111_properties; -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:18PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20181001063803.22330-9-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/smc91c111.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c index d2fd2040e8..99da2d9297 100644 --- a/hw/net/smc91c111.c +++ b/hw/net/smc91c111.c @@ -766,9 +766,9 @@ static NetClientInfo net_smc91c111_info = { .receive = smc91c111_receive, };
-static int smc91c111_init1(SysBusDevice *sbd) +static void smc91c111_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); smc91c111_state *s = SMC91C111(dev);
memory_region_init_io(&s->mmio, OBJECT(s), &smc91c111_mem_ops, s, @@ -780,7 +780,6 @@ static int smc91c111_init1(SysBusDevice *sbd) object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); /* ??? Save/restore. */ - return 0; }
static Property smc91c111_properties[] = { @@ -791,9 +790,8 @@ static Property smc91c111_properties[] = { static void smc91c111_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = smc91c111_init1; + dc->realize = smc91c111_realize; dc->reset = smc91c111_reset; dc->vmsd = &vmstate_smc91c111; dc->props = smc91c111_properties;
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-10-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/stellaris_enet.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c index 165562d788..3ee1e0f8a4 100644 --- a/hw/net/stellaris_enet.c +++ b/hw/net/stellaris_enet.c @@ -473,9 +473,9 @@ static NetClientInfo net_stellaris_enet_info = { .receive = stellaris_enet_receive, }; -static int stellaris_enet_init(SysBusDevice *sbd) +static void stellaris_enet_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); stellaris_enet_state *s = STELLARIS_ENET(dev); memory_region_init_io(&s->mmio, OBJECT(s), &stellaris_enet_ops, s, @@ -489,7 +489,6 @@ static int stellaris_enet_init(SysBusDevice *sbd) qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); stellaris_enet_reset(s); - return 0; } static Property stellaris_enet_properties[] = { @@ -500,9 +499,8 @@ static Property stellaris_enet_properties[] = { static void stellaris_enet_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); - k->init = stellaris_enet_init; + dc->realize = stellaris_enet_realize; dc->props = stellaris_enet_properties; dc->vmsd = &vmstate_stellaris_enet; } -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:19PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-10-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/stellaris_enet.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c index 165562d788..3ee1e0f8a4 100644 --- a/hw/net/stellaris_enet.c +++ b/hw/net/stellaris_enet.c @@ -473,9 +473,9 @@ static NetClientInfo net_stellaris_enet_info = { .receive = stellaris_enet_receive, };
-static int stellaris_enet_init(SysBusDevice *sbd) +static void stellaris_enet_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); stellaris_enet_state *s = STELLARIS_ENET(dev);
memory_region_init_io(&s->mmio, OBJECT(s), &stellaris_enet_ops, s, @@ -489,7 +489,6 @@ static int stellaris_enet_init(SysBusDevice *sbd) qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
stellaris_enet_reset(s); - return 0; }
static Property stellaris_enet_properties[] = { @@ -500,9 +499,8 @@ static Property stellaris_enet_properties[] = { static void stellaris_enet_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); - SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
- k->init = stellaris_enet_init; + dc->realize = stellaris_enet_realize; dc->props = stellaris_enet_properties; dc->vmsd = &vmstate_stellaris_enet; }
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Cédric Le Goater <clg@kaod.org> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-11-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/stellaris_enet.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c index 3ee1e0f8a4..b3375ebb45 100644 --- a/hw/net/stellaris_enet.c +++ b/hw/net/stellaris_enet.c @@ -457,8 +457,10 @@ static const MemoryRegionOps stellaris_enet_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static void stellaris_enet_reset(stellaris_enet_state *s) +static void stellaris_enet_reset(DeviceState *dev) { + stellaris_enet_state *s = STELLARIS_ENET(dev); + s->mdv = 0x80; s->rctl = SE_RCTL_BADCRC; s->im = SE_INT_PHY | SE_INT_MD | SE_INT_RXER | SE_INT_FOV | SE_INT_TXEMP @@ -487,8 +489,6 @@ static void stellaris_enet_realize(DeviceState *dev, Error **errp) s->nic = qemu_new_nic(&net_stellaris_enet_info, &s->conf, object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); - - stellaris_enet_reset(s); } static Property stellaris_enet_properties[] = { @@ -501,6 +501,7 @@ static void stellaris_enet_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc->realize = stellaris_enet_realize; + dc->reset = stellaris_enet_reset; dc->props = stellaris_enet_properties; dc->vmsd = &vmstate_stellaris_enet; } -- 2.18.0.rc1.1.g3f1ff2140
From: Cédric Le Goater <clg@kaod.org> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Cc: Rob Herring <robh@kernel.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-12-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/net/xgmac.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/net/xgmac.c b/hw/net/xgmac.c index fa001563d3..63f5a62ebf 100644 --- a/hw/net/xgmac.c +++ b/hw/net/xgmac.c @@ -374,9 +374,9 @@ static NetClientInfo net_xgmac_enet_info = { .receive = eth_rx, }; -static int xgmac_enet_init(SysBusDevice *sbd) +static void xgmac_enet_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); XgmacState *s = XGMAC(dev); memory_region_init_io(&s->iomem, OBJECT(s), &enet_mem_ops, s, @@ -397,8 +397,6 @@ static int xgmac_enet_init(SysBusDevice *sbd) (s->conf.macaddr.a[2] << 16) | (s->conf.macaddr.a[1] << 8) | s->conf.macaddr.a[0]; - - return 0; } static Property xgmac_properties[] = { @@ -408,10 +406,9 @@ static Property xgmac_properties[] = { static void xgmac_enet_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); - sbc->init = xgmac_enet_init; + dc->realize = xgmac_enet_realize; dc->vmsd = &vmstate_xgmac; dc->props = xgmac_properties; } -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:21PM -0300, Eduardo Habkost wrote:
From: Cédric Le Goater <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Cc: Rob Herring <robh@kernel.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20181001063803.22330-12-clg@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- hw/net/xgmac.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/net/xgmac.c b/hw/net/xgmac.c index fa001563d3..63f5a62ebf 100644 --- a/hw/net/xgmac.c +++ b/hw/net/xgmac.c @@ -374,9 +374,9 @@ static NetClientInfo net_xgmac_enet_info = { .receive = eth_rx, };
-static int xgmac_enet_init(SysBusDevice *sbd) +static void xgmac_enet_realize(DeviceState *dev, Error **errp) { - DeviceState *dev = DEVICE(sbd); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); XgmacState *s = XGMAC(dev);
memory_region_init_io(&s->iomem, OBJECT(s), &enet_mem_ops, s, @@ -397,8 +397,6 @@ static int xgmac_enet_init(SysBusDevice *sbd) (s->conf.macaddr.a[2] << 16) | (s->conf.macaddr.a[1] << 8) | s->conf.macaddr.a[0]; - - return 0; }
static Property xgmac_properties[] = { @@ -408,10 +406,9 @@ static Property xgmac_properties[] = {
static void xgmac_enet_class_init(ObjectClass *klass, void *data) { - SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass);
- sbc->init = xgmac_enet_init; + dc->realize = xgmac_enet_realize; dc->vmsd = &vmstate_xgmac; dc->props = xgmac_properties; }
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
From: Markus Armbruster <armbru@redhat.com> Calling error_report() in a function that takes an Error ** argument is suspicious. parse_numa() does that, and then fails without setting an error. Its caller main(), via qemu_opts_foreach(), is fine with it, but clean it up anyway. While there, give parse_numa() internal linkage. Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20181008173125.19678-26-armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- include/sysemu/numa.h | 1 - numa.c | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index 7a0ae751aa..21713b7e2f 100644 --- a/include/sysemu/numa.h +++ b/include/sysemu/numa.h @@ -22,7 +22,6 @@ struct NumaNodeMem { }; extern NodeInfo numa_info[MAX_NODES]; -int parse_numa(void *opaque, QemuOpts *opts, Error **errp); void parse_numa_opts(MachineState *ms); void numa_complete_configuration(MachineState *ms); void query_numa_node_mem(NumaNodeMem node_mem[]); diff --git a/numa.c b/numa.c index 1d7c49ad43..50ec016013 100644 --- a/numa.c +++ b/numa.c @@ -215,7 +215,7 @@ end: error_propagate(errp, err); } -int parse_numa(void *opaque, QemuOpts *opts, Error **errp) +static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) { NumaOptions *object = NULL; MachineState *ms = MACHINE(opaque); @@ -239,7 +239,7 @@ int parse_numa(void *opaque, QemuOpts *opts, Error **errp) end: qapi_free_NumaOptions(object); if (err) { - error_report_err(err); + error_propagate(errp, err); return -1; } @@ -444,9 +444,7 @@ void numa_complete_configuration(MachineState *ms) void parse_numa_opts(MachineState *ms) { - if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, NULL)) { - exit(1); - } + qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, &error_fatal); } void qmp_set_numa_node(NumaOptions *cmd, Error **errp) -- 2.18.0.rc1.1.g3f1ff2140
On Thu, Oct 18, 2018 at 05:04:22PM -0300, Eduardo Habkost wrote:
From: Markus Armbruster <armbru@redhat.com>
Calling error_report() in a function that takes an Error ** argument is suspicious. parse_numa() does that, and then fails without setting an error. Its caller main(), via qemu_opts_foreach(), is fine with it, but clean it up anyway.
While there, give parse_numa() internal linkage.
Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20181008173125.19678-26-armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
--- include/sysemu/numa.h | 1 - numa.c | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index 7a0ae751aa..21713b7e2f 100644 --- a/include/sysemu/numa.h +++ b/include/sysemu/numa.h @@ -22,7 +22,6 @@ struct NumaNodeMem { };
extern NodeInfo numa_info[MAX_NODES]; -int parse_numa(void *opaque, QemuOpts *opts, Error **errp); void parse_numa_opts(MachineState *ms); void numa_complete_configuration(MachineState *ms); void query_numa_node_mem(NumaNodeMem node_mem[]); diff --git a/numa.c b/numa.c index 1d7c49ad43..50ec016013 100644 --- a/numa.c +++ b/numa.c @@ -215,7 +215,7 @@ end: error_propagate(errp, err); }
-int parse_numa(void *opaque, QemuOpts *opts, Error **errp) +static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) { NumaOptions *object = NULL; MachineState *ms = MACHINE(opaque); @@ -239,7 +239,7 @@ int parse_numa(void *opaque, QemuOpts *opts, Error **errp) end: qapi_free_NumaOptions(object); if (err) { - error_report_err(err); + error_propagate(errp, err); return -1; }
@@ -444,9 +444,7 @@ void numa_complete_configuration(MachineState *ms)
void parse_numa_opts(MachineState *ms) { - if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, NULL)) { - exit(1); - } + qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, &error_fatal); }
void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid: On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets: TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-i440fx-3.0/device-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-q35-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-q35-3.0/device-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /arm/qom/n800: OK PASS: tests/cpu-plug-test [...] TEST: tests/cpu-plug-test... (pid=15040) /s390x/cpu-plug/s390-ccw-virtio-3.1/cpu-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.1/device-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.0/cpu-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.0/device-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK PASS: tests/cpu-plug-test (plus similar ppc64, x86_64 targets) I see similar warnings on hosts SPARC, PPC64BE, S390, 32-bit Arm and x86 FreeBSD. I also got a build failure on one of my configs, but I think that is caused by some latent bug in our makefiles where we don't correctly rebuild x86_64-softmmu/config-devices.mak when a change is made to default-configs/i386-softmmu.mak -- doing a hand rm of the config-devices.mak fixed it. thanks -- PMM
On 19/10/2018 16:12, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-i440fx-3.0/device-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-q35-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-q35-3.0/device-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /arm/qom/n800: OK PASS: tests/cpu-plug-test [...] TEST: tests/cpu-plug-test... (pid=15040) /s390x/cpu-plug/s390-ccw-virtio-3.1/cpu-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.1/device-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.0/cpu-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.0/device-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK PASS: tests/cpu-plug-test
(plus similar ppc64, x86_64 targets)
I see similar warnings on hosts SPARC, PPC64BE, S390, 32-bit Arm and x86 FreeBSD.
I also got a build failure on one of my configs, but I think that is caused by some latent bug in our makefiles where we don't correctly rebuild x86_64-softmmu/config-devices.mak when a change is made to default-configs/i386-softmmu.mak -- doing a hand rm of the config-devices.mak fixed it.
bisected to: 23d0571a24559b867fa47410aa8ec0519b0a1edd is the first bad commit commit 23d0571a24559b867fa47410aa8ec0519b0a1edd Author: Igor Mammedov <imammedo@redhat.com> Date: Thu Sep 13 13:06:01 2018 +0200 vl.c deprecate incorrect CPUs topology -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology so that total number of logical CPUs [sockets * cores * threads] would be equal to [maxcpus], however historically we didn't have such check in QEMU and it is possible to start VM with an invalid topology. Deprecate invalid options combination so we can make sure that the topology VM started with is always correct in the future. Users with an invalid sockets/cores/threads/maxcpus values should fix their CLI to make sure that [sockets * cores * threads] == [maxcpus]
thanks -- PMM
On Fri, Oct 19, 2018 at 7:00 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
On 19/10/2018 16:12, Peter Maydell wrote: ...
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-i440fx-3.0/device-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-q35-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /i386/cpu-plug/pc-q35-3.0/device-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) OK /arm/qom/n800: OK PASS: tests/cpu-plug-test [...] TEST: tests/cpu-plug-test... (pid=15040) /s390x/cpu-plug/s390-ccw-virtio-3.1/cpu-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.1/device-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.0/cpu-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK /s390x/cpu-plug/s390-ccw-virtio-3.0/device-add/1x3x1&maxcpus=6: qemu-system-s390x: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (1) != maxcpus (6) OK PASS: tests/cpu-plug-test
(plus similar ppc64, x86_64 targets)
I see similar warnings on hosts SPARC, PPC64BE, S390, 32-bit Arm and x86 FreeBSD.
I also got a build failure on one of my configs, but I think that is caused by some latent bug in our makefiles where we don't correctly rebuild x86_64-softmmu/config-devices.mak when a change is made to default-configs/i386-softmmu.mak -- doing a hand rm of the config-devices.mak fixed it.
bisected to:
23d0571a24559b867fa47410aa8ec0519b0a1edd is the first bad commit commit 23d0571a24559b867fa47410aa8ec0519b0a1edd Author: Igor Mammedov <imammedo@redhat.com> Date: Thu Sep 13 13:06:01 2018 +0200
vl.c deprecate incorrect CPUs topology
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology so that total number of logical CPUs [sockets * cores * threads] would be equal to [maxcpus], however historically we didn't have such check in QEMU and it is possible to start VM with an invalid topology. Deprecate invalid options combination so we can make sure that the topology VM started with is always correct in the future. Users with an invalid sockets/cores/threads/maxcpus values should fix their CLI to make sure that [sockets * cores * threads] == [maxcpus]
Per the commit message, I understand the tests need to be upgraded, they only test it is <= maxcpus while now we want == maxcpus: static void add_s390x_test_case(const char *mname) { ... data->sockets = 1; data->cores = 3; data->threads = 1; data->maxcpus = data->sockets * data->cores * data->threads * 2;
On Fri, Oct 19, 2018 at 03:12:31PM +0100, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) [...]
(plus similar ppc64, x86_64 targets)
Ouch. Apologies. Can we please do something make sure "make check" will fail on these cases? I'd like to be able to trust CI systems like travis-ci. -- Eduardo
On Fri, 19 Oct 2018 15:44:08 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 03:12:31PM +0100, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) [...]
(plus similar ppc64, x86_64 targets)
Ouch. Apologies.
Can we please do something make sure "make check" will fail on these cases? I'd like to be able to trust CI systems like travis-ci.
we probably don't want make check fail on warning. Test was written with assumption that s/c/t tuples matches initially present CPUs, hence a warning. Would something like following fix the issue (local x86 build/test looks fixed with it)? diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c index 3e93c8e..f4a677d 100644 --- a/tests/cpu-plug-test.c +++ b/tests/cpu-plug-test.c @@ -32,12 +32,12 @@ static void test_plug_with_cpu_add(gconstpointer data) unsigned int i; args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", s->machine, s->cpu_model, s->sockets, s->cores, s->threads, s->maxcpus); qtest_start(args); - for (i = s->sockets * s->cores * s->threads; i < s->maxcpus; i++) { + for (i = 1; i < s->maxcpus; i++) { response = qmp("{ 'execute': 'cpu-add'," " 'arguments': { 'id': %d } }", i); g_assert(response); @@ -56,7 +56,7 @@ static void test_plug_without_cpu_add(gconstpointer data) QDict *response; args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", s->machine, s->cpu_model, s->sockets, s->cores, s->threads, s->maxcpus); qtest_start(args); @@ -79,12 +79,12 @@ static void test_plug_with_device_add_x86(gconstpointer data) unsigned int s, c, t; args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", td->machine, td->cpu_model, td->sockets, td->cores, td->threads, td->maxcpus); qtest_start(args); - for (s = td->sockets; s < td->maxcpus / td->cores / td->threads; s++) { + for (s = 1; s < td->sockets; s++) { for (c = 0; c < td->cores; c++) { for (t = 0; t < td->threads; t++) { char *id = g_strdup_printf("id-%i-%i-%i", s, c, t); @@ -113,7 +113,7 @@ static void test_plug_with_device_add_coreid(gconstpointer data) td->sockets, td->cores, td->threads, td->maxcpus); qtest_start(args); - for (c = td->cores; c < td->maxcpus / td->sockets / td->threads; c++) { + for (c = 1; c < td->cores; c++) { char *id = g_strdup_printf("id-%i", c); qtest_qmp_device_add(td->device_model, id, "{'core-id':%u}", c); g_free(id); @@ -148,7 +148,7 @@ static void add_pc_test_case(const char *mname) data->sockets = 1; data->cores = 3; data->threads = 2; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads; if (g_str_has_suffix(mname, "-1.4") || (strcmp(mname, "pc-1.3") == 0) || (strcmp(mname, "pc-1.2") == 0) || @@ -203,7 +203,7 @@ static void add_pseries_test_case(const char *mname) data->sockets = 2; data->cores = 3; data->threads = 1; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads; path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u", mname, data->sockets, data->cores, @@ -229,7 +229,7 @@ static void add_s390x_test_case(const char *mname) data->sockets = 1; data->cores = 3; data->threads = 1; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads; data2 = g_memdup(data, sizeof(PlugTestData)); data2->machine = g_strdup(data->machine);
On Fri, Oct 19, 2018 at 09:53:45PM +0200, Igor Mammedov wrote:
On Fri, 19 Oct 2018 15:44:08 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 03:12:31PM +0100, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) [...]
(plus similar ppc64, x86_64 targets)
Ouch. Apologies.
Can we please do something make sure "make check" will fail on these cases? I'd like to be able to trust CI systems like travis-ci.
we probably don't want make check fail on warning.
I disagree. If a warning is blocking a pull request from being merged, it must make CI systems fail too. Otherwise we're defeating the purpose of CI systems.
Test was written with assumption that s/c/t tuples matches initially present CPUs, hence a warning. Would something like following fix the issue (local x86 build/test looks fixed with it)?
It works for me. I will queue it on machine-next, thanks!
diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c index 3e93c8e..f4a677d 100644 --- a/tests/cpu-plug-test.c +++ b/tests/cpu-plug-test.c @@ -32,12 +32,12 @@ static void test_plug_with_cpu_add(gconstpointer data) unsigned int i;
args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", s->machine, s->cpu_model, s->sockets, s->cores, s->threads, s->maxcpus); qtest_start(args);
- for (i = s->sockets * s->cores * s->threads; i < s->maxcpus; i++) { + for (i = 1; i < s->maxcpus; i++) { response = qmp("{ 'execute': 'cpu-add'," " 'arguments': { 'id': %d } }", i); g_assert(response); @@ -56,7 +56,7 @@ static void test_plug_without_cpu_add(gconstpointer data) QDict *response;
args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", s->machine, s->cpu_model, s->sockets, s->cores, s->threads, s->maxcpus); qtest_start(args); @@ -79,12 +79,12 @@ static void test_plug_with_device_add_x86(gconstpointer data) unsigned int s, c, t;
args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", td->machine, td->cpu_model, td->sockets, td->cores, td->threads, td->maxcpus); qtest_start(args);
- for (s = td->sockets; s < td->maxcpus / td->cores / td->threads; s++) { + for (s = 1; s < td->sockets; s++) { for (c = 0; c < td->cores; c++) { for (t = 0; t < td->threads; t++) { char *id = g_strdup_printf("id-%i-%i-%i", s, c, t); @@ -113,7 +113,7 @@ static void test_plug_with_device_add_coreid(gconstpointer data) td->sockets, td->cores, td->threads, td->maxcpus); qtest_start(args);
- for (c = td->cores; c < td->maxcpus / td->sockets / td->threads; c++) { + for (c = 1; c < td->cores; c++) { char *id = g_strdup_printf("id-%i", c); qtest_qmp_device_add(td->device_model, id, "{'core-id':%u}", c); g_free(id); @@ -148,7 +148,7 @@ static void add_pc_test_case(const char *mname) data->sockets = 1; data->cores = 3; data->threads = 2; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads; if (g_str_has_suffix(mname, "-1.4") || (strcmp(mname, "pc-1.3") == 0) || (strcmp(mname, "pc-1.2") == 0) || @@ -203,7 +203,7 @@ static void add_pseries_test_case(const char *mname) data->sockets = 2; data->cores = 3; data->threads = 1; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads;
path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u", mname, data->sockets, data->cores, @@ -229,7 +229,7 @@ static void add_s390x_test_case(const char *mname) data->sockets = 1; data->cores = 3; data->threads = 1; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads;
data2 = g_memdup(data, sizeof(PlugTestData)); data2->machine = g_strdup(data->machine);
-- Eduardo
On 19/10/2018 22:23, Eduardo Habkost wrote:
On Fri, Oct 19, 2018 at 09:53:45PM +0200, Igor Mammedov wrote:
On Fri, 19 Oct 2018 15:44:08 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 03:12:31PM +0100, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) [...]
(plus similar ppc64, x86_64 targets)
Ouch. Apologies.
Can we please do something make sure "make check" will fail on these cases? I'd like to be able to trust CI systems like travis-ci.
we probably don't want make check fail on warning.
I disagree. If a warning is blocking a pull request from being merged, it must make CI systems fail too. Otherwise we're defeating the purpose of CI systems.
We can, we also have the hardware (courtesy of Packet), we just need to make time to set it up. QEMU Summit is a good place where to talk about this.
Test was written with assumption that s/c/t tuples matches initially present CPUs, hence a warning. Would something like following fix the issue (local x86 build/test looks fixed with it)?
It works for me. I will queue it on machine-next, thanks!
diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c index 3e93c8e..f4a677d 100644 --- a/tests/cpu-plug-test.c +++ b/tests/cpu-plug-test.c @@ -32,12 +32,12 @@ static void test_plug_with_cpu_add(gconstpointer data) unsigned int i;
args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", s->machine, s->cpu_model, s->sockets, s->cores, s->threads, s->maxcpus); qtest_start(args);
- for (i = s->sockets * s->cores * s->threads; i < s->maxcpus; i++) { + for (i = 1; i < s->maxcpus; i++) { response = qmp("{ 'execute': 'cpu-add'," " 'arguments': { 'id': %d } }", i); g_assert(response); @@ -56,7 +56,7 @@ static void test_plug_without_cpu_add(gconstpointer data) QDict *response;
args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", s->machine, s->cpu_model, s->sockets, s->cores, s->threads, s->maxcpus); qtest_start(args); @@ -79,12 +79,12 @@ static void test_plug_with_device_add_x86(gconstpointer data) unsigned int s, c, t;
args = g_strdup_printf("-machine %s -cpu %s " - "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u", + "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u", td->machine, td->cpu_model, td->sockets, td->cores, td->threads, td->maxcpus); qtest_start(args);
- for (s = td->sockets; s < td->maxcpus / td->cores / td->threads; s++) { + for (s = 1; s < td->sockets; s++) { for (c = 0; c < td->cores; c++) { for (t = 0; t < td->threads; t++) { char *id = g_strdup_printf("id-%i-%i-%i", s, c, t); @@ -113,7 +113,7 @@ static void test_plug_with_device_add_coreid(gconstpointer data) td->sockets, td->cores, td->threads, td->maxcpus); qtest_start(args);
- for (c = td->cores; c < td->maxcpus / td->sockets / td->threads; c++) { + for (c = 1; c < td->cores; c++) { char *id = g_strdup_printf("id-%i", c); qtest_qmp_device_add(td->device_model, id, "{'core-id':%u}", c); g_free(id); @@ -148,7 +148,7 @@ static void add_pc_test_case(const char *mname) data->sockets = 1; data->cores = 3; data->threads = 2; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads; if (g_str_has_suffix(mname, "-1.4") || (strcmp(mname, "pc-1.3") == 0) || (strcmp(mname, "pc-1.2") == 0) || @@ -203,7 +203,7 @@ static void add_pseries_test_case(const char *mname) data->sockets = 2; data->cores = 3; data->threads = 1; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads;
path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u", mname, data->sockets, data->cores, @@ -229,7 +229,7 @@ static void add_s390x_test_case(const char *mname) data->sockets = 1; data->cores = 3; data->threads = 1; - data->maxcpus = data->sockets * data->cores * data->threads * 2; + data->maxcpus = data->sockets * data->cores * data->threads;
data2 = g_memdup(data, sizeof(PlugTestData)); data2->machine = g_strdup(data->machine);
This patch with Igor Signed-off-by: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Fri, 19 Oct 2018 17:23:21 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 09:53:45PM +0200, Igor Mammedov wrote:
On Fri, 19 Oct 2018 15:44:08 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 03:12:31PM +0100, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) [...]
(plus similar ppc64, x86_64 targets)
Ouch. Apologies.
Can we please do something make sure "make check" will fail on these cases? I'd like to be able to trust CI systems like travis-ci.
we probably don't want make check fail on warning.
I disagree. If a warning is blocking a pull request from being merged, it must make CI systems fail too. Otherwise we're defeating the purpose of CI systems.
When we deprecate options we are bound to trigger warning which are not errors and are meant to be there until deprecated options are removed/tested by make check. So what would you suggest to do wrt tests that use deprecated features, drop testing for it? [...]
Igor Mammedov <imammedo@redhat.com> writes:
On Fri, 19 Oct 2018 17:23:21 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 09:53:45PM +0200, Igor Mammedov wrote:
On Fri, 19 Oct 2018 15:44:08 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote:
On Fri, Oct 19, 2018 at 03:12:31PM +0100, Peter Maydell wrote:
On 18 October 2018 at 21:03, Eduardo Habkost <ehabkost@redhat.com> wrote:
The following changes since commit 09558375a634e17cea6cfbfec883ac2376d2dc7f:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181016-1' into staging (2018-10-16 17:42:56 +0100)
are available in the Git repository at:
git://github.com/ehabkost/qemu.git tags/machine-next-pull-request
for you to fetch changes up to 6d8e1bcc7dd5e819ce81e6a87fffe23e39c700cc:
numa: Clean up error reporting in parse_numa() (2018-10-17 16:33:40 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-18
* sysbus init/realize cleanups (Cédric Le Goater, Philippe Mathieu-Daudé) * memory-device refactoring (David Hildenbrand) * -smp: deprecate incorrect CPUs topology (Igor Mammedov) * -numa parsing cleanups (Markus Armbruster) * Fix hostmem-file memory leak (Zhang Yi) * Typo fix (Li Qiang)
----------------------------------------------------------------
Hi. This had some problems in merge testing, I'm afraid:
On aarch64 host, warnings running tests/cpu-plug-test for i386 and s390 targets:
TEST: tests/cpu-plug-test... (pid=12602) /i386/cpu-plug/pc-i440fx-3.0/cpu-add/1x3x2&maxcpus=12: qemu-system-i386: warning: Invalid CPU topology deprecated: sockets (1) * cores (3) * threads (2) != maxcpus (12) [...]
(plus similar ppc64, x86_64 targets)
Ouch. Apologies.
Can we please do something make sure "make check" will fail on these cases? I'd like to be able to trust CI systems like travis-ci.
we probably don't want make check fail on warning.
I disagree. If a warning is blocking a pull request from being merged, it must make CI systems fail too. Otherwise we're defeating the purpose of CI systems.
When we deprecate options we are bound to trigger warning which are not errors and are meant to be there until deprecated options are removed/tested by make check. So what would you suggest to do wrt tests that use deprecated features, drop testing for it?
We commonly suppress the warning when testing. Search for 'if (qtest_enabled())'.
participants (6)
-
David Gibson -
Eduardo Habkost -
Igor Mammedov -
Markus Armbruster -
Peter Maydell -
Philippe Mathieu-Daudé