From eaf1ffbe15fe67612c63d928415ee04eb4836dc7 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 12 Oct 2020 13:26:39 +0200 Subject: spapr: Clarify why DR connectors aren't user creatable DR connector is a device that emulates a firmware abstraction used by PAPR compliant guests to manage hotplug/dynamic-reconfiguration of PHBs, PCI devices, memory, and CPUs. It is internally created by the spapr platform and requires to be owned by either the machine (PHBs, CPUs, memory) or by a PHB (PCI devices). Signed-off-by: Greg Kurz Message-Id: <160250199940.765467.6896806997161856576.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr_drc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 697b28c343..77718cde1f 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -586,7 +586,8 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) dk->realize = realize; dk->unrealize = unrealize; /* - * Reason: it crashes FIXME find and document the real reason + * Reason: DR connector needs to be wired to either the machine or to a + * PHB in spapr_dr_connector_new(). */ dk->user_creatable = false; } -- cgit v1.2.3 From dff669d6a15fb92b063cb5aa691b4bb498727404 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Thu, 15 Oct 2020 23:03:18 +0200 Subject: ppc/spapr: re-assert IRQs during event-scan if there are pending If we hotplug a CPU during the first second of the kernel boot, the IRQ can be sent to the kernel while the RTAS event handler is not installed. The event is queued, but the kernel doesn't collect it and ignores the new CPU. As the code relies on edge-triggered IRQ, we can re-assert it during the event-scan RTAS call if there are still pending events (as it is already done in check-exception). Signed-off-by: Laurent Vivier Message-Id: <20201015210318.117386-1-lvivier@redhat.com> Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- hw/ppc/spapr_events.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index 1069d0197b..1add53547e 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -1000,10 +1000,22 @@ static void event_scan(PowerPCCPU *cpu, SpaprMachineState *spapr, target_ulong args, uint32_t nret, target_ulong rets) { + int i; if (nargs != 4 || nret != 1) { rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); return; } + + for (i = 0; i < EVENT_CLASS_MAX; i++) { + if (rtas_event_log_contains(EVENT_CLASS_MASK(i))) { + const SpaprEventSource *source = + spapr_event_sources_get_source(spapr->event_sources, i); + + g_assert(source->enabled); + qemu_irq_pulse(spapr_qirq(spapr, source->irq)); + } + } + rtas_st(rets, 0, RTAS_OUT_NO_ERRORS_FOUND); } -- cgit v1.2.3 From 2d154d2694009f9294e34875059e3a650ee5110a Mon Sep 17 00:00:00 2001 From: Elena Afanasova Date: Fri, 9 Oct 2020 06:41:36 -0700 Subject: hw/net: move allocation to the heap due to very large stack frame [dwg] The stack frame itself probably isn't that big a deal, but avoiding alloca() is generally recommended these days. Signed-off-by: Elena Afanasova Message-Id: <8f07132478469b35fb50a4706691e2b56b10a67b.camel@gmail.com> Signed-off-by: David Gibson --- hw/net/spapr_llan.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c index 2093f1bad0..581320a0e7 100644 --- a/hw/net/spapr_llan.c +++ b/hw/net/spapr_llan.c @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu, SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg); SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev); unsigned total_len; - uint8_t *lbuf, *p; + uint8_t *p; + g_autofree uint8_t *lbuf = NULL; int i, nbufs; int ret; @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu, return H_RESOURCE; } - lbuf = alloca(total_len); + lbuf = g_malloc(total_len); p = lbuf; for (i = 0; i < nbufs; i++) { ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]), -- cgit v1.2.3 From ce316b5118c732c5fef23d7763b8c01054bfcdfa Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 12 Oct 2020 12:15:21 +0200 Subject: spapr: Move spapr_create_nvdimm_dr_connectors() to core machine code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spapr_create_nvdimm_dr_connectors() function doesn't need to access any internal details of the sPAPR NVDIMM implementation. Also, pretty much like for the LMBs, only spapr_machine_init() is responsible for the creation of DR connectors for NVDIMMs. Make this clear by making this function static in hw/ppc/spapr.c. Signed-off-by: Greg Kurz Message-Id: <160249772183.757627.7396780936543977766.stgit@bahia.lan> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/spapr.c | 10 ++++++++++ hw/ppc/spapr_nvdimm.c | 11 ----------- include/hw/ppc/spapr_nvdimm.h | 1 - 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 63315f2d0f..ee716a12af 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2641,6 +2641,16 @@ static hwaddr spapr_rma_size(SpaprMachineState *spapr, Error **errp) return rma_size; } +static void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr) +{ + MachineState *machine = MACHINE(spapr); + int i; + + for (i = 0; i < machine->ram_slots; i++) { + spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_PMEM, i); + } +} + /* pSeries LPAR / sPAPR hardware init */ static void spapr_machine_init(MachineState *machine) { diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c index b3a489e9fe..9e3d94071f 100644 --- a/hw/ppc/spapr_nvdimm.c +++ b/hw/ppc/spapr_nvdimm.c @@ -106,17 +106,6 @@ void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp) } } -void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr) -{ - MachineState *machine = MACHINE(spapr); - int i; - - for (i = 0; i < machine->ram_slots; i++) { - spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_PMEM, i); - } -} - - static int spapr_dt_nvdimm(SpaprMachineState *spapr, void *fdt, int parent_offset, NVDIMMDevice *nvdimm) { diff --git a/include/hw/ppc/spapr_nvdimm.h b/include/hw/ppc/spapr_nvdimm.h index b834d82f55..490b19a009 100644 --- a/include/hw/ppc/spapr_nvdimm.h +++ b/include/hw/ppc/spapr_nvdimm.h @@ -31,6 +31,5 @@ void spapr_dt_persistent_memory(SpaprMachineState *spapr, void *fdt); bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm, uint64_t size, Error **errp); void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp); -void spapr_create_nvdimm_dr_connectors(SpaprMachineState *spapr); #endif -- cgit v1.2.3 From 90689a32ce2b84580646956c2417343943e5df37 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 15 Oct 2020 23:18:25 +0200 Subject: spapr: Fix leak of CPU machine specific data When a CPU core is being removed, the machine specific data of each CPU thread object is leaked. Fix this by calling the dedicated helper we have for that instead of simply unparenting the CPU object. Call it from a separate loop in spapr_cpu_core_unrealize() for symmetry with spapr_cpu_core_realize(). Signed-off-by: Greg Kurz Message-Id: <160279670540.1808373.17319746576919615623.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index b03620823a..c552112145 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -188,7 +188,6 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) } spapr_irq_cpu_intc_destroy(SPAPR_MACHINE(qdev_get_machine()), cpu); cpu_remove_sync(CPU(cpu)); - object_unparent(OBJECT(cpu)); } /* @@ -213,6 +212,15 @@ static void spapr_cpu_core_reset_handler(void *opaque) spapr_cpu_core_reset(opaque); } +static void spapr_delete_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) +{ + SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); + + cpu->machine_data = NULL; + g_free(spapr_cpu); + object_unparent(OBJECT(cpu)); +} + static void spapr_cpu_core_unrealize(DeviceState *dev) { SpaprCpuCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); @@ -224,6 +232,9 @@ static void spapr_cpu_core_unrealize(DeviceState *dev) for (i = 0; i < cc->nr_threads; i++) { spapr_unrealize_vcpu(sc->threads[i], sc); } + for (i = 0; i < cc->nr_threads; i++) { + spapr_delete_vcpu(sc->threads[i], sc); + } g_free(sc->threads); } @@ -294,15 +305,6 @@ err: return NULL; } -static void spapr_delete_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) -{ - SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); - - cpu->machine_data = NULL; - g_free(spapr_cpu); - object_unparent(OBJECT(cpu)); -} - static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) { /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user -- cgit v1.2.3 From f1023d21e81b7bf523ddf2ac91a48117f20ef9d7 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 15 Oct 2020 23:18:32 +0200 Subject: spapr: Unrealize vCPUs with qdev_unrealize() Since we introduced CPU hot-unplug in sPAPR, we don't unrealize the vCPU objects explicitly. Instead, we let QOM handle that for us under object_property_del_all() when the CPU core object is finalized. The only thing we do is calling cpu_remove_sync() to tear the vCPU thread down. This happens to work but it is ugly because: - we call qdev_realize() but the corresponding qdev_unrealize() is buried deep in the QOM code - we call cpu_remove_sync() to undo qemu_init_vcpu() called by ppc_cpu_realize() in target/ppc/translate_init.c.inc - the CPU init and teardown paths aren't really symmetrical The latter didn't bite us so far but a future patch that greatly simplifies the CPU core realize path needs it to avoid a crash in QOM. For all these reasons, have ppc_cpu_unrealize() to undo the changes of ppc_cpu_realize() by calling cpu_remove_sync() at the right place, and have the sPAPR CPU core code to call qdev_unrealize(). This requires to add a missing stub because translate_init.c.inc is also compiled for user mode. Signed-off-by: Greg Kurz Message-Id: <160279671236.1808373.14732005038172874990.stgit@bahia.lan> Signed-off-by: David Gibson --- accel/tcg/user-exec-stub.c | 4 ++++ hw/ppc/spapr_cpu_core.c | 4 ++-- target/ppc/translate_init.c.inc | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c index f6d8c8fb6f..b876f5c1e4 100644 --- a/accel/tcg/user-exec-stub.c +++ b/accel/tcg/user-exec-stub.c @@ -9,6 +9,10 @@ void cpu_resume(CPUState *cpu) { } +void cpu_remove_sync(CPUState *cpu) +{ +} + void qemu_init_vcpu(CPUState *cpu) { } diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index c552112145..e4aeb93c02 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -187,7 +187,7 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data); } spapr_irq_cpu_intc_destroy(SPAPR_MACHINE(qdev_get_machine()), cpu); - cpu_remove_sync(CPU(cpu)); + qdev_unrealize(DEVICE(cpu)); } /* @@ -255,7 +255,7 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr, kvmppc_set_papr(cpu); if (spapr_irq_cpu_intc_create(spapr, cpu, errp) < 0) { - cpu_remove_sync(CPU(cpu)); + qdev_unrealize(DEVICE(cpu)); return false; } diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc index bb66526280..d2a8204d60 100644 --- a/target/ppc/translate_init.c.inc +++ b/target/ppc/translate_init.c.inc @@ -10328,6 +10328,8 @@ static void ppc_cpu_unrealize(DeviceState *dev) pcc->parent_unrealize(dev); + cpu_remove_sync(CPU(cpu)); + for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) { if (cpu->opcodes[i] == &invalid_handler) { continue; -- cgit v1.2.3 From 96598cdb14da4d48c68f178a7b5f8d47f5c638f4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 15 Oct 2020 23:18:39 +0200 Subject: spapr: Drop spapr_delete_vcpu() unused argument The 'sc' argument is unused. Drop it. Signed-off-by: Greg Kurz Message-Id: <160279671929.1808373.10333672533575251075.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index e4aeb93c02..45eb212187 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -212,7 +212,7 @@ static void spapr_cpu_core_reset_handler(void *opaque) spapr_cpu_core_reset(opaque); } -static void spapr_delete_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) +static void spapr_delete_vcpu(PowerPCCPU *cpu) { SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); @@ -233,7 +233,7 @@ static void spapr_cpu_core_unrealize(DeviceState *dev) spapr_unrealize_vcpu(sc->threads[i], sc); } for (i = 0; i < cc->nr_threads; i++) { - spapr_delete_vcpu(sc->threads[i], sc); + spapr_delete_vcpu(sc->threads[i]); } g_free(sc->threads); } @@ -345,7 +345,7 @@ err_unrealize: } err: while (--i >= 0) { - spapr_delete_vcpu(sc->threads[i], sc); + spapr_delete_vcpu(sc->threads[i]); } g_free(sc->threads); } -- cgit v1.2.3 From 9370c28f12ca9336dd893e3b673a334c4938c58f Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 15 Oct 2020 23:18:46 +0200 Subject: spapr: Make spapr_cpu_core_unrealize() idempotent spapr_cpu_core_realize() has a rollback path which partially duplicates the code of spapr_cpu_core_unrealize(). Let's make spapr_cpu_core_unrealize() idempotent and call it instead. This requires to: - move the registration and unregistration of the reset handler around but it is harmless, - allocate the array of vCPUs with g_new0() to be able to filter out unused slots, - make sure to only unrealize vCPUs that have been already realized. Signed-off-by: Greg Kurz Message-Id: <160279672626.1808373.14142129300586424514.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 45eb212187..317fb9934f 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -227,15 +227,26 @@ static void spapr_cpu_core_unrealize(DeviceState *dev) CPUCore *cc = CPU_CORE(dev); int i; - qemu_unregister_reset(spapr_cpu_core_reset_handler, sc); - for (i = 0; i < cc->nr_threads; i++) { - spapr_unrealize_vcpu(sc->threads[i], sc); + if (sc->threads[i]) { + /* + * Since this we can get here from the error path of + * spapr_cpu_core_realize(), make sure we only unrealize + * vCPUs that have already been realized. + */ + if (object_property_get_bool(OBJECT(sc->threads[i]), "realized", + &error_abort)) { + spapr_unrealize_vcpu(sc->threads[i], sc); + } + } } for (i = 0; i < cc->nr_threads; i++) { - spapr_delete_vcpu(sc->threads[i]); + if (sc->threads[i]) { + spapr_delete_vcpu(sc->threads[i]); + } } g_free(sc->threads); + qemu_unregister_reset(spapr_cpu_core_reset_handler, sc); } static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr, @@ -322,32 +333,22 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) return; } - sc->threads = g_new(PowerPCCPU *, cc->nr_threads); + qemu_register_reset(spapr_cpu_core_reset_handler, sc); + sc->threads = g_new0(PowerPCCPU *, cc->nr_threads); for (i = 0; i < cc->nr_threads; i++) { sc->threads[i] = spapr_create_vcpu(sc, i, errp); if (!sc->threads[i]) { - goto err; + spapr_cpu_core_unrealize(dev); + return; } } for (j = 0; j < cc->nr_threads; j++) { if (!spapr_realize_vcpu(sc->threads[j], spapr, sc, errp)) { - goto err_unrealize; + spapr_cpu_core_unrealize(dev); + return; } } - - qemu_register_reset(spapr_cpu_core_reset_handler, sc); - return; - -err_unrealize: - while (--j >= 0) { - spapr_unrealize_vcpu(sc->threads[j], sc); - } -err: - while (--i >= 0) { - spapr_delete_vcpu(sc->threads[i]); - } - g_free(sc->threads); } static Property spapr_cpu_core_properties[] = { -- cgit v1.2.3 From 3cff86f036142057368b6040a8c78dce225500c7 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 15 Oct 2020 23:18:53 +0200 Subject: spapr: Simplify spapr_cpu_core_realize() and spapr_cpu_core_unrealize() Now that the error path of spapr_cpu_core_realize() is just to call idempotent spapr_cpu_core_unrealize() for rollback, no need to create and realize the vCPUs in two separate loops. Merge them and do them same in spapr_cpu_core_unrealize() for symmetry. Signed-off-by: Greg Kurz Message-Id: <160279673321.1808373.2248221100790367912.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 317fb9934f..2f7dc3c23d 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -238,10 +238,6 @@ static void spapr_cpu_core_unrealize(DeviceState *dev) &error_abort)) { spapr_unrealize_vcpu(sc->threads[i], sc); } - } - } - for (i = 0; i < cc->nr_threads; i++) { - if (sc->threads[i]) { spapr_delete_vcpu(sc->threads[i]); } } @@ -326,7 +322,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) TYPE_SPAPR_MACHINE); SpaprCpuCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); CPUCore *cc = CPU_CORE(OBJECT(dev)); - int i, j; + int i; if (!spapr) { error_setg(errp, TYPE_SPAPR_CPU_CORE " needs a pseries machine"); @@ -337,14 +333,8 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) sc->threads = g_new0(PowerPCCPU *, cc->nr_threads); for (i = 0; i < cc->nr_threads; i++) { sc->threads[i] = spapr_create_vcpu(sc, i, errp); - if (!sc->threads[i]) { - spapr_cpu_core_unrealize(dev); - return; - } - } - - for (j = 0; j < cc->nr_threads; j++) { - if (!spapr_realize_vcpu(sc->threads[j], spapr, sc, errp)) { + if (!sc->threads[i] || + !spapr_realize_vcpu(sc->threads[i], spapr, sc, errp)) { spapr_cpu_core_unrealize(dev); return; } -- cgit v1.2.3 From 84fd54961933a324e99bb52d0cc1de0ac9b7780e Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 19 Oct 2020 10:48:04 +0200 Subject: pc-dimm: Drop @errp argument of pc_dimm_plug() pc_dimm_plug() doesn't use it. It only aborts on error. Drop @errp and adapt the callers accordingly. [dwg: Removed unused label to fix compile] Signed-off-by: Greg Kurz Message-Id: <160309728447.2739814.12831204841251148202.stgit@bahia.lan> Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Igor Mammedov Signed-off-by: David Gibson --- hw/arm/virt.c | 9 +-------- hw/i386/pc.c | 8 +------- hw/mem/pc-dimm.c | 2 +- hw/ppc/spapr.c | 6 +----- include/hw/mem/pc-dimm.h | 2 +- 5 files changed, 5 insertions(+), 22 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index e465a988d6..27dbeb549e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2261,12 +2261,8 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev, VirtMachineState *vms = VIRT_MACHINE(hotplug_dev); MachineState *ms = MACHINE(hotplug_dev); bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); - Error *local_err = NULL; - pc_dimm_plug(PC_DIMM(dev), MACHINE(vms), &local_err); - if (local_err) { - goto out; - } + pc_dimm_plug(PC_DIMM(dev), MACHINE(vms)); if (is_nvdimm) { nvdimm_plug(ms->nvdimms_state); @@ -2274,9 +2270,6 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev, hotplug_handler_plug(HOTPLUG_HANDLER(vms->acpi_dev), dev, &error_abort); - -out: - error_propagate(errp, local_err); } static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev, diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 4e323755d0..0d9bd7d635 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1265,24 +1265,18 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, static void pc_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { - Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); X86MachineState *x86ms = X86_MACHINE(hotplug_dev); MachineState *ms = MACHINE(hotplug_dev); bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); - pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err); - if (local_err) { - goto out; - } + pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms)); if (is_nvdimm) { nvdimm_plug(ms->nvdimms_state); } hotplug_handler_plug(x86ms->acpi_dev, dev, &error_abort); -out: - error_propagate(errp, local_err); } static void pc_memory_unplug_request(HotplugHandler *hotplug_dev, diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index c30351070b..2ffc986734 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -64,7 +64,7 @@ void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, errp); } -void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp) +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine) { PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index ee716a12af..05abf6cc57 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3438,10 +3438,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, size = memory_device_get_region_size(MEMORY_DEVICE(dev), &error_abort); - pc_dimm_plug(dimm, MACHINE(ms), &local_err); - if (local_err) { - goto out; - } + pc_dimm_plug(dimm, MACHINE(ms)); if (!is_nvdimm) { addr = object_property_get_uint(OBJECT(dimm), @@ -3469,7 +3466,6 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, out_unplug: pc_dimm_unplug(dimm, MACHINE(ms)); -out: error_propagate(errp, local_err); } diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h index aec9527fdd..3d3db82641 100644 --- a/include/hw/mem/pc-dimm.h +++ b/include/hw/mem/pc-dimm.h @@ -72,6 +72,6 @@ struct PCDIMMDeviceClass { void pc_dimm_pre_plug(PCDIMMDevice *dimm, MachineState *machine, const uint64_t *legacy_align, Error **errp); -void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine, Error **errp); +void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine); void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine); #endif -- cgit v1.2.3 From 65226afd90abba32d06dd5699655d85b83a84a61 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 19 Oct 2020 10:48:16 +0200 Subject: spapr: Use appropriate getter for PC_DIMM_ADDR_PROP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PC_DIMM_ADDR_PROP property is defined as: DEFINE_PROP_UINT64(PC_DIMM_ADDR_PROP, PCDIMMDevice, addr, 0), Use object_property_get_uint() instead of object_property_get_int(). Signed-off-by: Greg Kurz Message-Id: <160309729609.2739814.4996614957953215591.stgit@bahia.lan> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/spapr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 05abf6cc57..b2405599ad 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3571,8 +3571,8 @@ static SpaprDimmState *spapr_recover_pending_dimm_state(SpaprMachineState *ms, uint64_t addr_start, addr; int i; - addr_start = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, - &error_abort); + addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, + &error_abort); addr = addr_start; for (i = 0; i < nr_lmbs; i++) { -- cgit v1.2.3 From 581778dd4727df93e3fe810d721e44157f64b97f Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 19 Oct 2020 10:48:27 +0200 Subject: spapr: Use appropriate getter for PC_DIMM_SLOT_PROP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PC_DIMM_SLOT_PROP property is defined as: DEFINE_PROP_INT32(PC_DIMM_SLOT_PROP, PCDIMMDevice, slot, PC_DIMM_UNASSIGNED_SLOT), Use object_property_get_int() instead of object_property_get_uint(). Since spapr_memory_plug() only gets called if pc_dimm_pre_plug() succeeded, we expect to have a valid >= 0 slot number, either because the user passed a valid slot number or because pc_dimm_get_free_slot() picked one up for us. Signed-off-by: Greg Kurz Message-Id: <160309730758.2739814.15821922745424652642.stgit@bahia.lan> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/spapr.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index b2405599ad..f8856ccf27 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3433,7 +3433,8 @@ 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); - uint64_t size, addr, slot; + uint64_t size, addr; + int64_t slot; bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM); size = memory_device_get_region_size(MEMORY_DEVICE(dev), &error_abort); @@ -3450,11 +3451,13 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT), &local_err); } else { - slot = object_property_get_uint(OBJECT(dimm), - PC_DIMM_SLOT_PROP, &local_err); + slot = object_property_get_int(OBJECT(dimm), + PC_DIMM_SLOT_PROP, &local_err); if (local_err) { goto out_unplug; } + /* We should have valid slot number at this point */ + g_assert(slot >= 0); spapr_add_nvdimm(dev, slot, &local_err); } -- cgit v1.2.3 From 271ced1d62e0c46089fab47c8560c4e550806d69 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 19 Oct 2020 10:48:41 +0200 Subject: spapr: Pass &error_abort when getting some PC DIMM properties Both PC_DIMM_SLOT_PROP and PC_DIMM_ADDR_PROP are defined in the default property list of the PC DIMM device class: DEFINE_PROP_UINT64(PC_DIMM_ADDR_PROP, PCDIMMDevice, addr, 0), DEFINE_PROP_INT32(PC_DIMM_SLOT_PROP, PCDIMMDevice, slot, PC_DIMM_UNASSIGNED_SLOT), They should thus be always gettable for both PC DIMMs and NVDIMMs. An error in getting them can only be the result of a programming error. It doesn't make much sense to propagate the error in this case. Abort instead. Signed-off-by: Greg Kurz Message-Id: <160309732180.2739814.7243774674998010907.stgit@bahia.lan> Reviewed-by: Igor Mammedov Signed-off-by: David Gibson --- hw/ppc/spapr.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f8856ccf27..a5aef7a6ff 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3443,19 +3443,13 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, if (!is_nvdimm) { addr = object_property_get_uint(OBJECT(dimm), - PC_DIMM_ADDR_PROP, &local_err); - if (local_err) { - goto out_unplug; - } + PC_DIMM_ADDR_PROP, &error_abort); spapr_add_lmbs(dev, addr, size, spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT), &local_err); } else { slot = object_property_get_int(OBJECT(dimm), - PC_DIMM_SLOT_PROP, &local_err); - if (local_err) { - goto out_unplug; - } + PC_DIMM_SLOT_PROP, &error_abort); /* We should have valid slot number at this point */ g_assert(slot >= 0); spapr_add_nvdimm(dev, slot, &local_err); @@ -3633,7 +3627,6 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { SpaprMachineState *spapr = SPAPR_MACHINE(hotplug_dev); - Error *local_err = NULL; PCDIMMDevice *dimm = PC_DIMM(dev); uint32_t nr_lmbs; uint64_t size, addr_start, addr; @@ -3649,11 +3642,7 @@ static void spapr_memory_unplug_request(HotplugHandler *hotplug_dev, nr_lmbs = size / SPAPR_MEMORY_BLOCK_SIZE; addr_start = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, - &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + &error_abort); /* * An existing pending dimm state for this DIMM means that there is an -- cgit v1.2.3 From 6e837f98ba03bab8008b7c1a6c125298ce41de7a Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 19 Oct 2020 10:49:01 +0200 Subject: spapr: Simplify error handling in spapr_memory_plug() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As recommended in "qapi/error.h", add a bool return value to spapr_add_lmbs() and spapr_add_nvdimm(), and use them instead of local_err in spapr_memory_plug(). This allows to get rid of the error propagation overhead. Signed-off-by: Greg Kurz Message-Id: <160309734178.2739814.3488437759887793902.stgit@bahia.lan> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/spapr.c | 22 ++++++++++------------ hw/ppc/spapr_nvdimm.c | 5 +++-- include/hw/ppc/spapr_nvdimm.h | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a5aef7a6ff..0cc19b5863 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3382,7 +3382,7 @@ int spapr_lmb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, return 0; } -static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, +static bool spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, bool dedicated_hp_event_source, Error **errp) { SpaprDrc *drc; @@ -3403,7 +3403,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, addr / SPAPR_MEMORY_BLOCK_SIZE); spapr_drc_detach(drc); } - return; + return false; } if (!hotplugged) { spapr_drc_reset(drc); @@ -3425,12 +3425,12 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t addr_start, uint64_t size, nr_lmbs); } } + return true; } static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { - Error *local_err = NULL; SpaprMachineState *ms = SPAPR_MACHINE(hotplug_dev); PCDIMMDevice *dimm = PC_DIMM(dev); uint64_t size, addr; @@ -3444,26 +3444,24 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, if (!is_nvdimm) { addr = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP, &error_abort); - spapr_add_lmbs(dev, addr, size, - spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT), - &local_err); + if (!spapr_add_lmbs(dev, addr, size, + spapr_ovec_test(ms->ov5_cas, OV5_HP_EVT), errp)) { + goto out_unplug; + } } else { slot = object_property_get_int(OBJECT(dimm), PC_DIMM_SLOT_PROP, &error_abort); /* We should have valid slot number at this point */ g_assert(slot >= 0); - spapr_add_nvdimm(dev, slot, &local_err); - } - - if (local_err) { - goto out_unplug; + if (!spapr_add_nvdimm(dev, slot, errp)) { + goto out_unplug; + } } return; out_unplug: pc_dimm_unplug(dimm, MACHINE(ms)); - error_propagate(errp, local_err); } static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c index 9e3d94071f..a833a63b5e 100644 --- a/hw/ppc/spapr_nvdimm.c +++ b/hw/ppc/spapr_nvdimm.c @@ -89,7 +89,7 @@ bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm, } -void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp) +bool spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp) { SpaprDrc *drc; bool hotplugged = spapr_drc_hotplugged(dev); @@ -98,12 +98,13 @@ void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp) g_assert(drc); if (!spapr_drc_attach(drc, dev, errp)) { - return; + return false; } if (hotplugged) { spapr_hotplug_req_add_by_index(drc); } + return true; } static int spapr_dt_nvdimm(SpaprMachineState *spapr, void *fdt, diff --git a/include/hw/ppc/spapr_nvdimm.h b/include/hw/ppc/spapr_nvdimm.h index 490b19a009..344582d2f5 100644 --- a/include/hw/ppc/spapr_nvdimm.h +++ b/include/hw/ppc/spapr_nvdimm.h @@ -30,6 +30,6 @@ int spapr_pmem_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, void spapr_dt_persistent_memory(SpaprMachineState *spapr, void *fdt); bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm, uint64_t size, Error **errp); -void spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp); +bool spapr_add_nvdimm(DeviceState *dev, uint64_t slot, Error **errp); #endif -- cgit v1.2.3 From c3e051ed6d2a0337fa5172d27231a193f18f92c4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 26 Oct 2020 13:40:40 +0100 Subject: spapr: Use error_append_hint() in spapr_reallocate_hpt() Hints should be added with the dedicated error_append_hint() API because we don't want to print them when using QMP. This requires to insert ERRP_GUARD as explained in "qapi/error.h". Signed-off-by: Greg Kurz Message-Id: <160371604030.305923.17464161378167312662.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 0cc19b5863..ba0894e73a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1486,6 +1486,7 @@ void spapr_free_hpt(SpaprMachineState *spapr) void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp) { + ERRP_GUARD(); long rc; /* Clean up any HPT info from a previous boot */ @@ -1500,17 +1501,18 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, if (rc < 0) { /* kernel-side HPT needed, but couldn't allocate one */ - error_setg_errno(errp, errno, - "Failed to allocate KVM HPT of order %d (try smaller maxmem?)", + error_setg_errno(errp, errno, "Failed to allocate KVM HPT of order %d", shift); + error_append_hint(errp, "Try smaller maxmem?\n"); /* This is almost certainly fatal, but if the caller really * wants to carry on with shift == 0, it's welcome to try */ } else if (rc > 0) { /* kernel-side HPT allocated */ if (rc != shift) { error_setg(errp, - "Requested order %d HPT, but kernel allocated order %ld (try smaller maxmem?)", + "Requested order %d HPT, but kernel allocated order %ld", shift, rc); + error_append_hint(errp, "Try smaller maxmem?\n"); } spapr->htab_shift = shift; -- cgit v1.2.3 From 0a06e4d6267ca150d62fbc371afab2fbb5586cb8 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 26 Oct 2020 13:40:47 +0100 Subject: target/ppc: Fix kvmppc_load_htab_chunk() error reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If kvmppc_load_htab_chunk() fails, its return value is propagated up to vmstate_load(). It should thus be a negative errno, not -1 (which maps to EPERM and would lure the user into thinking that the problem is necessarily related to a lack of privilege). Return the error reported by KVM or ENOSPC in case of short write. While here, propagate the error message through an @errp argument and have the caller to print it with error_report_err() instead of relying on fprintf(). Signed-off-by: Greg Kurz Message-Id: <160371604713.305923.5264900354159029580.stgit@bahia.lan> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/spapr.c | 4 +++- target/ppc/kvm.c | 11 +++++------ target/ppc/kvm_ppc.h | 5 +++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index ba0894e73a..ec2536dba1 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2347,8 +2347,10 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id) assert(fd >= 0); - rc = kvmppc_load_htab_chunk(f, fd, index, n_valid, n_invalid); + rc = kvmppc_load_htab_chunk(f, fd, index, n_valid, n_invalid, + &local_err); if (rc < 0) { + error_report_err(local_err); return rc; } } diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index d85ba8ffe0..0223b93ea5 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -2683,7 +2683,7 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns) } int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index, - uint16_t n_valid, uint16_t n_invalid) + uint16_t n_valid, uint16_t n_invalid, Error **errp) { struct kvm_get_htab_header *buf; size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64; @@ -2698,14 +2698,13 @@ int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index, rc = write(fd, buf, chunksize); if (rc < 0) { - fprintf(stderr, "Error writing KVM hash table: %s\n", - strerror(errno)); - return rc; + error_setg_errno(errp, errno, "Error writing the KVM hash table"); + return -errno; } if (rc != chunksize) { /* We should never get a short write on a single chunk */ - fprintf(stderr, "Short write, restoring KVM hash table\n"); - return -1; + error_setg(errp, "Short write while restoring the KVM hash table"); + return -ENOSPC; } return 0; } diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h index 72e05f1cd2..73ce2bc951 100644 --- a/target/ppc/kvm_ppc.h +++ b/target/ppc/kvm_ppc.h @@ -56,7 +56,7 @@ int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function); int kvmppc_get_htab_fd(bool write, uint64_t index, Error **errp); int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns); int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index, - uint16_t n_valid, uint16_t n_invalid); + uint16_t n_valid, uint16_t n_invalid, Error **errp); void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n); void kvmppc_write_hpte(hwaddr ptex, uint64_t pte0, uint64_t pte1); bool kvmppc_has_cap_fixup_hcalls(void); @@ -316,7 +316,8 @@ static inline int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, } static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index, - uint16_t n_valid, uint16_t n_invalid) + uint16_t n_valid, uint16_t n_invalid, + Error **errp) { abort(); } -- cgit v1.2.3 From a4e3a7c02bec45b1054c5e4fe3234519498fb55a Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 26 Oct 2020 13:40:54 +0100 Subject: spapr: Improve spapr_reallocate_hpt() error reporting spapr_reallocate_hpt() has three users, two of which pass &error_fatal and the third one, htab_load(), passes &local_err, uses it to detect failures and simply propagates -EINVAL up to vmstate_load(), which will cause QEMU to exit. It is thus confusing that spapr_reallocate_hpt() doesn't return right away when an error is detected in some cases. Also, the comment suggesting that the caller is welcome to try to carry on seems like a remnant in this respect. This can be improved: - change spapr_reallocate_hpt() to always report a negative errno on failure, either as reported by KVM or -ENOSPC if the HPT is smaller than what was asked, - use that to detect failures in htab_load() which is preferred over checking &local_err, - propagate this negative errno to vmstate_load() because it is more accurate than propagating -EINVAL for all possible errors. [dwg: Fix compile error due to omitted prelim patch] Signed-off-by: Greg Kurz Message-Id: <160371605460.305923.5890143959901241157.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr.c | 20 +++++++++++--------- include/hw/ppc/spapr.h | 3 +-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index ec2536dba1..227075103e 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1483,8 +1483,7 @@ void spapr_free_hpt(SpaprMachineState *spapr) close_htab_fd(spapr); } -void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, - Error **errp) +int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp) { ERRP_GUARD(); long rc; @@ -1496,7 +1495,7 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, if (rc == -EOPNOTSUPP) { error_setg(errp, "HPT not supported in nested guests"); - return; + return -EOPNOTSUPP; } if (rc < 0) { @@ -1504,8 +1503,7 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, error_setg_errno(errp, errno, "Failed to allocate KVM HPT of order %d", shift); error_append_hint(errp, "Try smaller maxmem?\n"); - /* This is almost certainly fatal, but if the caller really - * wants to carry on with shift == 0, it's welcome to try */ + return -errno; } else if (rc > 0) { /* kernel-side HPT allocated */ if (rc != shift) { @@ -1513,6 +1511,7 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, "Requested order %d HPT, but kernel allocated order %ld", shift, rc); error_append_hint(errp, "Try smaller maxmem?\n"); + return -ENOSPC; } spapr->htab_shift = shift; @@ -1526,7 +1525,7 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, if (!spapr->htab) { error_setg_errno(errp, errno, "Could not allocate HPT of order %d", shift); - return; + return -ENOMEM; } memset(spapr->htab, 0, size); @@ -1539,6 +1538,7 @@ void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, /* We're setting up a hash table, so that means we're not radix */ spapr->patb_entry = 0; spapr_set_all_lpcrs(0, LPCR_HR | LPCR_UPRT); + return 0; } void spapr_setup_hpt(SpaprMachineState *spapr) @@ -2292,11 +2292,13 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id) } if (section_hdr) { + int ret; + /* First section gives the htab size */ - spapr_reallocate_hpt(spapr, section_hdr, &local_err); - if (local_err) { + ret = spapr_reallocate_hpt(spapr, section_hdr, &local_err); + if (ret < 0) { error_report_err(local_err); - return -EINVAL; + return ret; } return 0; } diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index bb47896f17..2e89e36cfb 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -846,8 +846,7 @@ void spapr_hotplug_req_add_by_count_indexed(SpaprDrcType drc_type, void spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type, uint32_t count, uint32_t index); int spapr_hpt_shift_for_ramsize(uint64_t ramsize); -void spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, - Error **errp); +int spapr_reallocate_hpt(SpaprMachineState *spapr, int shift, Error **errp); void spapr_clear_pending_events(SpaprMachineState *spapr); void spapr_clear_pending_hotplug_events(SpaprMachineState *spapr); int spapr_max_server_number(SpaprMachineState *spapr); -- cgit v1.2.3 From 136fbf654dd5fa88a5057dcc43947536f3b418df Mon Sep 17 00:00:00 2001 From: zhaolichang Date: Fri, 9 Oct 2020 14:44:37 +0800 Subject: ppc/: fix some comment spelling errors I found that there are many spelling errors in the comments of qemu/target/ppc. I used spellcheck to check the spelling errors and found some errors in the folder. Signed-off-by: zhaolichang Reviewed-by: David Edmondson Message-Id: <20201009064449.2336-3-zhaolichang@huawei.com> Reviewed-by: Thomas Huth Reviewed-by: Greg Kurz Signed-off-by: David Gibson --- target/ppc/cpu.h | 6 +++--- target/ppc/excp_helper.c | 6 +++--- target/ppc/fpu_helper.c | 2 +- target/ppc/internal.h | 2 +- target/ppc/kvm.c | 2 +- target/ppc/machine.c | 2 +- target/ppc/mmu-hash64.c | 2 +- target/ppc/mmu_helper.c | 4 ++-- target/ppc/translate_init.c.inc | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index e8aa185d4f..2eb41a295a 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -615,7 +615,7 @@ enum { #define FPSCR_VXCVI 8 /* Floating-point invalid operation exception (int) */ #define FPSCR_VE 7 /* Floating-point invalid operation exception enable */ #define FPSCR_OE 6 /* Floating-point overflow exception enable */ -#define FPSCR_UE 5 /* Floating-point undeflow exception enable */ +#define FPSCR_UE 5 /* Floating-point underflow exception enable */ #define FPSCR_ZE 4 /* Floating-point zero divide exception enable */ #define FPSCR_XE 3 /* Floating-point inexact exception enable */ #define FPSCR_NI 2 /* Floating-point non-IEEE mode */ @@ -2331,13 +2331,13 @@ enum { /* Internal hardware exception sources */ PPC_INTERRUPT_DECR, /* Decrementer exception */ PPC_INTERRUPT_HDECR, /* Hypervisor decrementer exception */ - PPC_INTERRUPT_PIT, /* Programmable inteval timer interrupt */ + PPC_INTERRUPT_PIT, /* Programmable interval timer interrupt */ PPC_INTERRUPT_FIT, /* Fixed interval timer interrupt */ PPC_INTERRUPT_WDT, /* Watchdog timer interrupt */ PPC_INTERRUPT_CDOORBELL, /* Critical doorbell interrupt */ PPC_INTERRUPT_DOORBELL, /* Doorbell interrupt */ PPC_INTERRUPT_PERFM, /* Performance monitor interrupt */ - PPC_INTERRUPT_HMI, /* Hypervisor Maintainance interrupt */ + PPC_INTERRUPT_HMI, /* Hypervisor Maintenance interrupt */ PPC_INTERRUPT_HDOORBELL, /* Hypervisor Doorbell interrupt */ PPC_INTERRUPT_HVIRT, /* Hypervisor virtualization interrupt */ }; diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index a988ba15f4..d7411bcc81 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -231,7 +231,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) } /* - * Exception targetting modifiers + * Exception targeting modifiers * * LPES0 is supported on POWER7/8/9 * LPES1 is not supported (old iSeries mode) @@ -1015,7 +1015,7 @@ static void ppc_hw_interrupt(CPUPPCState *env) * This means we will incorrectly execute past the power management * instruction instead of triggering a reset. * - * It generally means a discrepancy between the wakup conditions in the + * It generally means a discrepancy between the wakeup conditions in the * processor has_work implementation and the logic in this function. */ cpu_abort(env_cpu(env), @@ -1191,7 +1191,7 @@ void helper_rfi(CPUPPCState *env) void helper_rfid(CPUPPCState *env) { /* - * The architeture defines a number of rules for which bits can + * The architecture defines a number of rules for which bits can * change but in practice, we handle this in hreg_store_msr() * which will be called by do_rfi(), so there is no need to filter * here diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index ae43b08eb5..9b8c8b70b6 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -1804,7 +1804,7 @@ uint32_t helper_efdcmpeq(CPUPPCState *env, uint64_t op1, uint64_t op2) /* - * VSX_ADD_SUB - VSX floating point add/subract + * VSX_ADD_SUB - VSX floating point add/subtract * name - instruction mnemonic * op - operation (add or sub) * nels - number of elements (1, 2 or 4) diff --git a/target/ppc/internal.h b/target/ppc/internal.h index 15d655b356..b4df127f4a 100644 --- a/target/ppc/internal.h +++ b/target/ppc/internal.h @@ -1,5 +1,5 @@ /* - * PowerPC interal definitions for qemu. + * PowerPC internal definitions for qemu. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 0223b93ea5..daf690a678 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -487,7 +487,7 @@ int kvm_arch_init_vcpu(CPUState *cs) /* * KVM-HV has transactional memory on POWER8 also without * the KVM_CAP_PPC_HTM extension, so enable it here - * instead as long as it's availble to userspace on the + * instead as long as it's available to userspace on the * host. */ if (qemu_getauxval(AT_HWCAP2) & PPC_FEATURE2_HAS_HTM) { diff --git a/target/ppc/machine.c b/target/ppc/machine.c index 5e58377376..c38e7b1268 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -337,7 +337,7 @@ static int cpu_post_load(void *opaque, int version_id) /* * If we're operating in compat mode, we should be ok as long as - * the destination supports the same compatiblity mode. + * the destination supports the same compatibility mode. * * Otherwise, however, we require that the destination has exactly * the same CPU model as the source. diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index c31d21e6a9..977b2d1561 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -883,7 +883,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, /* * Note on LPCR usage: 970 uses HID4, but our special variant of * store_spr copies relevant fields into env->spr[SPR_LPCR]. - * Similarily we filter unimplemented bits when storing into LPCR + * Similarly we filter unimplemented bits when storing into LPCR * depending on the MMU version. This code can thus just use the * LPCR "as-is". */ diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c index 8972714775..50aa18a763 100644 --- a/target/ppc/mmu_helper.c +++ b/target/ppc/mmu_helper.c @@ -179,7 +179,7 @@ static inline int ppc6xx_tlb_pte_check(mmu_ctx_t *ctx, target_ulong pte0, } /* Compute access rights */ access = pp_check(ctx->key, pp, ctx->nx); - /* Keep the matching PTE informations */ + /* Keep the matching PTE information */ ctx->raddr = pte1; ctx->prot = access; ret = check_prot(ctx->prot, rw, type); @@ -2176,7 +2176,7 @@ void helper_store_sr(CPUPPCState *env, target_ulong srnum, target_ulong value) env->sr[srnum] = value; /* * Invalidating 256MB of virtual memory in 4kB pages is way - * longer than flusing the whole TLB. + * longer than flushing the whole TLB. */ #if !defined(FLUSH_ALL_TLBS) && 0 { diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc index d2a8204d60..dc68da3cfd 100644 --- a/target/ppc/translate_init.c.inc +++ b/target/ppc/translate_init.c.inc @@ -792,7 +792,7 @@ static void gen_spr_generic(CPUPPCState *env) &spr_read_xer, &spr_write_xer, &spr_read_xer, &spr_write_xer, 0x00000000); - /* Branch contol */ + /* Branch control */ spr_register(env, SPR_LR, "LR", &spr_read_lr, &spr_write_lr, &spr_read_lr, &spr_write_lr, -- cgit v1.2.3