[libvirt] [PULL v2 00/43] Machine queue, 2018-10-25
Changes v1 -> v2: * Fix 'make check' warnings (Igor) The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3: Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +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 8fa922c241e63f018f5b55c03ac494ae3d5fe594: net: xgmac: convert SysBus init method to a realize method (2018-10-24 06:44:59 -0300) ---------------------------------------------------------------- Machine queue, 2018-10-25 * 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 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 +-- 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 +++---- tests/cpu-plug-test.c | 18 ++--- 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 ++++ 38 files changed, 344 insertions(+), 286 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
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> [ehabkost: squashed unit test fix] Message-Id: <20181019215345.521d58d7@igors-macbook-pro.local> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- tests/cpu-plug-test.c | 18 +++++++++--------- vl.c | 7 +++++++ qemu-deprecated.texi | 12 ++++++++++++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c index 3e93c8e096..f4a677d238 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); diff --git a/vl.c b/vl.c index b2a405f80f..31febe965c 100644 --- a/vl.c +++ b/vl.c @@ -1266,6 +1266,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
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 31febe965c..1fcacc5caa 100644 --- a/vl.c +++ b/vl.c @@ -1230,11 +1230,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
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
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 25, 2018 at 10:32:23AM -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);
Uh.. as in v1, it looks like this should be trace_sun4v_rtc_write().
}
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
On Thu, Oct 25, 2018 at 06:17:59PM +0100, David Gibson wrote:
On Thu, Oct 25, 2018 at 10:32:23AM -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);
Uh.. as in v1, it looks like this should be trace_sun4v_rtc_write().
Oops, my bad. I can fix this manually in case there's a v3 pull request, but I would really prefer that to be included in a follow up patch instead of making this block the entire pull request. -- Eduardo
On 26/10/18 10:27, Eduardo Habkost wrote:
On Thu, Oct 25, 2018 at 06:17:59PM +0100, David Gibson wrote:
On Thu, Oct 25, 2018 at 10:32:23AM -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);
Uh.. as in v1, it looks like this should be trace_sun4v_rtc_write().
Oops, my bad. I can fix this manually in case there's a v3 pull request, but I would really prefer that to be included in a follow up patch instead of making this block the entire pull request.
Oops sorry... I noticed you fixed that in 605be3a8c0, thanks! Regards, Phil.
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
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
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
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
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
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
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
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
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
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
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 eab8572f2a..f095725dba 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 210cff2781..64c998c4c8 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -51,7 +51,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 c85c6c8ca3..6c1c5c0a37 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 30722ccf98..39d882af6f 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -34,7 +34,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
On 25 October 2018 at 14:32, Eduardo Habkost <ehabkost@redhat.com> wrote:
Changes v1 -> v2: * Fix 'make check' warnings (Igor)
The following changes since commit 13399aad4fa87b2878c49d02a5d3bafa6c966ba3:
Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-10-22' into staging (2018-10-23 17:20:23 +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 8fa922c241e63f018f5b55c03ac494ae3d5fe594:
net: xgmac: convert SysBus init method to a realize method (2018-10-24 06:44:59 -0300)
---------------------------------------------------------------- Machine queue, 2018-10-25
* 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
Applied, thanks. -- PMM
participants (4)
-
David Gibson -
Eduardo Habkost -
Peter Maydell -
Philippe Mathieu-Daudé