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>
This commit is contained in:
David Gibson 2019-09-25 13:24:14 +10:00
parent 580dde5e4a
commit e594c2ad1c
5 changed files with 21 additions and 24 deletions

View File

@ -528,12 +528,17 @@ static void spapr_xive_register_types(void)
type_init(spapr_xive_register_types) 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; XiveSource *xsrc = &xive->source;
assert(lisn < xive->nr_irqs); 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 * 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()) { if (kvm_irqchip_in_kernel()) {
Error *local_err = NULL; return kvmppc_xive_source_reset_one(xsrc, lisn, errp);
kvmppc_xive_source_reset_one(xsrc, lisn, &local_err);
if (local_err) {
error_report_err(local_err);
return false;
}
} }
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); assert(lisn < xive->nr_irqs);
xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID); xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID);
return true;
} }
/* /*

View File

@ -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 * only need to inform the KVM XIVE device about their type: LSI or
* MSI. * 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); SpaprXive *xive = SPAPR_XIVE(xsrc->xive);
uint64_t state = 0; uint64_t state = 0;
/* The KVM XIVE device is not in use */ /* The KVM XIVE device is not in use */
if (xive->fd == -1) { if (xive->fd == -1) {
return; return -ENODEV;
} }
if (xive_source_irq_is_lsi(xsrc, srcno)) { if (xive_source_irq_is_lsi(xsrc, srcno)) {
@ -249,7 +249,7 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp)
} }
} }
kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state, return kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_SOURCE, srcno, &state,
true, errp); true, errp);
} }

View File

@ -246,7 +246,10 @@ static void spapr_irq_init_xive(SpaprMachineState *spapr, Error **errp)
/* Enable the CPU IPIs */ /* Enable the CPU IPIs */
for (i = 0; i < nr_servers; ++i) { 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); 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, static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi,
Error **errp) Error **errp)
{ {
if (!spapr_xive_irq_claim(spapr->xive, irq, lsi)) { return spapr_xive_irq_claim(spapr->xive, irq, lsi, errp);
error_setg(errp, "IRQ %d is invalid", irq);
return -1;
}
return 0;
} }
static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq)

View File

@ -54,8 +54,8 @@ typedef struct SpaprXive {
*/ */
#define SPAPR_XIVE_BLOCK_ID 0x0 #define SPAPR_XIVE_BLOCK_ID 0x0
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);
bool spapr_xive_irq_free(SpaprXive *xive, uint32_t lisn); void spapr_xive_irq_free(SpaprXive *xive, int lisn);
void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon); void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
int spapr_xive_post_load(SpaprXive *xive, int version_id); int spapr_xive_post_load(SpaprXive *xive, int version_id);

View File

@ -425,7 +425,7 @@ static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx)
* KVM XIVE device helpers * KVM XIVE device helpers
*/ */
void kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp); int kvmppc_xive_source_reset_one(XiveSource *xsrc, int srcno, Error **errp);
void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val); void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val);
void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp); void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp);
void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp); void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp);