From 4b160fad4f6179fa7f8385f87f124cfce1f809d3 Mon Sep 17 00:00:00 2001 From: Gustavo Romero Date: Wed, 22 Jul 2020 19:43:54 -0400 Subject: ppc/xive: Fix some typos in comments Fix some typos in comments about code modeling coalescing points in the XIVE routing engine (IVRE). Signed-off-by: Gustavo Romero Message-Id: <1595461434-27725-1-git-send-email-gromero@linux.ibm.com> Signed-off-by: David Gibson --- hw/intc/xive.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 9a16243..9b55e03 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1502,7 +1502,7 @@ static bool xive_presenter_notify(XiveFabric *xfb, uint8_t format, /* * Notification using the END ESe/ESn bit (Event State Buffer for - * escalation and notification). Profide futher coalescing in the + * escalation and notification). Provide further coalescing in the * Router. */ static bool xive_router_end_es_notify(XiveRouter *xrtr, uint8_t end_blk, @@ -1581,7 +1581,7 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, /* * Check the END ESn (Event State Buffer for notification) for - * even futher coalescing in the Router + * even further coalescing in the Router */ if (!xive_end_is_notify(&end)) { /* ESn[Q]=1 : end of notification */ @@ -1660,7 +1660,7 @@ do_escalation: /* * Check the END ESe (Event State Buffer for escalation) for even - * futher coalescing in the Router + * further coalescing in the Router */ if (!xive_end_is_uncond_escalation(&end)) { /* ESe[Q]=1 : end of notification */ -- cgit v1.1 From 82f086b5e7ec0adff5a3972f74c446325b4fef9a Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 6 Aug 2020 18:56:05 +0200 Subject: spapr/xive: Fix xive->fd if kvm_create_device() fails If the creation of the KVM XIVE device fails for some reasons, the negative errno ends up in xive->fd, but the rest of the code assumes that xive->fd either contains an open fd, ie. positive value, or -1. This doesn't cause any misbehavior except kvmppc_xive_disconnect() that will try to close(xive->fd) during rollback and likely be rewarded with an EBADF. Only set xive->fd with a open fd. Signed-off-by: Greg Kurz Message-Id: <159673296585.766512.15404407281299745442.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index edb7ee0..d55ea46 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -745,6 +745,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, size_t esb_len = (1ull << xsrc->esb_shift) * xsrc->nr_irqs; size_t tima_len = 4ull << TM_SHIFT; CPUState *cs; + int fd; /* * The KVM XIVE device already in use. This is the case when @@ -760,11 +761,12 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, } /* First, create the KVM XIVE device */ - xive->fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_XIVE, false); - if (xive->fd < 0) { - error_setg_errno(errp, -xive->fd, "XIVE: error creating KVM device"); + fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_XIVE, false); + if (fd < 0) { + error_setg_errno(errp, -fd, "XIVE: error creating KVM device"); return -1; } + xive->fd = fd; /* Tell KVM about the # of VCPUs we may have */ if (kvm_device_check_attr(xive->fd, KVM_DEV_XIVE_GRP_CTRL, -- cgit v1.1 From e781139539f3d73ac00f2aea17f5a154b10e4302 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 6 Aug 2020 18:56:13 +0200 Subject: spapr/xive: Simplify kvmppc_xive_disconnect() Since this function begins with: /* The KVM XIVE device is not in use */ if (!xive || xive->fd == -1) { return; } we obviously don't need to check xive->fd again. Signed-off-by: Greg Kurz Message-Id: <159673297296.766512.14780055521619233656.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index d55ea46..893a1ee 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -873,10 +873,8 @@ void kvmppc_xive_disconnect(SpaprInterruptController *intc) * and removed from the list of devices of the VM. The VCPU * presenters are also detached from the device. */ - if (xive->fd != -1) { - close(xive->fd); - xive->fd = -1; - } + close(xive->fd); + xive->fd = -1; kvm_kernel_irqchip = false; kvm_msi_via_irqfd_allowed = false; -- cgit v1.1 From cf36e5b3760df0a1fdd38970294cf7b0968fcc5c Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 7 Aug 2020 13:32:06 +0200 Subject: ppc/xive: Rework setup of XiveSource::esb_mmio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Depending on whether XIVE is emultated or backed with a KVM XIVE device, the ESB MMIOs of a XIVE source point to an I/O memory region or a mapped memory region. This is currently handled by checking kvm_irqchip_in_kernel() returns false in xive_source_realize(). This is a bit awkward as we usually need to do extra things when we're using the in-kernel backend, not less. But most important, we can do better: turn the existing "xive.esb" memory region into a plain container, introduce an "xive.esb-emulated" I/O subregion and rename the existing "xive.esb" subregion in the KVM code to "xive.esb-kvm". Since "xive.esb-kvm" is added with overlap and a higher priority, it prevails over "xive.esb-emulated" (ie. a guest using KVM XIVE will interact with "xive.esb-kvm" instead of the default "xive.esb-emulated" region. While here, consolidate the computation of the MMIO region size in a common helper. Suggested-by: Cédric Le Goater Signed-off-by: Greg Kurz Message-Id: <159679992680.876294.7520540158586170894.stgit@bahia.lan> Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 4 ++-- hw/intc/xive.c | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 893a1ee..6130882 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -742,7 +742,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, SpaprXive *xive = SPAPR_XIVE(intc); XiveSource *xsrc = &xive->source; Error *local_err = NULL; - size_t esb_len = (1ull << xsrc->esb_shift) * xsrc->nr_irqs; + size_t esb_len = xive_source_esb_len(xsrc); size_t tima_len = 4ull << TM_SHIFT; CPUState *cs; int fd; @@ -788,7 +788,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, } memory_region_init_ram_device_ptr(&xsrc->esb_mmio_kvm, OBJECT(xsrc), - "xive.esb", esb_len, xsrc->esb_mmap); + "xive.esb-kvm", esb_len, xsrc->esb_mmap); memory_region_add_subregion_overlap(&xsrc->esb_mmio, 0, &xsrc->esb_mmio_kvm, 1); diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 9b55e03..561d746 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1128,6 +1128,7 @@ static void xive_source_reset(void *dev) static void xive_source_realize(DeviceState *dev, Error **errp) { XiveSource *xsrc = XIVE_SOURCE(dev); + size_t esb_len = xive_source_esb_len(xsrc); assert(xsrc->xive); @@ -1147,11 +1148,11 @@ static void xive_source_realize(DeviceState *dev, Error **errp) xsrc->status = g_malloc0(xsrc->nr_irqs); xsrc->lsi_map = bitmap_new(xsrc->nr_irqs); - if (!kvm_irqchip_in_kernel()) { - memory_region_init_io(&xsrc->esb_mmio, OBJECT(xsrc), - &xive_source_esb_ops, xsrc, "xive.esb", - (1ull << xsrc->esb_shift) * xsrc->nr_irqs); - } + memory_region_init(&xsrc->esb_mmio, OBJECT(xsrc), "xive.esb", esb_len); + memory_region_init_io(&xsrc->esb_mmio_emulated, OBJECT(xsrc), + &xive_source_esb_ops, xsrc, "xive.esb-emulated", + esb_len); + memory_region_add_subregion(&xsrc->esb_mmio, 0, &xsrc->esb_mmio_emulated); qemu_register_reset(xive_source_reset, dev); } -- cgit v1.1 From e519cdd9bc02990bddd395656bdaec821c94c8fe Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 7 Aug 2020 13:32:14 +0200 Subject: ppc/xive: Introduce dedicated kvm_irqchip_in_kernel() wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calls to the KVM XIVE device are guarded by kvm_irqchip_in_kernel(). This ensures that QEMU won't try to use the device if KVM is disabled or if an in-kernel irqchip isn't required. When using ic-mode=dual with the pseries machine, we have two possible interrupt controllers: XIVE and XICS. The kvm_irqchip_in_kernel() helper will return true as soon as any of the KVM device is created. It might lure QEMU to think that the other one is also around, while it is not. This is exactly what happens with ic-mode=dual at machine init when claiming IRQ numbers, which must be done on all possible IRQ backends, eg. RTAS event sources or the PHB0 LSI table : only the KVM XICS device is active but we end up calling kvmppc_xive_source_reset_one() anyway, which fails. This doesn't cause any trouble because of another bug : kvmppc_xive_source_reset_one() lacks an error_setg() and callers don't see the failure. Most of the other kvmppc_xive_* functions have similar xive->fd checks to filter out the case when KVM XIVE isn't active. It might look safer to have idempotent functions but it doesn't really help to understand what's going on when debugging. Since we already have all the kvm_irqchip_in_kernel() in place, also have the callers to check xive->fd as well before calling KVM XIVE specific code. This is straight-forward for the spapr specific XIVE code. Some more care is needed for the platform agnostic XIVE code since it cannot access xive->fd directly. Introduce new in_kernel() methods in some base XIVE classes for this purpose and implement them only in spapr. In all cases, we still need to call kvm_irqchip_in_kernel() so that compilers can optimize the kvmppc_xive_* calls away when CONFIG_KVM isn't defined, thus avoiding the need for stubs. Signed-off-by: Greg Kurz Message-Id: <159679993438.876294.7285654331498605426.stgit@bahia.lan> Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/intc/spapr_xive.c | 45 +++++++++++++++++++++++++++++++-------------- hw/intc/xive.c | 25 +++++++++++++++++++------ 2 files changed, 50 insertions(+), 20 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 89c8cd9..3c84f64 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -148,12 +148,19 @@ static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end, xive_end_queue_pic_print_info(end, 6, mon); } +/* + * kvm_irqchip_in_kernel() will cause the compiler to turn this + * info a nop if CONFIG_KVM isn't defined. + */ +#define spapr_xive_in_kernel(xive) \ + (kvm_irqchip_in_kernel() && (xive)->fd != -1) + void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon) { XiveSource *xsrc = &xive->source; int i; - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { Error *local_err = NULL; kvmppc_xive_synchronize_state(xive, &local_err); @@ -507,8 +514,10 @@ static const VMStateDescription vmstate_spapr_xive_eas = { static int vmstate_spapr_xive_pre_save(void *opaque) { - if (kvm_irqchip_in_kernel()) { - return kvmppc_xive_pre_save(SPAPR_XIVE(opaque)); + SpaprXive *xive = SPAPR_XIVE(opaque); + + if (spapr_xive_in_kernel(xive)) { + return kvmppc_xive_pre_save(xive); } return 0; @@ -520,8 +529,10 @@ static int vmstate_spapr_xive_pre_save(void *opaque) */ static int spapr_xive_post_load(SpaprInterruptController *intc, int version_id) { - if (kvm_irqchip_in_kernel()) { - return kvmppc_xive_post_load(SPAPR_XIVE(intc), version_id); + SpaprXive *xive = SPAPR_XIVE(intc); + + if (spapr_xive_in_kernel(xive)) { + return kvmppc_xive_post_load(xive, version_id); } return 0; @@ -564,7 +575,7 @@ static int spapr_xive_claim_irq(SpaprInterruptController *intc, int lisn, xive_source_irq_set_lsi(xsrc, lisn); } - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { return kvmppc_xive_source_reset_one(xsrc, lisn, errp); } @@ -641,7 +652,7 @@ static void spapr_xive_set_irq(SpaprInterruptController *intc, int irq, int val) { SpaprXive *xive = SPAPR_XIVE(intc); - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { kvmppc_xive_source_set_irq(&xive->source, irq, val); } else { xive_source_set_irq(&xive->source, irq, val); @@ -749,11 +760,16 @@ static void spapr_xive_deactivate(SpaprInterruptController *intc) spapr_xive_mmio_set_enabled(xive, false); - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { kvmppc_xive_disconnect(intc); } } +static bool spapr_xive_in_kernel_xptr(const XivePresenter *xptr) +{ + return spapr_xive_in_kernel(SPAPR_XIVE(xptr)); +} + static void spapr_xive_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -788,6 +804,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) sicc->post_load = spapr_xive_post_load; xpc->match_nvt = spapr_xive_match_nvt; + xpc->in_kernel = spapr_xive_in_kernel_xptr; } static const TypeInfo spapr_xive_info = { @@ -1058,7 +1075,7 @@ static target_ulong h_int_set_source_config(PowerPCCPU *cpu, new_eas.w = xive_set_field64(EAS_END_DATA, new_eas.w, eisn); } - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { Error *local_err = NULL; kvmppc_xive_set_source_config(xive, lisn, &new_eas, &local_err); @@ -1379,7 +1396,7 @@ static target_ulong h_int_set_queue_config(PowerPCCPU *cpu, */ out: - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { Error *local_err = NULL; kvmppc_xive_set_queue_config(xive, end_blk, end_idx, &end, &local_err); @@ -1480,7 +1497,7 @@ static target_ulong h_int_get_queue_config(PowerPCCPU *cpu, args[2] = 0; } - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { Error *local_err = NULL; kvmppc_xive_get_queue_config(xive, end_blk, end_idx, end, &local_err); @@ -1642,7 +1659,7 @@ static target_ulong h_int_esb(PowerPCCPU *cpu, return H_P3; } - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { args[0] = kvmppc_xive_esb_rw(xsrc, lisn, offset, data, flags & SPAPR_XIVE_ESB_STORE); } else { @@ -1717,7 +1734,7 @@ static target_ulong h_int_sync(PowerPCCPU *cpu, * under KVM */ - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { Error *local_err = NULL; kvmppc_xive_sync_source(xive, lisn, &local_err); @@ -1761,7 +1778,7 @@ static target_ulong h_int_reset(PowerPCCPU *cpu, device_legacy_reset(DEVICE(xive)); - if (kvm_irqchip_in_kernel()) { + if (spapr_xive_in_kernel(xive)) { Error *local_err = NULL; kvmppc_xive_reset(xive, &local_err); diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 561d746..a453e8f 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -592,6 +592,17 @@ static const char * const xive_tctx_ring_names[] = { "USER", "OS", "POOL", "PHYS", }; +/* + * kvm_irqchip_in_kernel() will cause the compiler to turn this + * info a nop if CONFIG_KVM isn't defined. + */ +#define xive_in_kernel(xptr) \ + (kvm_irqchip_in_kernel() && \ + ({ \ + XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr); \ + xpc->in_kernel ? xpc->in_kernel(xptr) : false; \ + })) + void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon) { int cpu_index; @@ -606,7 +617,7 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon) cpu_index = tctx->cs ? tctx->cs->cpu_index : -1; - if (kvm_irqchip_in_kernel()) { + if (xive_in_kernel(tctx->xptr)) { Error *local_err = NULL; kvmppc_xive_cpu_synchronize_state(tctx, &local_err); @@ -671,7 +682,7 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp) } /* Connect the presenter to the VCPU (required for CPU hotplug) */ - if (kvm_irqchip_in_kernel()) { + if (xive_in_kernel(tctx->xptr)) { kvmppc_xive_cpu_connect(tctx, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -682,10 +693,11 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp) static int vmstate_xive_tctx_pre_save(void *opaque) { + XiveTCTX *tctx = XIVE_TCTX(opaque); Error *local_err = NULL; - if (kvm_irqchip_in_kernel()) { - kvmppc_xive_cpu_get_state(XIVE_TCTX(opaque), &local_err); + if (xive_in_kernel(tctx->xptr)) { + kvmppc_xive_cpu_get_state(tctx, &local_err); if (local_err) { error_report_err(local_err); return -1; @@ -697,14 +709,15 @@ static int vmstate_xive_tctx_pre_save(void *opaque) static int vmstate_xive_tctx_post_load(void *opaque, int version_id) { + XiveTCTX *tctx = XIVE_TCTX(opaque); Error *local_err = NULL; - if (kvm_irqchip_in_kernel()) { + if (xive_in_kernel(tctx->xptr)) { /* * Required for hotplugged CPU, for which the state comes * after all states of the machine. */ - kvmppc_xive_cpu_set_state(XIVE_TCTX(opaque), &local_err); + kvmppc_xive_cpu_set_state(tctx, &local_err); if (local_err) { error_report_err(local_err); return -1; -- cgit v1.1 From a4907119348d928e8b72e2d3b7566e50f272fa09 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 7 Aug 2020 13:32:21 +0200 Subject: spapr/xive: Convert KVM device fd checks to assert() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All callers guard these functions with an xive_in_kernel() helper. Make it clear that they are only to be called when the KVM XIVE device exists. Note that the check on xive is dropped in kvmppc_xive_disconnect(). It really cannot be NULL since it comes from set_active_intc() which only passes pointers to allocated objects. Signed-off-by: Greg Kurz Reviewed-by: David Gibson Message-Id: <159679994169.876294.11026653581505077112.stgit@bahia.lan> Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 35 +++++++---------------------------- 1 file changed, 7 insertions(+), 28 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 6130882..82a6f99 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -79,10 +79,7 @@ void kvmppc_xive_cpu_set_state(XiveTCTX *tctx, Error **errp) uint64_t state[2]; int ret; - /* The KVM XIVE device is not in use yet */ - if (xive->fd == -1) { - return; - } + assert(xive->fd != -1); /* word0 and word1 of the OS ring. */ state[0] = *((uint64_t *) &tctx->regs[TM_QW1_OS]); @@ -101,10 +98,7 @@ void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp) uint64_t state[2] = { 0 }; int ret; - /* The KVM XIVE device is not in use */ - if (xive->fd == -1) { - return; - } + assert(xive->fd != -1); ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_VP_STATE, state); if (ret != 0) { @@ -156,10 +150,7 @@ void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) unsigned long vcpu_id; int ret; - /* The KVM XIVE device is not in use */ - if (xive->fd == -1) { - return; - } + assert(xive->fd != -1); /* Check if CPU was hot unplugged and replugged. */ if (kvm_cpu_is_enabled(tctx->cs)) { @@ -245,10 +236,7 @@ int kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp) SpaprXive *xive = SPAPR_XIVE(xsrc->xive); uint64_t state = 0; - /* The KVM XIVE device is not in use */ - if (xive->fd == -1) { - return -ENODEV; - } + assert(xive->fd != -1); if (xive_source_irq_is_lsi(xsrc, srcno)) { state |= KVM_XIVE_LEVEL_SENSITIVE; @@ -592,10 +580,7 @@ static void kvmppc_xive_change_state_handler(void *opaque, int running, void kvmppc_xive_synchronize_state(SpaprXive *xive, Error **errp) { - /* The KVM XIVE device is not in use */ - if (xive->fd == -1) { - return; - } + assert(xive->fd != -1); /* * When the VM is stopped, the sources are masked and the previous @@ -622,10 +607,7 @@ int kvmppc_xive_pre_save(SpaprXive *xive) { Error *local_err = NULL; - /* The KVM XIVE device is not in use */ - if (xive->fd == -1) { - return 0; - } + assert(xive->fd != -1); /* EAT: there is no extra state to query from KVM */ @@ -845,10 +827,7 @@ void kvmppc_xive_disconnect(SpaprInterruptController *intc) XiveSource *xsrc; size_t esb_len; - /* The KVM XIVE device is not in use */ - if (!xive || xive->fd == -1) { - return; - } + assert(xive->fd != -1); /* Clear the KVM mapping */ xsrc = &xive->source; -- cgit v1.1 From 3885ca66881f1d2568e169dcbf793fd493146d14 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:05 +0200 Subject: spapr/xive: Rework error handling of kvmppc_xive_cpu_connect() Use error_setg_errno() instead of error_setg(strerror()). While here, use -ret instead of errno since kvm_vcpu_enable_cap() returns a negative errno on failure. Use ERRP_GUARD() to ensure that errp can be passed to error_append_hint(), and get rid of the local_err boilerplate. Propagate the return value so that callers may use it as well to check failures. Signed-off-by: Greg Kurz Message-Id: <159707844549.1489912.4862921680328017645.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 82a6f99..aa1a2f9 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -144,8 +144,9 @@ void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp) } } -void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) +int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) { + ERRP_GUARD(); SpaprXive *xive = SPAPR_XIVE(tctx->xptr); unsigned long vcpu_id; int ret; @@ -154,7 +155,7 @@ void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) /* Check if CPU was hot unplugged and replugged. */ if (kvm_cpu_is_enabled(tctx->cs)) { - return; + return 0; } vcpu_id = kvm_arch_vcpu_id(tctx->cs); @@ -162,20 +163,18 @@ void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) ret = kvm_vcpu_enable_cap(tctx->cs, KVM_CAP_PPC_IRQ_XIVE, 0, xive->fd, vcpu_id, 0); if (ret < 0) { - Error *local_err = NULL; - - error_setg(&local_err, - "XIVE: unable to connect CPU%ld to KVM device: %s", - vcpu_id, strerror(errno)); - if (errno == ENOSPC) { - error_append_hint(&local_err, "Try -smp maxcpus=N with N < %u\n", + error_setg_errno(errp, -ret, + "XIVE: unable to connect CPU%ld to KVM device", + vcpu_id); + if (ret == -ENOSPC) { + error_append_hint(errp, "Try -smp maxcpus=N with N < %u\n", MACHINE(qdev_get_machine())->smp.max_cpus); } - error_propagate(errp, local_err); - return; + return ret; } kvm_cpu_enable(tctx->cs); + return 0; } /* -- cgit v1.1 From 46407a2531da4ff206c1aefe8c3f6d8ad53f2de4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:12 +0200 Subject: spapr/xive: Rework error handling of kvmppc_xive_source_reset() Since kvmppc_xive_source_reset_one() has a return value, convert kvmppc_xive_source_reset() to use it for error checking. This allows to get rid of the local_err boiler plate. Propagate the return value so that callers may use it as well to check failures. Signed-off-by: Greg Kurz Message-Id: <159707845245.1489912.9151822670764690034.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index aa1a2f9..d801bf5 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -248,24 +248,25 @@ int kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp) true, errp); } -static void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp) +static int kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp) { SpaprXive *xive = SPAPR_XIVE(xsrc->xive); int i; for (i = 0; i < xsrc->nr_irqs; i++) { - Error *local_err = NULL; + int ret; if (!xive_eas_is_valid(&xive->eat[i])) { continue; } - kvmppc_xive_source_reset_one(xsrc, i, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; + ret = kvmppc_xive_source_reset_one(xsrc, i, errp); + if (ret < 0) { + return ret; } } + + return 0; } /* -- cgit v1.1 From b14adb4a27c80a255fb35451d7cb2bc70743e7f4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:19 +0200 Subject: spapr/xive: Rework error handling of kvmppc_xive_mmap() Callers currently check failures of kvmppc_xive_mmap() through the @errp argument, which isn't a recommanded practice. It is preferred to use a return value when possible. Since NULL isn't an invalid address in theory, it seems better to return MAP_FAILED and to teach callers to handle it. Signed-off-by: Greg Kurz Message-Id: <159707845972.1489912.719896767746375765.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index d801bf5..b2a36fd 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -698,6 +698,7 @@ int kvmppc_xive_post_load(SpaprXive *xive, int version_id) return 0; } +/* Returns MAP_FAILED on error and sets errno */ static void *kvmppc_xive_mmap(SpaprXive *xive, int pgoff, size_t len, Error **errp) { @@ -708,7 +709,6 @@ static void *kvmppc_xive_mmap(SpaprXive *xive, int pgoff, size_t len, pgoff << page_shift); if (addr == MAP_FAILED) { error_setg_errno(errp, errno, "XIVE: unable to set memory mapping"); - return NULL; } return addr; @@ -728,6 +728,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, size_t tima_len = 4ull << TM_SHIFT; CPUState *cs; int fd; + void *addr; /* * The KVM XIVE device already in use. This is the case when @@ -763,11 +764,12 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, /* * 1. Source ESB pages - KVM mapping */ - xsrc->esb_mmap = kvmppc_xive_mmap(xive, KVM_XIVE_ESB_PAGE_OFFSET, esb_len, - &local_err); - if (local_err) { + addr = kvmppc_xive_mmap(xive, KVM_XIVE_ESB_PAGE_OFFSET, esb_len, + &local_err); + if (addr == MAP_FAILED) { goto fail; } + xsrc->esb_mmap = addr; memory_region_init_ram_device_ptr(&xsrc->esb_mmio_kvm, OBJECT(xsrc), "xive.esb-kvm", esb_len, xsrc->esb_mmap); @@ -781,11 +783,13 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, /* * 3. TIMA pages - KVM mapping */ - xive->tm_mmap = kvmppc_xive_mmap(xive, KVM_XIVE_TIMA_PAGE_OFFSET, tima_len, - &local_err); - if (local_err) { + addr = kvmppc_xive_mmap(xive, KVM_XIVE_TIMA_PAGE_OFFSET, tima_len, + &local_err); + if (addr == MAP_FAILED) { goto fail; } + xive->tm_mmap = addr; + memory_region_init_ram_device_ptr(&xive->tm_mmio_kvm, OBJECT(xive), "xive.tima", tima_len, xive->tm_mmap); memory_region_add_subregion_overlap(&xive->tm_mmio, 0, -- cgit v1.1 From 5fa36b7ffbcb2056249929a7b1ee4e30c07dc67c Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:26 +0200 Subject: spapr/xive: Rework error handling of kvmppc_xive_cpu_[gs]et_state() kvm_set_one_reg() returns a negative errno on failure, use that instead of errno. Also propagate it to callers so they can use it to check for failures and hopefully get rid of their local_err boilerplate. Signed-off-by: Greg Kurz Message-Id: <159707846665.1489912.14267225652103441921.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index b2a36fd..5e088cc 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -73,7 +73,7 @@ static void kvm_cpu_disable_all(void) * XIVE Thread Interrupt Management context (KVM) */ -void kvmppc_xive_cpu_set_state(XiveTCTX *tctx, Error **errp) +int kvmppc_xive_cpu_set_state(XiveTCTX *tctx, Error **errp) { SpaprXive *xive = SPAPR_XIVE(tctx->xptr); uint64_t state[2]; @@ -86,13 +86,16 @@ void kvmppc_xive_cpu_set_state(XiveTCTX *tctx, Error **errp) ret = kvm_set_one_reg(tctx->cs, KVM_REG_PPC_VP_STATE, state); if (ret != 0) { - error_setg_errno(errp, errno, + error_setg_errno(errp, -ret, "XIVE: could not restore KVM state of CPU %ld", kvm_arch_vcpu_id(tctx->cs)); + return ret; } + + return 0; } -void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp) +int kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp) { SpaprXive *xive = SPAPR_XIVE(tctx->xptr); uint64_t state[2] = { 0 }; @@ -102,14 +105,16 @@ void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp) ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_VP_STATE, state); if (ret != 0) { - error_setg_errno(errp, errno, + error_setg_errno(errp, -ret, "XIVE: could not capture KVM state of CPU %ld", kvm_arch_vcpu_id(tctx->cs)); - return; + return ret; } /* word0 and word1 of the OS ring. */ *((uint64_t *) &tctx->regs[TM_QW1_OS]) = state[0]; + + return 0; } typedef struct { -- cgit v1.1 From f9a548edf2a54f59c37032dee3763f532e968fee Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:33 +0200 Subject: spapr/xive: Rework error handling of kvmppc_xive_[gs]et_queue_config() Since kvm_device_access() returns a negative errno on failure, convert kvmppc_xive_get_queue_config() and kvmppc_xive_set_queue_config() to use it for error checking. This allows to get rid of the local_err boilerplate. Propagate the return value so that callers may use it as well to check failures. Signed-off-by: Greg Kurz Message-Id: <159707847357.1489912.2032291280645236480.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 5e088cc..696623f 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -374,15 +374,15 @@ void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val) /* * sPAPR XIVE interrupt controller (KVM) */ -void kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk, - uint32_t end_idx, XiveEND *end, - Error **errp) +int kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk, + uint32_t end_idx, XiveEND *end, + Error **errp) { struct kvm_ppc_xive_eq kvm_eq = { 0 }; uint64_t kvm_eq_idx; uint8_t priority; uint32_t server; - Error *local_err = NULL; + int ret; assert(xive_end_is_valid(end)); @@ -394,11 +394,10 @@ void kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk, kvm_eq_idx |= server << KVM_XIVE_EQ_SERVER_SHIFT & KVM_XIVE_EQ_SERVER_MASK; - kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx, - &kvm_eq, false, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; + ret = kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx, + &kvm_eq, false, errp); + if (ret < 0) { + return ret; } /* @@ -408,17 +407,18 @@ void kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk, */ end->w1 = xive_set_field32(END_W1_GENERATION, 0ul, kvm_eq.qtoggle) | xive_set_field32(END_W1_PAGE_OFF, 0ul, kvm_eq.qindex); + + return 0; } -void kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk, - uint32_t end_idx, XiveEND *end, - Error **errp) +int kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk, + uint32_t end_idx, XiveEND *end, + Error **errp) { struct kvm_ppc_xive_eq kvm_eq = { 0 }; uint64_t kvm_eq_idx; uint8_t priority; uint32_t server; - Error *local_err = NULL; /* * Build the KVM state from the local END structure. @@ -456,12 +456,9 @@ void kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk, kvm_eq_idx |= server << KVM_XIVE_EQ_SERVER_SHIFT & KVM_XIVE_EQ_SERVER_MASK; - kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx, - &kvm_eq, true, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + return + kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx, + &kvm_eq, true, errp); } void kvmppc_xive_reset(SpaprXive *xive, Error **errp) -- cgit v1.1 From d53482a73bba983f521d6b9652e6f68e856ab794 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:40 +0200 Subject: spapr/xive: Rework error handling in kvmppc_xive_get_queues() Since kvmppc_xive_get_queue_config() has a return value, convert kvmppc_xive_get_queues() to use it for error checking. This allows to get rid of the local_err boiler plate. Propagate the return value so that callers may use it as well to check failures. Signed-off-by: Greg Kurz Message-Id: <159707848069.1489912.14879208798696134531.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 696623f..4142aaf 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -467,23 +467,24 @@ void kvmppc_xive_reset(SpaprXive *xive, Error **errp) NULL, true, errp); } -static void kvmppc_xive_get_queues(SpaprXive *xive, Error **errp) +static int kvmppc_xive_get_queues(SpaprXive *xive, Error **errp) { - Error *local_err = NULL; int i; + int ret; for (i = 0; i < xive->nr_ends; i++) { if (!xive_end_is_valid(&xive->endt[i])) { continue; } - kvmppc_xive_get_queue_config(xive, SPAPR_XIVE_BLOCK_ID, i, - &xive->endt[i], &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; + ret = kvmppc_xive_get_queue_config(xive, SPAPR_XIVE_BLOCK_ID, i, + &xive->endt[i], errp); + if (ret < 0) { + return ret; } } + + return 0; } /* -- cgit v1.1 From d55daadcb801f309556fdbab00b2653d20e26603 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:47 +0200 Subject: spapr/xive: Rework error handling of kvmppc_xive_set_source_config() Since kvm_device_access() returns a negative errno on failure, convert kvmppc_xive_set_source_config() to use it for error checking. This allows to get rid of the local_err boilerplate. Propagate the return value so that callers may use it as well to check failures. Signed-off-by: Greg Kurz Message-Id: <159707848764.1489912.17078842252160674523.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 4142aaf..f2dda69 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -186,8 +186,8 @@ int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) * XIVE Interrupt Source (KVM) */ -void kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas, - Error **errp) +int kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas, + Error **errp) { uint32_t end_idx; uint32_t end_blk; @@ -196,7 +196,6 @@ void kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas, bool masked; uint32_t eisn; uint64_t kvm_src; - Error *local_err = NULL; assert(xive_eas_is_valid(eas)); @@ -216,12 +215,8 @@ void kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas, kvm_src |= ((uint64_t)eisn << KVM_XIVE_SOURCE_EISN_SHIFT) & KVM_XIVE_SOURCE_EISN_MASK; - kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE_CONFIG, lisn, - &kvm_src, true, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + return kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE_CONFIG, lisn, + &kvm_src, true, errp); } void kvmppc_xive_sync_source(SpaprXive *xive, uint32_t lisn, Error **errp) -- cgit v1.1 From 42a92d925d03ec49dfdefb43c15b46c3ca55f9e4 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:54:54 +0200 Subject: spapr/kvm: Fix error handling in kvmppc_xive_pre_save() Now that kvmppc_xive_get_queues() returns a negative errno on failure, check with that because it is preferred to local_err. And most of all, propagate it because vmstate expects negative errnos. Signed-off-by: Greg Kurz Message-Id: <159707849455.1489912.6034461176847728064.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index f2dda69..1686b03 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -604,16 +604,17 @@ void kvmppc_xive_synchronize_state(SpaprXive *xive, Error **errp) int kvmppc_xive_pre_save(SpaprXive *xive) { Error *local_err = NULL; + int ret; assert(xive->fd != -1); /* EAT: there is no extra state to query from KVM */ /* ENDT */ - kvmppc_xive_get_queues(xive, &local_err); - if (local_err) { + ret = kvmppc_xive_get_queues(xive, &local_err); + if (ret < 0) { error_report_err(local_err); - return -1; + return ret; } return 0; -- cgit v1.1 From a845a54cbe0f197b833921b2af13fee88bb8240d Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:55:01 +0200 Subject: spapr/xive: Fix error handling in kvmppc_xive_post_load() Now that all these functions return a negative errno on failure, check that because it is preferred to local_err. And most of all, propagate it because vmstate expects negative errnos. Signed-off-by: Greg Kurz Message-Id: <159707850148.1489912.18355118622296682631.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 1686b03..005729e 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -631,6 +631,7 @@ int kvmppc_xive_post_load(SpaprXive *xive, int version_id) Error *local_err = NULL; CPUState *cs; int i; + int ret; /* The KVM XIVE device should be in use */ assert(xive->fd != -1); @@ -641,11 +642,10 @@ int kvmppc_xive_post_load(SpaprXive *xive, int version_id) continue; } - kvmppc_xive_set_queue_config(xive, SPAPR_XIVE_BLOCK_ID, i, - &xive->endt[i], &local_err); - if (local_err) { - error_report_err(local_err); - return -1; + ret = kvmppc_xive_set_queue_config(xive, SPAPR_XIVE_BLOCK_ID, i, + &xive->endt[i], &local_err); + if (ret < 0) { + goto fail; } } @@ -660,16 +660,14 @@ int kvmppc_xive_post_load(SpaprXive *xive, int version_id) * previously set in KVM. Since we don't do that for all interrupts * at reset time anymore, let's do it now. */ - kvmppc_xive_source_reset_one(&xive->source, i, &local_err); - if (local_err) { - error_report_err(local_err); - return -1; + ret = kvmppc_xive_source_reset_one(&xive->source, i, &local_err); + if (ret < 0) { + goto fail; } - kvmppc_xive_set_source_config(xive, i, &xive->eat[i], &local_err); - if (local_err) { - error_report_err(local_err); - return -1; + ret = kvmppc_xive_set_source_config(xive, i, &xive->eat[i], &local_err); + if (ret < 0) { + goto fail; } } @@ -686,15 +684,18 @@ int kvmppc_xive_post_load(SpaprXive *xive, int version_id) CPU_FOREACH(cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); - kvmppc_xive_cpu_set_state(spapr_cpu_state(cpu)->tctx, &local_err); - if (local_err) { - error_report_err(local_err); - return -1; + ret = kvmppc_xive_cpu_set_state(spapr_cpu_state(cpu)->tctx, &local_err); + if (ret < 0) { + goto fail; } } /* The source states will be restored when the machine starts running */ return 0; + +fail: + error_report_err(local_err); + return ret; } /* Returns MAP_FAILED on error and sets errno */ -- cgit v1.1 From 2a8100cb61959e7a934049f2bd3a49a0f84a066b Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:55:08 +0200 Subject: ppc/xive: Fix error handling in vmstate_xive_tctx_*() callbacks Now that kvmppc_xive_cpu_get_state() and kvmppc_xive_cpu_set_state() return negative errnos on failures, use that instead local_err because it is the recommended practice. Also return that instead of -1 since vmstate expects negative errnos. Signed-off-by: Greg Kurz Message-Id: <159707850840.1489912.14912810818646455474.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/xive.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/xive.c b/hw/intc/xive.c index a453e8f..17ca5a1 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -695,12 +695,13 @@ static int vmstate_xive_tctx_pre_save(void *opaque) { XiveTCTX *tctx = XIVE_TCTX(opaque); Error *local_err = NULL; + int ret; if (xive_in_kernel(tctx->xptr)) { - kvmppc_xive_cpu_get_state(tctx, &local_err); - if (local_err) { + ret = kvmppc_xive_cpu_get_state(tctx, &local_err); + if (ret < 0) { error_report_err(local_err); - return -1; + return ret; } } @@ -711,16 +712,17 @@ static int vmstate_xive_tctx_post_load(void *opaque, int version_id) { XiveTCTX *tctx = XIVE_TCTX(opaque); Error *local_err = NULL; + int ret; if (xive_in_kernel(tctx->xptr)) { /* * Required for hotplugged CPU, for which the state comes * after all states of the machine. */ - kvmppc_xive_cpu_set_state(tctx, &local_err); - if (local_err) { + ret = kvmppc_xive_cpu_set_state(tctx, &local_err); + if (ret < 0) { error_report_err(local_err); - return -1; + return ret; } } -- cgit v1.1 From 6cdc0e20631faf781047b515215208660393c9a9 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:55:15 +0200 Subject: spapr/xive: Simplify error handling in kvmppc_xive_connect() Now that all these functions return a negative errno on failure, check that and get rid of the local_err boilerplate. Signed-off-by: Greg Kurz Message-Id: <159707851537.1489912.1030839306195472651.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 005729e..e9a3611 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -723,12 +723,12 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, { SpaprXive *xive = SPAPR_XIVE(intc); XiveSource *xsrc = &xive->source; - Error *local_err = NULL; size_t esb_len = xive_source_esb_len(xsrc); size_t tima_len = 4ull << TM_SHIFT; CPUState *cs; int fd; void *addr; + int ret; /* * The KVM XIVE device already in use. This is the case when @@ -754,9 +754,10 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, /* Tell KVM about the # of VCPUs we may have */ if (kvm_device_check_attr(xive->fd, KVM_DEV_XIVE_GRP_CTRL, KVM_DEV_XIVE_NR_SERVERS)) { - if (kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_CTRL, - KVM_DEV_XIVE_NR_SERVERS, &nr_servers, true, - &local_err)) { + ret = kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_CTRL, + KVM_DEV_XIVE_NR_SERVERS, &nr_servers, true, + errp); + if (ret < 0) { goto fail; } } @@ -764,8 +765,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, /* * 1. Source ESB pages - KVM mapping */ - addr = kvmppc_xive_mmap(xive, KVM_XIVE_ESB_PAGE_OFFSET, esb_len, - &local_err); + addr = kvmppc_xive_mmap(xive, KVM_XIVE_ESB_PAGE_OFFSET, esb_len, errp); if (addr == MAP_FAILED) { goto fail; } @@ -783,8 +783,7 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, /* * 3. TIMA pages - KVM mapping */ - addr = kvmppc_xive_mmap(xive, KVM_XIVE_TIMA_PAGE_OFFSET, tima_len, - &local_err); + addr = kvmppc_xive_mmap(xive, KVM_XIVE_TIMA_PAGE_OFFSET, tima_len, errp); if (addr == MAP_FAILED) { goto fail; } @@ -802,15 +801,15 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, CPU_FOREACH(cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); - kvmppc_xive_cpu_connect(spapr_cpu_state(cpu)->tctx, &local_err); - if (local_err) { + ret = kvmppc_xive_cpu_connect(spapr_cpu_state(cpu)->tctx, errp); + if (ret < 0) { goto fail; } } /* Update the KVM sources */ - kvmppc_xive_source_reset(xsrc, &local_err); - if (local_err) { + ret = kvmppc_xive_source_reset(xsrc, errp); + if (ret < 0) { goto fail; } @@ -820,7 +819,6 @@ int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers, return 0; fail: - error_propagate(errp, local_err); kvmppc_xive_disconnect(intc); return -1; } -- cgit v1.1 From 61203f2b356f8aa7b6cfd3f792cd1f0cc7bdf99b Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:55:22 +0200 Subject: ppc/xive: Simplify error handling in xive_tctx_realize() Now that kvmppc_xive_cpu_connect() returns a negative errno on failure, use that and get rid of the local_err boilerplate. Signed-off-by: Greg Kurz Message-Id: <159707852234.1489912.16410314514265848075.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/xive.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 17ca5a1..489e625 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -662,7 +662,6 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp) XiveTCTX *tctx = XIVE_TCTX(dev); PowerPCCPU *cpu; CPUPPCState *env; - Error *local_err = NULL; assert(tctx->cs); assert(tctx->xptr); @@ -683,9 +682,7 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp) /* Connect the presenter to the VCPU (required for CPU hotplug) */ if (xive_in_kernel(tctx->xptr)) { - kvmppc_xive_cpu_connect(tctx, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (kvmppc_xive_cpu_connect(tctx, errp) < 0) { return; } } -- cgit v1.1 From 1118b6b72719ff83f2be1efc11d7248cf225074c Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Mon, 10 Aug 2020 18:55:29 +0200 Subject: spapr/xive: Simplify error handling of kvmppc_xive_cpu_synchronize_state() Now that kvmppc_xive_cpu_get_state() returns negative on error, use that and get rid of the temporary Error object and error_propagate(). Signed-off-by: Greg Kurz Message-Id: <159707852916.1489912.8376334685349668124.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive_kvm.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index e9a3611..d871bb1 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -119,7 +119,8 @@ int kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp) typedef struct { XiveTCTX *tctx; - Error *err; + Error **errp; + int ret; } XiveCpuGetState; static void kvmppc_xive_cpu_do_synchronize_state(CPUState *cpu, @@ -127,14 +128,14 @@ static void kvmppc_xive_cpu_do_synchronize_state(CPUState *cpu, { XiveCpuGetState *s = arg.host_ptr; - kvmppc_xive_cpu_get_state(s->tctx, &s->err); + s->ret = kvmppc_xive_cpu_get_state(s->tctx, s->errp); } -void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp) +int kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp) { XiveCpuGetState s = { .tctx = tctx, - .err = NULL, + .errp = errp, }; /* @@ -143,10 +144,7 @@ void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp) run_on_cpu(tctx->cs, kvmppc_xive_cpu_do_synchronize_state, RUN_ON_CPU_HOST_PTR(&s)); - if (s.err) { - error_propagate(errp, s.err); - return; - } + return s.ret; } int kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp) -- cgit v1.1 From 3110f0ee19ccdb50adff3dfa1321039f69efddcd Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 13 Aug 2020 19:28:10 +0200 Subject: spapr/xive: Use xive_source_esb_len() static inline size_t xive_source_esb_len(XiveSource *xsrc) { return (1ull << xsrc->esb_shift) * xsrc->nr_irqs; } Signed-off-by: Greg Kurz Message-Id: <159733969034.320580.6571451425779179477.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/intc/spapr_xive.c | 2 +- hw/intc/spapr_xive_kvm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/intc') diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 3c84f64..4bd0d60 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -336,7 +336,7 @@ static void spapr_xive_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(SYS_BUS_DEVICE(xive), &end_xsrc->esb_mmio); /* Set the mapping address of the END ESB pages after the source ESBs */ - xive->end_base = xive->vc_base + (1ull << xsrc->esb_shift) * xsrc->nr_irqs; + xive->end_base = xive->vc_base + xive_source_esb_len(xsrc); /* * Allocate the routing tables diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index d871bb1..e8667ce 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -831,7 +831,7 @@ void kvmppc_xive_disconnect(SpaprInterruptController *intc) /* Clear the KVM mapping */ xsrc = &xive->source; - esb_len = (1ull << xsrc->esb_shift) * xsrc->nr_irqs; + esb_len = xive_source_esb_len(xsrc); if (xsrc->esb_mmap) { memory_region_del_subregion(&xsrc->esb_mmio, &xsrc->esb_mmio_kvm); -- cgit v1.1