From 6f4e1405b91da0d0a1084ae3aff2bd308432778f Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 20 Jul 2020 10:25:36 +0100 Subject: hw/arm/virt: Enable MTE via a machine property Control this cpu feature via a machine property, much as we do with secure=on, since both require specialized support in the machine setup to be functional. Default MTE to off, since this feature implies extra overhead. Signed-off-by: Richard Henderson Message-id: 20200713213341.590275-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 9005dae356..5866c4ce20 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1837,12 +1837,19 @@ static void machvirt_init(MachineState *machine) OBJECT(secure_sysmem), &error_abort); } - /* - * The cpu adds the property if and only if MemTag is supported. - * If it is, we must allocate the ram to back that up. - */ - if (object_property_find(cpuobj, "tag-memory", NULL)) { + if (vms->mte) { + /* Create the memory region only once, but link to all cpus. */ if (!tag_sysmem) { + /* + * The property exists only if MemTag is supported. + * If it is, we must allocate the ram to back that up. + */ + if (!object_property_find(cpuobj, "tag-memory", NULL)) { + error_report("MTE requested, but not supported " + "by the guest CPU"); + exit(1); + } + tag_sysmem = g_new(MemoryRegion, 1); memory_region_init(tag_sysmem, OBJECT(machine), "tag-memory", UINT64_MAX / 32); @@ -2061,6 +2068,20 @@ static void virt_set_ras(Object *obj, bool value, Error **errp) vms->ras = value; } +static bool virt_get_mte(Object *obj, Error **errp) +{ + VirtMachineState *vms = VIRT_MACHINE(obj); + + return vms->mte; +} + +static void virt_set_mte(Object *obj, bool value, Error **errp) +{ + VirtMachineState *vms = VIRT_MACHINE(obj); + + vms->mte = value; +} + static char *virt_get_gic_version(Object *obj, Error **errp) { VirtMachineState *vms = VIRT_MACHINE(obj); @@ -2481,6 +2502,14 @@ static void virt_instance_init(Object *obj) "Set on/off to enable/disable reporting host memory errors " "to a KVM guest using ACPI and guest external abort exceptions"); + /* MTE is disabled by default. */ + vms->mte = false; + object_property_add_bool(obj, "mte", virt_get_mte, virt_set_mte); + object_property_set_description(obj, "mte", + "Set on/off to enable/disable emulating a " + "guest CPU which implements the ARM " + "Memory Tagging Extension"); + vms->irqmap = a15irqmap; virt_flash_create(vms); -- cgit v1.2.3 From 7f6185ed9ca977a9836f35b994e4f85cecae8437 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 20 Jul 2020 10:25:36 +0100 Subject: hw/arm/virt: Error for MTE enabled with KVM While we expect KVM to support MTE at some future point, it certainly won't be ready in time for qemu 5.1. Signed-off-by: Richard Henderson Message-id: 20200713213341.590275-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'hw') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 5866c4ce20..a7f3d442db 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1773,6 +1773,12 @@ static void machvirt_init(MachineState *machine) exit(1); } + if (vms->mte && kvm_enabled()) { + error_report("mach-virt: KVM does not support providing " + "MTE to the guest CPU"); + exit(1); + } + create_fdt(vms); possible_cpus = mc->possible_cpu_arch_ids(machine); -- cgit v1.2.3 From 19bd6aafbd184be963d2b7c24874e3252a7088b7 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 20 Jul 2020 10:25:36 +0100 Subject: hw/arm/virt: Disable memory hotplug when MTE is enabled When MTE is enabled, tag memory must exist for all RAM. It might be possible to simultaneously hot plug tag memory alongside the corresponding normal memory, but for now just disable hotplug. Signed-off-by: Richard Henderson Message-id: 20200713213341.590275-4-richard.henderson@linaro.org Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'hw') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index a7f3d442db..ecfee362a1 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2194,6 +2194,11 @@ static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } + if (vms->mte) { + error_setg(errp, "memory hotplug is not enabled: MTE is enabled"); + return; + } + if (is_nvdimm && !ms->nvdimms_state->is_enabled) { error_setg(errp, "nvdimm is not enabled: add 'nvdimm=on' to '-M'"); return; -- cgit v1.2.3 From b51238e251cefb496b60d3db992af175fb354a68 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Sat, 11 Jul 2020 15:24:23 +0100 Subject: qdev: Move doc comments from qdev.c to qdev-core.h The doc-comments which document the qdev API are split between the header file and the C source files, because as a project we haven't been consistent about where we put them. Move all the doc-comments in qdev.c to the header files, so that users of the APIs don't have to look at the implementation files for this information. In the process, unify them into our doc-comment format and expand on them in some cases to clarify expected use cases. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20200711142425.16283-2-peter.maydell@linaro.org --- hw/core/qdev.c | 33 --------------------------------- 1 file changed, 33 deletions(-) (limited to 'hw') diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 01796823b4..96772a15bd 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -128,13 +128,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) } } -/* - * Create a device on the heap. - * A type @name must exist. - * This only initializes the device state structure and allows - * properties to be set. The device still needs to be realized. See - * qdev-core.h. - */ DeviceState *qdev_new(const char *name) { if (!object_class_by_name(name)) { @@ -143,11 +136,6 @@ DeviceState *qdev_new(const char *name) return DEVICE(object_new(name)); } -/* - * Try to create a device on the heap. - * This is like qdev_new(), except it returns %NULL when type @name - * does not exist. - */ DeviceState *qdev_try_new(const char *name) { if (!module_object_class_by_name(name)) { @@ -378,14 +366,6 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, qdev_unrealize(dev); } -/* - * Realize @dev. - * @dev must not be plugged into a bus. - * If @bus, plug @dev into @bus. This takes a reference to @dev. - * If @dev has no QOM parent, make one up, taking another reference. - * On success, return true. - * On failure, store an error through @errp and return false. - */ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp) { assert(!dev->realized && !dev->parent_bus); @@ -399,16 +379,6 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp) return object_property_set_bool(OBJECT(dev), "realized", true, errp); } -/* - * Realize @dev and drop a reference. - * This is like qdev_realize(), except the caller must hold a - * (private) reference, which is dropped on return regardless of - * success or failure. Intended use: - * dev = qdev_new(); - * [...] - * qdev_realize_and_unref(dev, bus, errp); - * Now @dev can go away without further ado. - */ bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp) { bool ret; @@ -814,9 +784,6 @@ static void qdev_class_add_property(DeviceClass *klass, Property *prop) prop->info->description); } -/* @qdev_alias_all_properties - Add alias properties to the source object for - * all qdev properties on the target DeviceState. - */ void qdev_alias_all_properties(DeviceState *target, Object *source) { ObjectClass *class; -- cgit v1.2.3 From 3f410039b79c0468e18142c6ddfede6f6f7b0427 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 13 Jul 2020 15:37:16 +0100 Subject: hw/arm/armsse: Assert info->num_cpus is in-bounds in armsse_realize() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In armsse_realize() we have a loop over [0, info->num_cpus), which indexes into various fixed-size arrays in the ARMSSE struct. This confuses Coverity, which warns that we might overrun those arrays (CID 1430326, 1430337, 1430371, 1430414, 1430430). This can't actually happen, because the info struct is always one of the entries in the armsse_variants[] array and num_cpus is either 1 or 2; we also already assert in armsse_init() that num_cpus is not too large. However, adding an assert to armsse_realize() like the one in armsse_init() should help Coverity figure out that these code paths aren't possible. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20200713143716.9881-1-peter.maydell@linaro.org --- hw/arm/armsse.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'hw') diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index 64fcab895f..dcbff9bd8f 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -452,6 +452,8 @@ static void armsse_realize(DeviceState *dev, Error **errp) return; } + assert(info->num_cpus <= SSE_MAX_CPUS); + /* max SRAM_ADDR_WIDTH: 24 - log2(SRAM_NUM_BANK) */ assert(is_power_of_2(info->sram_banks)); addr_width_max = 24 - ctz32(info->sram_banks); -- cgit v1.2.3