From a005b3ef50439b5bc6b2eb0b5bda8e8c7c2368bf Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 26 Feb 2016 10:44:07 +0100 Subject: xics: report errors with the QEMU Error API Using the return value to report errors is error prone: - xics_alloc() returns -1 on error but spapr_vio_busdev_realize() errors on 0 - xics_alloc_block() returns the unclear value of ics->offset - 1 on error but both rtas_ibm_change_msi() and spapr_phb_realize() error on 0 This patch adds an errp argument to xics_alloc() and xics_alloc_block() to report errors. The return value of these functions is a valid IRQ number if errp is NULL. It is undefined otherwise. The corresponding error traces get promotted to error messages. Note that the "can't allocate IRQ" error message in spapr_vio_busdev_realize() also moves to xics_alloc(). Similar error message consolidation isn't really applicable to xics_alloc_block() because callers have extra context (device config address, MSI or MSIX). This fixes the issues mentioned above. Based on previous work from Brian W. Hart. Signed-off-by: Greg Kurz Signed-off-by: David Gibson --- hw/intc/xics.c | 13 +++++++++---- hw/ppc/spapr_events.c | 3 ++- hw/ppc/spapr_pci.c | 16 ++++++++++------ hw/ppc/spapr_vio.c | 7 ++++--- 4 files changed, 25 insertions(+), 14 deletions(-) (limited to 'hw') diff --git a/hw/intc/xics.c b/hw/intc/xics.c index e66ae328..213a370 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -712,7 +712,7 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) return -1; } -int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi) +int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi, Error **errp) { ICSState *ics = &icp->ics[src]; int irq; @@ -720,14 +720,14 @@ int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi) if (irq_hint) { assert(src == xics_find_source(icp, irq_hint)); if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) { - trace_xics_alloc_failed_hint(src, irq_hint); + error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint); return -1; } irq = irq_hint; } else { irq = ics_find_free_block(ics, 1, 1); if (irq < 0) { - trace_xics_alloc_failed_no_left(src); + error_setg(errp, "can't allocate IRQ: no IRQ left"); return -1; } irq += ics->offset; @@ -743,7 +743,8 @@ int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi) * Allocate block of consecutive IRQs, and return the number of the first IRQ in the block. * If align==true, aligns the first IRQ number to num. */ -int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align) +int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align, + Error **errp) { int i, first = -1; ICSState *ics = &icp->ics[src]; @@ -763,6 +764,10 @@ int xics_alloc_block(XICSState *icp, int src, int num, bool lsi, bool align) } else { first = ics_find_free_block(ics, num, 1); } + if (first < 0) { + error_setg(errp, "can't find a free %d-IRQ block", num); + return -1; + } if (first >= 0) { for (i = first; i < first + num; ++i) { diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index f5eac4b..39f4682 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -588,7 +588,8 @@ out_no_events: void spapr_events_init(sPAPRMachineState *spapr) { QTAILQ_INIT(&spapr->pending_events); - spapr->check_exception_irq = xics_alloc(spapr->icp, 0, 0, false); + spapr->check_exception_irq = xics_alloc(spapr->icp, 0, 0, false, + &error_fatal); spapr->epow_notifier.notify = spapr_powerdown_req; qemu_register_powerdown_notifier(&spapr->epow_notifier); spapr_rtas_register(RTAS_CHECK_EXCEPTION, "check-exception", diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 9b2b546..e8edad3 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -280,6 +280,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, PCIDevice *pdev = NULL; spapr_pci_msi *msi; int *config_addr_key; + Error *err = NULL; switch (func) { case RTAS_CHANGE_MSI_FN: @@ -354,9 +355,10 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, /* Allocate MSIs */ irq = xics_alloc_block(spapr->icp, 0, req_num, false, - ret_intr_type == RTAS_TYPE_MSI); - if (!irq) { - error_report("Cannot allocate MSIs for device %x", config_addr); + ret_intr_type == RTAS_TYPE_MSI, &err); + if (err) { + error_reportf_err(err, "Can't allocate MSIs for device %x: ", + config_addr); rtas_st(rets, 0, RTAS_OUT_HW_ERROR); return; } @@ -1367,10 +1369,12 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) /* Initialize the LSI table */ for (i = 0; i < PCI_NUM_PINS; i++) { uint32_t irq; + Error *local_err = NULL; - irq = xics_alloc_block(spapr->icp, 0, 1, true, false); - if (!irq) { - error_setg(errp, "spapr_allocate_lsi failed"); + irq = xics_alloc_block(spapr->icp, 0, 1, true, false, &local_err); + if (local_err) { + error_propagate(errp, local_err); + error_prepend(errp, "can't allocate LSIs: "); return; } diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index ac6666a..0f61a55 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -431,6 +431,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev; VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev); char *id; + Error *local_err = NULL; if (dev->reg != -1) { /* @@ -463,9 +464,9 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) dev->qdev.id = id; } - dev->irq = xics_alloc(spapr->icp, 0, dev->irq, false); - if (!dev->irq) { - error_setg(errp, "can't allocate IRQ"); + dev->irq = xics_alloc(spapr->icp, 0, dev->irq, false, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } -- cgit v1.1