xive: Improve irq claim/free path

spapr_xive_irq_claim() returns a bool to indicate if it succeeded.
But most of the callers and one callee use int return values and/or an
Error * with more information instead.  In any case, ints are a more
common idiom for success/failure states than bools (one never knows
what sense they'll be in).

So instead change to an int return value to indicate presence of error
+ an Error * to describe the details through that call chain.

It also didn't actually check if the irq was already claimed, which is
one of the primary purposes of the claim path, so do that.

spapr_xive_irq_free() also returned a bool... which no callers checked
and was always true, so just drop it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>

3 files changed, 18 insertions, 21 deletions

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 47b5ec0..04879ab 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -528,12 +528,17 @@ static void spapr_xive_register_types(void)
 
 type_init(spapr_xive_register_types)
 
-bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi)
+int spapr_xive_irq_claim(SpaprXive *xive, int lisn, bool lsi, Error **errp)
 {
     XiveSource *xsrc = &xive->source;
 
     assert(lisn < xive->nr_irqs);
 
+    if (xive_eas_is_valid(&xive->eat[lisn])) {
+        error_setg(errp, "IRQ %d is not free", lisn);
+        return -EBUSY;
+    }
+
     /*
      * Set default values when allocating an IRQ number
      */
@@ -543,24 +548,17 @@ bool spapr_xive_irq_claim(SpaprXive *xive, uint32_t lisn, bool lsi)
     }
 
     if (kvm_irqchip_in_kernel()) {
-        Error *local_err = NULL;
-
-        kvmppc_xive_source_reset_one(xsrc, lisn, &local_err);
-        if (local_err) {
-            error_report_err(local_err);
-            return false;
-        }
+        return kvmppc_xive_source_reset_one(xsrc, lisn, errp);
     }
 
-    return true;
+    return 0;
 }
 
-bool spapr_xive_irq_free(SpaprXive *xive, uint32_t lisn)
+void spapr_xive_irq_free(SpaprXive *xive, int lisn)
 {
     assert(lisn < xive->nr_irqs);
 
     xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID);
-    return true;
 }
 
 /*
diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index 2006f96..51b334b 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -232,14 +232,14 @@ void kvmppc_xive_sync_source(SpaprXive *xive, uint32_t lisn, Error **errp)
  * only need to inform the KVM XIVE device about their type: LSI or
  * MSI.
  */
-void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp)
+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;
+        return -ENODEV;
     }
 
     if (xive_source_irq_is_lsi(xsrc, srcno)) {
@@ -249,8 +249,8 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp)
         }
     }
 
-    kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state,
-                      true, errp);
+    return kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state,
+                             true, errp);
 }
 
 static void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp)
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 025c802..516bf00 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -246,7 +246,10 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp)
 
     /* Enable the CPU IPIs */
     for (i = 0; i < nr_servers; ++i) {
-        spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, false);
+        if (spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i,
+                                 false, errp) < 0) {
+            return;
+        }
     }
 
     spapr_xive_hcall_init(spapr);
@@ -255,11 +258,7 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp)
 static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi,
                                 Error **errp)
 {
-    if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) {
-        error_setg(errp, "IRQ %d is invalid", irq);
-        return -1;
-    }
-    return 0;
+    return spapr_xive_irq_claim(spapr->xive, irq, lsi, errp);
 }
 
 static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq)