From 878b2b48ee49a24488ddf8654a59a568c864c815 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 4 Oct 2019 09:38:50 +0200 Subject: xive: Make some device types not user creatable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some device types of the XIVE model are exposed to the QEMU command line: $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive name "xive-end-source", desc "XIVE END Source" name "xive-source", desc "XIVE Interrupt Source" name "xive-tctx", desc "XIVE Interrupt Thread Context" These are internal devices that shouldn't be instantiable by the user. By the way, they can't be because their respective realize functions expect link properties that can't be set from the command line: qemu-system-ppc64: -device xive-source: required link 'xive' not found: Property '.xive' not found qemu-system-ppc64: -device xive-end-source: required link 'xive' not found: Property '.xive' not found qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found: Property '.cpu' not found Hide them by setting dc->user_creatable to false in their respective class init functions. Signed-off-by: Greg Kurz Message-Id: <157017473006.331610.2983143972519884544.stgit@bahia.lan> Message-Id: <157045578401.865784.6058183726552779559.stgit@bahia.lan> Reviewed-by: Cédric Le Goater [dwg: Folded comment update into base patch] Signed-off-by: David Gibson --- hw/intc/xive.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 29df06df11..453d389848 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -670,6 +670,11 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) dc->realize = xive_tctx_realize; dc->unrealize = xive_tctx_unrealize; dc->vmsd = &vmstate_xive_tctx; + /* + * Reason: part of XIVE interrupt controller, needs to be wired up + * by xive_tctx_create(). + */ + dc->user_creatable = false; } static const TypeInfo xive_tctx_info = { @@ -1118,6 +1123,11 @@ static void xive_source_class_init(ObjectClass *klass, void *data) dc->props = xive_source_properties; dc->realize = xive_source_realize; dc->vmsd = &vmstate_xive_source; + /* + * Reason: part of XIVE interrupt controller, needs to be wired up, + * e.g. by spapr_xive_instance_init(). + */ + dc->user_creatable = false; } static const TypeInfo xive_source_info = { @@ -1853,6 +1863,11 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data) dc->desc = "XIVE END Source"; dc->props = xive_end_source_properties; dc->realize = xive_end_source_realize; + /* + * Reason: part of XIVE interrupt controller, needs to be wired up, + * e.g. by spapr_xive_instance_init(). + */ + dc->user_creatable = false; } static const TypeInfo xive_end_source_info = { -- cgit v1.2.3 From e6144bf912a69b747be43f490a815871dca4f1ed Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Fri, 4 Oct 2019 10:37:47 +0200 Subject: xics: Make some device types not user creatable Some device types of the XICS model are exposed to the QEMU command line: $ ppc64-softmmu/qemu-system-ppc64 -device help | grep ic[sp] name "icp" name "ics" name "ics-spapr" name "pnv-icp", desc "PowerNV ICP" These are internal devices that shouldn't be instantiable by the user. By the way, they can't be because their respective realize functions expect link properties that can't be set from the command line: qemu-system-ppc64: -device icp: required link 'xics' not found: Property '.xics' not found qemu-system-ppc64: -device ics: required link 'xics' not found: Property '.xics' not found qemu-system-ppc64: -device ics-spapr: required link 'xics' not found: Property '.xics' not found qemu-system-ppc64: -device pnv-icp: required link 'xics' not found: Property '.xics' not found Hide them by setting dc->user_creatable to false in the base class "icp" and "ics" init functions. Signed-off-by: Greg Kurz Message-Id: <157017826724.337875.14822177178282524024.stgit@bahia.lan> Message-Id: <157045578962.865784.8551555523533955113.stgit@bahia.lan> [dwg: Folded reason comment into base patch] Signed-off-by: David Gibson --- hw/intc/xics.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index dfe7dbd254..b5ac408f7b 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -369,6 +369,11 @@ static void icp_class_init(ObjectClass *klass, void *data) dc->realize = icp_realize; dc->unrealize = icp_unrealize; + /* + * Reason: part of XICS interrupt controller, needs to be wired up + * by icp_create(). + */ + dc->user_creatable = false; } static const TypeInfo icp_info = { @@ -689,6 +694,11 @@ static void ics_class_init(ObjectClass *klass, void *data) dc->props = ics_properties; dc->reset = ics_reset; dc->vmsd = &vmstate_ics; + /* + * Reason: part of XICS interrupt controller, needs to be wired up, + * e.g. by spapr_irq_init(). + */ + dc->user_creatable = false; } static const TypeInfo ics_info = { -- cgit v1.2.3 From 8d745875c28528a30155bfa0ca992e2202d08b96 Mon Sep 17 00:00:00 2001 From: Stefan Brankovic Date: Fri, 4 Oct 2019 15:43:59 +0200 Subject: target/ppc: Fix for optimized vsl/vsr instructions In previous implementation, invocation of TCG shift function could request shift of TCG variable by 64 bits when variable 'sh' is 0, which is not supported in TCG (values can be shifted by 0 to 63 bits). This patch fixes this by using two separate invocation of TCG shift functions, with maximum shift amount of 32. Name of variable 'shifted' is changed to 'carry' so variable naming is similar to old helper implementation. Variables 'avrA' and 'avrB' are replaced with variable 'avr'. Fixes: 4e6d0920e7547e6af4bbac5ffe9adfe6ea621822 Reported-by: "Paul A. Clark" Reported-by: Mark Cave-Ayland Suggested-by: Aleksandar Markovic Signed-off-by: Stefan Brankovic Message-Id: <1570196639-7025-2-git-send-email-stefan.brankovic@rt-rk.com> Tested-by: Paul A. Clarke Signed-off-by: David Gibson --- target/ppc/translate/vmx-impl.inc.c | 84 ++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c index 2472a5217a..81d5a7a341 100644 --- a/target/ppc/translate/vmx-impl.inc.c +++ b/target/ppc/translate/vmx-impl.inc.c @@ -590,40 +590,38 @@ static void trans_vsl(DisasContext *ctx) int VT = rD(ctx->opcode); int VA = rA(ctx->opcode); int VB = rB(ctx->opcode); - TCGv_i64 avrA = tcg_temp_new_i64(); - TCGv_i64 avrB = tcg_temp_new_i64(); + TCGv_i64 avr = tcg_temp_new_i64(); TCGv_i64 sh = tcg_temp_new_i64(); - TCGv_i64 shifted = tcg_temp_new_i64(); + TCGv_i64 carry = tcg_temp_new_i64(); TCGv_i64 tmp = tcg_temp_new_i64(); - /* Place bits 125-127 of vB in sh. */ - get_avr64(avrB, VB, false); - tcg_gen_andi_i64(sh, avrB, 0x07ULL); + /* Place bits 125-127 of vB in 'sh'. */ + get_avr64(avr, VB, false); + tcg_gen_andi_i64(sh, avr, 0x07ULL); /* - * Save highest sh bits of lower doubleword element of vA in variable - * shifted and perform shift on lower doubleword. + * Save highest 'sh' bits of lower doubleword element of vA in variable + * 'carry' and perform shift on lower doubleword. */ - get_avr64(avrA, VA, false); - tcg_gen_subfi_i64(tmp, 64, sh); - tcg_gen_shr_i64(shifted, avrA, tmp); - tcg_gen_andi_i64(shifted, shifted, 0x7fULL); - tcg_gen_shl_i64(avrA, avrA, sh); - set_avr64(VT, avrA, false); + get_avr64(avr, VA, false); + tcg_gen_subfi_i64(tmp, 32, sh); + tcg_gen_shri_i64(carry, avr, 32); + tcg_gen_shr_i64(carry, carry, tmp); + tcg_gen_shl_i64(avr, avr, sh); + set_avr64(VT, avr, false); /* * Perform shift on higher doubleword element of vA and replace lowest - * sh bits with shifted. + * 'sh' bits with 'carry'. */ - get_avr64(avrA, VA, true); - tcg_gen_shl_i64(avrA, avrA, sh); - tcg_gen_or_i64(avrA, avrA, shifted); - set_avr64(VT, avrA, true); + get_avr64(avr, VA, true); + tcg_gen_shl_i64(avr, avr, sh); + tcg_gen_or_i64(avr, avr, carry); + set_avr64(VT, avr, true); - tcg_temp_free_i64(avrA); - tcg_temp_free_i64(avrB); + tcg_temp_free_i64(avr); tcg_temp_free_i64(sh); - tcg_temp_free_i64(shifted); + tcg_temp_free_i64(carry); tcg_temp_free_i64(tmp); } @@ -639,39 +637,37 @@ static void trans_vsr(DisasContext *ctx) int VT = rD(ctx->opcode); int VA = rA(ctx->opcode); int VB = rB(ctx->opcode); - TCGv_i64 avrA = tcg_temp_new_i64(); - TCGv_i64 avrB = tcg_temp_new_i64(); + TCGv_i64 avr = tcg_temp_new_i64(); TCGv_i64 sh = tcg_temp_new_i64(); - TCGv_i64 shifted = tcg_temp_new_i64(); + TCGv_i64 carry = tcg_temp_new_i64(); TCGv_i64 tmp = tcg_temp_new_i64(); - /* Place bits 125-127 of vB in sh. */ - get_avr64(avrB, VB, false); - tcg_gen_andi_i64(sh, avrB, 0x07ULL); + /* Place bits 125-127 of vB in 'sh'. */ + get_avr64(avr, VB, false); + tcg_gen_andi_i64(sh, avr, 0x07ULL); /* - * Save lowest sh bits of higher doubleword element of vA in variable - * shifted and perform shift on higher doubleword. + * Save lowest 'sh' bits of higher doubleword element of vA in variable + * 'carry' and perform shift on higher doubleword. */ - get_avr64(avrA, VA, true); - tcg_gen_subfi_i64(tmp, 64, sh); - tcg_gen_shl_i64(shifted, avrA, tmp); - tcg_gen_andi_i64(shifted, shifted, 0xfe00000000000000ULL); - tcg_gen_shr_i64(avrA, avrA, sh); - set_avr64(VT, avrA, true); + get_avr64(avr, VA, true); + tcg_gen_subfi_i64(tmp, 32, sh); + tcg_gen_shli_i64(carry, avr, 32); + tcg_gen_shl_i64(carry, carry, tmp); + tcg_gen_shr_i64(avr, avr, sh); + set_avr64(VT, avr, true); /* * Perform shift on lower doubleword element of vA and replace highest - * sh bits with shifted. + * 'sh' bits with 'carry'. */ - get_avr64(avrA, VA, false); - tcg_gen_shr_i64(avrA, avrA, sh); - tcg_gen_or_i64(avrA, avrA, shifted); - set_avr64(VT, avrA, false); + get_avr64(avr, VA, false); + tcg_gen_shr_i64(avr, avr, sh); + tcg_gen_or_i64(avr, avr, carry); + set_avr64(VT, avr, false); - tcg_temp_free_i64(avrA); - tcg_temp_free_i64(avrB); + tcg_temp_free_i64(avr); tcg_temp_free_i64(sh); - tcg_temp_free_i64(shifted); + tcg_temp_free_i64(carry); tcg_temp_free_i64(tmp); } -- cgit v1.2.3 From 106695ab1231c7883a1ea6c543434f5c74476f54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Mon, 7 Oct 2019 10:40:54 +0200 Subject: ppc/pnv: Improve trigger data definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trigger data is used for both triggers of a HW source interrupts, PHB, PSI, and triggers for rerouting interrupts between interrupt controllers. When an interrupt is rerouted, the trigger data follows an "END trigger" format. In that case, the remote IC needs EAS containing an END index to perform a lookup of an END. An END trigger, bit0 of word0 set to '1', is defined as : |0123|4567|0123|4567|0123|4567|0123|4567| W0 E=1 |1P--|BLOC| END IDX | W1 E=1 |M | END DATA | An EAS is defined as : |0123|4567|0123|4567|0123|4567|0123|4567| W0 |V---|BLOC| END IDX | W1 |M | END DATA | The END trigger adds an extra 'PQ' bit, bit1 of word0 set to '1', signaling that the PQ bits have been checked. That bit is unused in the initial EAS definition. When a HW device performs the trigger, the trigger data follows an "EAS trigger" format because the trigger data in that case contains an EAS index which the IC needs to look for. An EAS trigger, bit0 of word0 set to '0', is defined as : |0123|4567|0123|4567|0123|4567|0123|4567| W0 E=0 |0P--|---- ---- ---- ---- ---- ---- ----| W1 E=0 |BLOC| EAS INDEX | There is also a 'PQ' bit, bit1 of word0 to '1', signaling that the PQ bits have been checked. Introduce these new trigger bits and rename the XIVE_SRCNO macros in XIVE_EAS to reflect better the nature of the data. Signed-off-by: Cédric Le Goater Message-Id: <20191007084102.29776-2-clg@kaod.org> Signed-off-by: David Gibson --- hw/intc/pnv_xive.c | 20 ++++++++++++++++---- hw/intc/xive.c | 4 ++-- include/hw/ppc/xive_regs.h | 26 +++++++++++++++++++++++--- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c index ed6e9d71bb..348f2fdd26 100644 --- a/hw/intc/pnv_xive.c +++ b/hw/intc/pnv_xive.c @@ -385,7 +385,7 @@ static int pnv_xive_get_eas(XiveRouter *xrtr, uint8_t blk, uint32_t idx, PnvXive *xive = PNV_XIVE(xrtr); if (pnv_xive_get_ic(blk) != xive) { - xive_error(xive, "VST: EAS %x is remote !?", XIVE_SRCNO(blk, idx)); + xive_error(xive, "VST: EAS %x is remote !?", XIVE_EAS(blk, idx)); return -1; } @@ -431,7 +431,7 @@ static void pnv_xive_notify(XiveNotifier *xn, uint32_t srcno) PnvXive *xive = PNV_XIVE(xn); uint8_t blk = xive->chip->chip_id; - xive_router_notify(xn, XIVE_SRCNO(blk, srcno)); + xive_router_notify(xn, XIVE_EAS(blk, srcno)); } /* @@ -1225,12 +1225,24 @@ static const MemoryRegionOps pnv_xive_ic_reg_ops = { static void pnv_xive_ic_hw_trigger(PnvXive *xive, hwaddr addr, uint64_t val) { + uint8_t blk; + uint32_t idx; + + if (val & XIVE_TRIGGER_END) { + xive_error(xive, "IC: END trigger at @0x%"HWADDR_PRIx" data 0x%"PRIx64, + addr, val); + return; + } + /* * Forward the source event notification directly to the Router. * The source interrupt number should already be correctly encoded * with the chip block id by the sending device (PHB, PSI). */ - xive_router_notify(XIVE_NOTIFIER(xive), val); + blk = XIVE_EAS_BLOCK(val); + idx = XIVE_EAS_INDEX(val); + + xive_router_notify(XIVE_NOTIFIER(xive), XIVE_EAS(blk, idx)); } static void pnv_xive_ic_notify_write(void *opaque, hwaddr addr, uint64_t val, @@ -1566,7 +1578,7 @@ void pnv_xive_pic_print_info(PnvXive *xive, Monitor *mon) { XiveRouter *xrtr = XIVE_ROUTER(xive); uint8_t blk = xive->chip->chip_id; - uint32_t srcno0 = XIVE_SRCNO(blk, 0); + uint32_t srcno0 = XIVE_EAS(blk, 0); uint32_t nr_ipis = pnv_xive_nr_ipis(xive); uint32_t nr_ends = pnv_xive_nr_ends(xive); XiveEAS eas; diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 453d389848..d420c6571e 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1658,8 +1658,8 @@ do_escalation: void xive_router_notify(XiveNotifier *xn, uint32_t lisn) { XiveRouter *xrtr = XIVE_ROUTER(xn); - uint8_t eas_blk = XIVE_SRCNO_BLOCK(lisn); - uint32_t eas_idx = XIVE_SRCNO_INDEX(lisn); + uint8_t eas_blk = XIVE_EAS_BLOCK(lisn); + uint32_t eas_idx = XIVE_EAS_INDEX(lisn); XiveEAS eas; /* EAS cache lookup */ diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h index 08c8bf7172..55307cd153 100644 --- a/include/hw/ppc/xive_regs.h +++ b/include/hw/ppc/xive_regs.h @@ -22,9 +22,29 @@ /* * Interrupt source number encoding on PowerBUS */ -#define XIVE_SRCNO_BLOCK(srcno) (((srcno) >> 28) & 0xf) -#define XIVE_SRCNO_INDEX(srcno) ((srcno) & 0x0fffffff) -#define XIVE_SRCNO(blk, idx) ((uint32_t)(blk) << 28 | (idx)) +/* + * Trigger data definition + * + * The trigger definition is used for triggers both for HW source + * interrupts (PHB, PSI), as well as for rerouting interrupts between + * Interrupt Controller. + * + * HW source controllers set bit0 of word0 to ‘0’ as they provide EAS + * information (EAS block + EAS index) in the 8 byte data and not END + * information, which is use for rerouting interrupts. + * + * bit1 of word0 to ‘1’ signals that the state bit check has been + * performed. + */ +#define XIVE_TRIGGER_END PPC_BIT(0) +#define XIVE_TRIGGER_PQ PPC_BIT(1) + +/* + * QEMU macros to manipulate the trigger payload in native endian + */ +#define XIVE_EAS_BLOCK(n) (((n) >> 28) & 0xf) +#define XIVE_EAS_INDEX(n) ((n) & 0x0fffffff) +#define XIVE_EAS(blk, idx) ((uint32_t)(blk) << 28 | (idx)) #define TM_SHIFT 16 -- cgit v1.2.3 From 06d26eeb47de96c0fa0cc1d5e95124b0a809b3ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Mon, 7 Oct 2019 10:40:55 +0200 Subject: ppc/pnv: Use address_space_stq_be() when triggering an interrupt from PSI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Include the XIVE_TRIGGER_PQ bit in the trigger data which is how hardware signals to the IC that the PQ bits of the interrupt source have been checked. Signed-off-by: Cédric Le Goater Message-Id: <20191007084102.29776-3-clg@kaod.org> Signed-off-by: David Gibson --- hw/ppc/pnv_psi.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c index a997f16bb4..68d0dfacfe 100644 --- a/hw/ppc/pnv_psi.c +++ b/hw/ppc/pnv_psi.c @@ -660,10 +660,19 @@ static void pnv_psi_notify(XiveNotifier *xf, uint32_t srcno) uint32_t offset = (psi->regs[PSIHB_REG(PSIHB9_IVT_OFFSET)] >> PSIHB9_IVT_OFF_SHIFT); - uint64_t lisn = cpu_to_be64(offset + srcno); + uint64_t data = XIVE_TRIGGER_PQ | offset | srcno; + MemTxResult result; - if (valid) { - cpu_physical_memory_write(notify_addr, &lisn, sizeof(lisn)); + if (!valid) { + return; + } + + address_space_stq_be(&address_space_memory, notify_addr, data, + MEMTXATTRS_UNSPECIFIED, &result); + if (result != MEMTX_OK) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: trigger failed @%" + HWADDR_PRIx "\n", __func__, notif_port); + return; } } -- cgit v1.2.3 From 29cb4187497dcc6be80c9cd2a87764be89d940f6 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 3 Oct 2019 14:02:00 +0200 Subject: spapr: Set VSMT to smp_threads by default Support for setting VSMT is available in KVM since linux-4.13. Most distros that support KVM on POWER already have it. It thus seem reasonable enough to have the default machine to set VSMT to smp_threads. This brings contiguous VCPU ids and thus brings their upper bound down to the machine's max_cpus. This is especially useful for XIVE KVM devices, which may thus allocate only one VP descriptor per VCPU. Signed-off-by: Greg Kurz Message-Id: <157010411885.246126.12610015369068227139.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr.c | 7 ++++++- include/hw/ppc/spapr.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 4eb97d3a9b..428b834f30 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2496,6 +2496,7 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx) static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) { MachineState *ms = MACHINE(spapr); + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); Error *local_err = NULL; bool vsmt_user = !!spapr->vsmt; int kvm_smt = kvmppc_smt_threads(); @@ -2522,7 +2523,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) goto out; } /* In this case, spapr->vsmt has been set by the command line */ - } else { + } else if (!smc->smp_threads_vsmt) { /* * Default VSMT value is tricky, because we need it to be as * consistent as possible (for migration), but this requires @@ -2531,6 +2532,8 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp) * overwhelmingly common case in production systems. */ spapr->vsmt = MAX(8, smp_threads); + } else { + spapr->vsmt = smp_threads; } /* KVM: If necessary, set the SMT mode: */ @@ -4438,6 +4441,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) smc->irq = &spapr_irq_dual; smc->dr_phb_enabled = true; smc->linux_pci_probe = true; + smc->smp_threads_vsmt = true; } static const TypeInfo spapr_machine_info = { @@ -4505,6 +4509,7 @@ static void spapr_machine_4_1_class_options(MachineClass *mc) spapr_machine_4_2_class_options(mc); smc->linux_pci_probe = false; + smc->smp_threads_vsmt = false; compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len); compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); } diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index cbd1a4c9f3..2009eb64f9 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -122,6 +122,7 @@ struct SpaprMachineClass { bool broken_host_serial_model; /* present real host info to the guest */ bool pre_4_1_migration; /* don't migrate hpt-max-page-size */ bool linux_pci_probe; + bool smp_threads_vsmt; /* set VSMT to smp_threads by default */ void (*phb_placement)(SpaprMachineState *spapr, uint32_t index, uint64_t *buid, hwaddr *pio, -- cgit v1.2.3 From 150e25f85baa7b7952ddd1bdfd7ff7801213ce51 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 24 Sep 2019 16:25:08 +1000 Subject: spapr, xics, xive: Introduce SpaprInterruptController QOM interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SpaprIrq structure is used to represent ths spapr machine's irq backend. Except that it kind of conflates two concepts: one is the backend proper - a specific interrupt controller that we might or might not be using, the other is the irq configuration which covers the layout of irq space and which interrupt controllers are allowed. This leads to some pretty confusing code paths for the "dual" configuration where its hooks redirect to other SpaprIrq structures depending on the currently active irq controller. To clean this up, we start by introducing a new SpaprInterruptController QOM interface to represent strictly an interrupt controller backend, not counting anything configuration related. We implement this interface in the XICs and XIVE interrupt controllers, and in future we'll move relevant methods from SpaprIrq into it. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 4 ++++ hw/intc/xics_spapr.c | 4 ++++ hw/ppc/spapr_irq.c | 13 +++++++++++++ include/hw/ppc/spapr_irq.h | 14 ++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 04879abf2e..b67e9c3245 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -519,6 +519,10 @@ static const TypeInfo spapr_xive_info = { .instance_init = spapr_xive_instance_init, .instance_size = sizeof(SpaprXive), .class_init = spapr_xive_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_SPAPR_INTC }, + { } + }, }; static void spapr_xive_register_types(void) diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 6e5eb24b3c..4874e6be55 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -343,6 +343,10 @@ static const TypeInfo ics_spapr_info = { .name = TYPE_ICS_SPAPR, .parent = TYPE_ICS, .class_init = ics_spapr_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_SPAPR_INTC }, + { } + }, }; static void xics_spapr_register_types(void) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 457eabe24c..8791dec1ba 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -23,6 +23,12 @@ #include "trace.h" +static const TypeInfo spapr_intc_info = { + .name = TYPE_SPAPR_INTC, + .parent = TYPE_INTERFACE, + .class_size = sizeof(SpaprInterruptControllerClass), +}; + void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis) { spapr->irq_map_nr = nr_msis; @@ -762,3 +768,10 @@ SpaprIrq spapr_irq_xics_legacy = { .set_irq = spapr_irq_set_irq_xics, .init_kvm = spapr_irq_init_kvm_xics, }; + +static void spapr_irq_register_types(void) +{ + type_register_static(&spapr_intc_info); +} + +type_init(spapr_irq_register_types) diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 69a37f608e..b9398e0be3 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -31,6 +31,20 @@ typedef struct SpaprMachineState SpaprMachineState; +typedef struct SpaprInterruptController SpaprInterruptController; + +#define TYPE_SPAPR_INTC "spapr-interrupt-controller" +#define SPAPR_INTC(obj) \ + INTERFACE_CHECK(SpaprInterruptController, (obj), TYPE_SPAPR_INTC) +#define SPAPR_INTC_CLASS(klass) \ + OBJECT_CLASS_CHECK(SpaprInterruptControllerClass, (klass), TYPE_SPAPR_INTC) +#define SPAPR_INTC_GET_CLASS(obj) \ + OBJECT_GET_CLASS(SpaprInterruptControllerClass, (obj), TYPE_SPAPR_INTC) + +typedef struct SpaprInterruptControllerClass { + InterfaceClass parent; +} SpaprInterruptControllerClass; + void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, Error **errp); -- cgit v1.2.3 From ebd6be089b4c87554362b516c3ba530217d3f3db Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 14:11:23 +1000 Subject: spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This method essentially represents code which belongs to the interrupt controller, but needs to be called on all possible intcs, rather than just the currently active one. The "dual" version therefore calls into the xics and xive versions confusingly. Handle this more directly, by making it instead a method on the intc backend, and always calling it on every backend that exists. While we're there, streamline the error reporting a bit. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 25 ++++++++++++++ hw/intc/xics_spapr.c | 18 +++++++++++ hw/ppc/spapr_cpu_core.c | 3 +- hw/ppc/spapr_irq.c | 81 ++++++++++++++-------------------------------- include/hw/ppc/spapr_irq.h | 13 ++++++-- 5 files changed, 79 insertions(+), 61 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index b67e9c3245..9338daba3d 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -495,10 +495,33 @@ static Property spapr_xive_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc, + PowerPCCPU *cpu, Error **errp) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + Object *obj; + SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); + + obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(xive), errp); + if (!obj) { + return -1; + } + + spapr_cpu->tctx = XIVE_TCTX(obj); + + /* + * (TCG) Early setting the OS CAM line for hotplugged CPUs as they + * don't beneficiate from the reset of the XIVE IRQ backend + */ + spapr_xive_set_tctx_os_cam(spapr_cpu->tctx); + return 0; +} + static void spapr_xive_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); XiveRouterClass *xrc = XIVE_ROUTER_CLASS(klass); + SpaprInterruptControllerClass *sicc = SPAPR_INTC_CLASS(klass); dc->desc = "sPAPR XIVE Interrupt Controller"; dc->props = spapr_xive_properties; @@ -511,6 +534,8 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) xrc->get_nvt = spapr_xive_get_nvt; xrc->write_nvt = spapr_xive_write_nvt; xrc->get_tctx = spapr_xive_get_tctx; + + sicc->cpu_intc_create = spapr_xive_cpu_intc_create; } static const TypeInfo spapr_xive_info = { diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 4874e6be55..946311b858 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -330,13 +330,31 @@ void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); } +static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, + PowerPCCPU *cpu, Error **errp) +{ + ICSState *ics = ICS_SPAPR(intc); + Object *obj; + SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); + + obj = icp_create(OBJECT(cpu), TYPE_ICP, ics->xics, errp); + if (!obj) { + return -1; + } + + spapr_cpu->icp = ICP(obj); + return 0; +} + static void ics_spapr_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); ICSStateClass *isc = ICS_CLASS(klass); + SpaprInterruptControllerClass *sicc = SPAPR_INTC_CLASS(klass); device_class_set_parent_realize(dc, ics_spapr_realize, &isc->parent_realize); + sicc->cpu_intc_create = xics_spapr_cpu_intc_create; } static const TypeInfo ics_spapr_info = { diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 1d93de8161..3e4302c7d5 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -237,8 +237,7 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr, qemu_register_reset(spapr_cpu_reset, cpu); spapr_cpu_reset(cpu); - spapr->irq->cpu_intc_create(spapr, cpu, &local_err); - if (local_err) { + if (spapr_irq_cpu_intc_create(spapr, cpu, &local_err) < 0) { goto error_unregister; } diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 8791dec1ba..9cb2fc71ca 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -138,23 +138,6 @@ static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon) ics_pic_print_info(spapr->ics, mon); } -static void spapr_irq_cpu_intc_create_xics(SpaprMachineState *spapr, - PowerPCCPU *cpu, Error **errp) -{ - Error *local_err = NULL; - Object *obj; - SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); - - obj = icp_create(OBJECT(cpu), TYPE_ICP, XICS_FABRIC(spapr), - &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - spapr_cpu->icp = ICP(obj); -} - static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) { if (!kvm_irqchip_in_kernel()) { @@ -203,7 +186,6 @@ SpaprIrq spapr_irq_xics = { .free = spapr_irq_free_xics, .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, - .cpu_intc_create = spapr_irq_cpu_intc_create_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, .set_irq = spapr_irq_set_irq_xics, @@ -239,28 +221,6 @@ static void spapr_irq_print_info_xive(SpaprMachineState *spapr, spapr_xive_pic_print_info(spapr->xive, mon); } -static void spapr_irq_cpu_intc_create_xive(SpaprMachineState *spapr, - PowerPCCPU *cpu, Error **errp) -{ - Error *local_err = NULL; - Object *obj; - SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); - - obj = xive_tctx_create(OBJECT(cpu), XIVE_ROUTER(spapr->xive), &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - spapr_cpu->tctx = XIVE_TCTX(obj); - - /* - * (TCG) Early setting the OS CAM line for hotplugged CPUs as they - * don't beneficiate from the reset of the XIVE IRQ backend - */ - spapr_xive_set_tctx_os_cam(spapr_cpu->tctx); -} - static int spapr_irq_post_load_xive(SpaprMachineState *spapr, int version_id) { return spapr_xive_post_load(spapr->xive, version_id); @@ -316,7 +276,6 @@ SpaprIrq spapr_irq_xive = { .free = spapr_irq_free_xive, .print_info = spapr_irq_print_info_xive, .dt_populate = spapr_dt_xive, - .cpu_intc_create = spapr_irq_cpu_intc_create_xive, .post_load = spapr_irq_post_load_xive, .reset = spapr_irq_reset_xive, .set_irq = spapr_irq_set_irq_xive, @@ -381,20 +340,6 @@ static void spapr_irq_dt_populate_dual(SpaprMachineState *spapr, spapr_irq_current(spapr)->dt_populate(spapr, nr_servers, fdt, phandle); } -static void spapr_irq_cpu_intc_create_dual(SpaprMachineState *spapr, - PowerPCCPU *cpu, Error **errp) -{ - Error *local_err = NULL; - - spapr_irq_xive.cpu_intc_create(spapr, cpu, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - spapr_irq_xics.cpu_intc_create(spapr, cpu, errp); -} - static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id) { /* @@ -460,7 +405,6 @@ SpaprIrq spapr_irq_dual = { .free = spapr_irq_free_dual, .print_info = spapr_irq_print_info_dual, .dt_populate = spapr_irq_dt_populate_dual, - .cpu_intc_create = spapr_irq_cpu_intc_create_dual, .post_load = spapr_irq_post_load_dual, .reset = spapr_irq_reset_dual, .set_irq = spapr_irq_set_irq_dual, @@ -527,6 +471,30 @@ static int spapr_irq_check(SpaprMachineState *spapr, Error **errp) /* * sPAPR IRQ frontend routines for devices */ +#define ALL_INTCS(spapr_) \ + { SPAPR_INTC((spapr_)->ics), SPAPR_INTC((spapr_)->xive), } + +int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, + PowerPCCPU *cpu, Error **errp) +{ + SpaprInterruptController *intcs[] = ALL_INTCS(spapr); + int i; + int rc; + + for (i = 0; i < ARRAY_SIZE(intcs); i++) { + SpaprInterruptController *intc = intcs[i]; + if (intc) { + SpaprInterruptControllerClass *sicc = SPAPR_INTC_GET_CLASS(intc); + rc = sicc->cpu_intc_create(intc, cpu, errp); + if (rc < 0) { + return rc; + } + } + } + + return 0; +} + void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -762,7 +730,6 @@ SpaprIrq spapr_irq_xics_legacy = { .free = spapr_irq_free_xics, .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, - .cpu_intc_create = spapr_irq_cpu_intc_create_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, .set_irq = spapr_irq_set_irq_xics, diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index b9398e0be3..5e641e23c1 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -43,8 +43,19 @@ typedef struct SpaprInterruptController SpaprInterruptController; typedef struct SpaprInterruptControllerClass { InterfaceClass parent; + + /* + * These methods will typically be called on all intcs, active and + * inactive + */ + int (*cpu_intc_create)(SpaprInterruptController *intc, + PowerPCCPU *cpu, Error **errp); } SpaprInterruptControllerClass; +int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, + PowerPCCPU *cpu, Error **errp); + + void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, Error **errp); @@ -61,8 +72,6 @@ typedef struct SpaprIrq { void (*print_info)(SpaprMachineState *spapr, Monitor *mon); void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); - void (*cpu_intc_create)(SpaprMachineState *spapr, PowerPCCPU *cpu, - Error **errp); int (*post_load)(SpaprMachineState *spapr, int version_id); void (*reset)(SpaprMachineState *spapr, Error **errp); void (*set_irq)(void *opaque, int srcno, int val); -- cgit v1.2.3 From 0b0e52b1317f2a51704cbf32047864869763dea3 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 14:31:13 +1000 Subject: spapr, xics, xive: Move irq claim and free from SpaprIrq to SpaprInterruptController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These methods, like cpu_intc_create, really belong to the interrupt controller, but need to be called on all possible intcs. Like cpu_intc_create, therefore, make them methods on the intc and always call it for all existing intcs. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 71 +++++++++++++++------------- hw/intc/xics_spapr.c | 29 ++++++++++++ hw/ppc/spapr_irq.c | 110 +++++++++++++------------------------------- include/hw/ppc/spapr_irq.h | 5 +- include/hw/ppc/spapr_xive.h | 2 - 5 files changed, 102 insertions(+), 115 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 9338daba3d..ff1a175b44 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -487,6 +487,42 @@ static const VMStateDescription vmstate_spapr_xive = { }, }; +static int spapr_xive_claim_irq(SpaprInterruptController *intc, int lisn, + bool lsi, Error **errp) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + 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 + */ + xive->eat[lisn].w |= cpu_to_be64(EAS_VALID | EAS_MASKED); + if (lsi) { + xive_source_irq_set_lsi(xsrc, lisn); + } + + if (kvm_irqchip_in_kernel()) { + return kvmppc_xive_source_reset_one(xsrc, lisn, errp); + } + + return 0; +} + +static void spapr_xive_free_irq(SpaprInterruptController *intc, int lisn) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + assert(lisn < xive->nr_irqs); + + xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID); +} + static Property spapr_xive_properties[] = { DEFINE_PROP_UINT32("nr-irqs", SpaprXive, nr_irqs, 0), DEFINE_PROP_UINT32("nr-ends", SpaprXive, nr_ends, 0), @@ -536,6 +572,8 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) xrc->get_tctx = spapr_xive_get_tctx; sicc->cpu_intc_create = spapr_xive_cpu_intc_create; + sicc->claim_irq = spapr_xive_claim_irq; + sicc->free_irq = spapr_xive_free_irq; } static const TypeInfo spapr_xive_info = { @@ -557,39 +595,6 @@ static void spapr_xive_register_types(void) type_init(spapr_xive_register_types) -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 - */ - xive->eat[lisn].w |= cpu_to_be64(EAS_VALID | EAS_MASKED); - if (lsi) { - xive_source_irq_set_lsi(xsrc, lisn); - } - - if (kvm_irqchip_in_kernel()) { - return kvmppc_xive_source_reset_one(xsrc, lisn, errp); - } - - return 0; -} - -void spapr_xive_irq_free(SpaprXive *xive, int lisn) -{ - assert(lisn < xive->nr_irqs); - - xive->eat[lisn].w &= cpu_to_be64(~EAS_VALID); -} - /* * XIVE hcalls * diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 946311b858..224fe1efcd 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -346,6 +346,33 @@ static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, return 0; } +static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, + bool lsi, Error **errp) +{ + ICSState *ics = ICS_SPAPR(intc); + + assert(ics); + assert(ics_valid_irq(ics, irq)); + + if (!ics_irq_free(ics, irq - ics->offset)) { + error_setg(errp, "IRQ %d is not free", irq); + return -EBUSY; + } + + ics_set_irq_type(ics, irq - ics->offset, lsi); + return 0; +} + +static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) +{ + ICSState *ics = ICS_SPAPR(intc); + uint32_t srcno = irq - ics->offset; + + assert(ics_valid_irq(ics, irq)); + + memset(&ics->irqs[srcno], 0, sizeof(ICSIRQState)); +} + static void ics_spapr_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -355,6 +382,8 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) device_class_set_parent_realize(dc, ics_spapr_realize, &isc->parent_realize); sicc->cpu_intc_create = xics_spapr_cpu_intc_create; + sicc->claim_irq = xics_spapr_claim_irq; + sicc->free_irq = xics_spapr_free_irq; } static const TypeInfo ics_spapr_info = { diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 9cb2fc71ca..83882cfad3 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -98,33 +98,6 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr, * XICS IRQ backend. */ -static int spapr_irq_claim_xics(SpaprMachineState *spapr, int irq, bool lsi, - Error **errp) -{ - ICSState *ics = spapr->ics; - - assert(ics); - assert(ics_valid_irq(ics, irq)); - - if (!ics_irq_free(ics, irq - ics->offset)) { - error_setg(errp, "IRQ %d is not free", irq); - return -1; - } - - ics_set_irq_type(ics, irq - ics->offset, lsi); - return 0; -} - -static void spapr_irq_free_xics(SpaprMachineState *spapr, int irq) -{ - ICSState *ics = spapr->ics; - uint32_t srcno = irq - ics->offset; - - assert(ics_valid_irq(ics, irq)); - - memset(&ics->irqs[srcno], 0, sizeof(ICSIRQState)); -} - static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon) { CPUState *cs; @@ -182,8 +155,6 @@ SpaprIrq spapr_irq_xics = { .xics = true, .xive = false, - .claim = spapr_irq_claim_xics, - .free = spapr_irq_free_xics, .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, @@ -196,17 +167,6 @@ SpaprIrq spapr_irq_xics = { * XIVE IRQ backend. */ -static int spapr_irq_claim_xive(SpaprMachineState *spapr, int irq, bool lsi, - Error **errp) -{ - return spapr_xive_irq_claim(spapr->xive, irq, lsi, errp); -} - -static void spapr_irq_free_xive(SpaprMachineState *spapr, int irq) -{ - spapr_xive_irq_free(spapr->xive, irq); -} - static void spapr_irq_print_info_xive(SpaprMachineState *spapr, Monitor *mon) { @@ -272,8 +232,6 @@ SpaprIrq spapr_irq_xive = { .xics = false, .xive = true, - .claim = spapr_irq_claim_xive, - .free = spapr_irq_free_xive, .print_info = spapr_irq_print_info_xive, .dt_populate = spapr_dt_xive, .post_load = spapr_irq_post_load_xive, @@ -301,33 +259,6 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) &spapr_irq_xive : &spapr_irq_xics; } -static int spapr_irq_claim_dual(SpaprMachineState *spapr, int irq, bool lsi, - Error **errp) -{ - Error *local_err = NULL; - int ret; - - ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return ret; - } - - ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return ret; - } - - return ret; -} - -static void spapr_irq_free_dual(SpaprMachineState *spapr, int irq) -{ - spapr_irq_xics.free(spapr, irq); - spapr_irq_xive.free(spapr, irq); -} - static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon) { spapr_irq_current(spapr)->print_info(spapr, mon); @@ -401,8 +332,6 @@ SpaprIrq spapr_irq_dual = { .xics = true, .xive = true, - .claim = spapr_irq_claim_dual, - .free = spapr_irq_free_dual, .print_info = spapr_irq_print_info_dual, .dt_populate = spapr_irq_dt_populate_dual, .post_load = spapr_irq_post_load_dual, @@ -572,8 +501,11 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) /* Enable the CPU IPIs */ for (i = 0; i < nr_servers; ++i) { - if (spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, - false, errp) < 0) { + SpaprInterruptControllerClass *sicc + = SPAPR_INTC_GET_CLASS(spapr->xive); + + if (sicc->claim_irq(SPAPR_INTC(spapr->xive), SPAPR_IRQ_IPI + i, + false, errp) < 0) { return; } } @@ -587,21 +519,45 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { + SpaprInterruptController *intcs[] = ALL_INTCS(spapr); + int i; + int rc; + assert(irq >= SPAPR_XIRQ_BASE); assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); - return spapr->irq->claim(spapr, irq, lsi, errp); + for (i = 0; i < ARRAY_SIZE(intcs); i++) { + SpaprInterruptController *intc = intcs[i]; + if (intc) { + SpaprInterruptControllerClass *sicc = SPAPR_INTC_GET_CLASS(intc); + rc = sicc->claim_irq(intc, irq, lsi, errp); + if (rc < 0) { + return rc; + } + } + } + + return 0; } void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) { - int i; + SpaprInterruptController *intcs[] = ALL_INTCS(spapr); + int i, j; assert(irq >= SPAPR_XIRQ_BASE); assert((irq + num) <= (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); for (i = irq; i < (irq + num); i++) { - spapr->irq->free(spapr, i); + for (j = 0; j < ARRAY_SIZE(intcs); j++) { + SpaprInterruptController *intc = intcs[j]; + + if (intc) { + SpaprInterruptControllerClass *sicc + = SPAPR_INTC_GET_CLASS(intc); + sicc->free_irq(intc, i); + } + } } } @@ -726,8 +682,6 @@ SpaprIrq spapr_irq_xics_legacy = { .xics = true, .xive = false, - .claim = spapr_irq_claim_xics, - .free = spapr_irq_free_xics, .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 5e641e23c1..adfef0fcbe 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -50,6 +50,9 @@ typedef struct SpaprInterruptControllerClass { */ int (*cpu_intc_create)(SpaprInterruptController *intc, PowerPCCPU *cpu, Error **errp); + int (*claim_irq)(SpaprInterruptController *intc, int irq, bool lsi, + Error **errp); + void (*free_irq)(SpaprInterruptController *intc, int irq); } SpaprInterruptControllerClass; int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, @@ -67,8 +70,6 @@ typedef struct SpaprIrq { bool xics; bool xive; - int (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error **errp); - void (*free)(SpaprMachineState *spapr, int irq); void (*print_info)(SpaprMachineState *spapr, Monitor *mon); void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index 0df20a6590..8f875673f5 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -54,8 +54,6 @@ typedef struct SpaprXive { */ #define SPAPR_XIVE_BLOCK_ID 0x0 -int spapr_xive_irq_claim(SpaprXive *xive, int lisn, bool lsi, Error **errp); -void spapr_xive_irq_free(SpaprXive *xive, int lisn); void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon); int spapr_xive_post_load(SpaprXive *xive, int version_id); -- cgit v1.2.3 From 81106ddd1aee5c06e390eaffb07f857f925628f4 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 15:41:39 +1000 Subject: spapr: Formalize notion of active interrupt controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spapr now has the mechanism of constructing both XICS and XIVE instances of the SpaprInterruptController interface. However, only one of the interrupt controllers will actually be active at any given time, depending on feature negotiation with the guest. This is handled in the current code via spapr_irq_current() which checks the OV5 vector from feature negotiation to determine the current backend. Determining the active controller at the point we need it like this can be pretty confusing, because it makes it very non obvious at what points the active controller can change. This can make it difficult to reason about the code and where a change of active controller could appear in sequence with other events. Make this mechanism more explicit by adding an 'active_intc' pointer and an explicit spapr_irq_update_active_intc() function to update it from the CAS state. We also add hooks on the intc backend which will get called when it is activated or deactivated. For now we just introduce the switch and hooks, later patches will actually start using them. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr_irq.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 5 +++-- include/hw/ppc/spapr_irq.h | 5 +++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 83882cfad3..249a2688ac 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -586,6 +586,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) int spapr_irq_post_load(SpaprMachineState *spapr, int version_id) { + spapr_irq_update_active_intc(spapr); return spapr->irq->post_load(spapr, version_id); } @@ -593,6 +594,8 @@ void spapr_irq_reset(SpaprMachineState *spapr, Error **errp) { assert(!spapr->irq_map || bitmap_empty(spapr->irq_map, spapr->irq_map_nr)); + spapr_irq_update_active_intc(spapr); + if (spapr->irq->reset) { spapr->irq->reset(spapr, errp); } @@ -619,6 +622,54 @@ int spapr_irq_get_phandle(SpaprMachineState *spapr, void *fdt, Error **errp) return phandle; } +static void set_active_intc(SpaprMachineState *spapr, + SpaprInterruptController *new_intc) +{ + SpaprInterruptControllerClass *sicc; + + assert(new_intc); + + if (new_intc == spapr->active_intc) { + /* Nothing to do */ + return; + } + + if (spapr->active_intc) { + sicc = SPAPR_INTC_GET_CLASS(spapr->active_intc); + if (sicc->deactivate) { + sicc->deactivate(spapr->active_intc); + } + } + + sicc = SPAPR_INTC_GET_CLASS(new_intc); + if (sicc->activate) { + sicc->activate(new_intc, &error_fatal); + } + + spapr->active_intc = new_intc; +} + +void spapr_irq_update_active_intc(SpaprMachineState *spapr) +{ + SpaprInterruptController *new_intc; + + if (!spapr->ics) { + /* + * XXX before we run CAS, ov5_cas is initialized empty, which + * indicates XICS, even if we have ic-mode=xive. TODO: clean + * up the CAS path so that we have a clearer way of handling + * this. + */ + new_intc = SPAPR_INTC(spapr->xive); + } else if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) { + new_intc = SPAPR_INTC(spapr->xive); + } else { + new_intc = SPAPR_INTC(spapr->ics); + } + + set_active_intc(spapr, new_intc); +} + /* * XICS legacy routines - to deprecate one day */ diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 2009eb64f9..3b34cf5207 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -144,7 +144,6 @@ struct SpaprMachineState { struct SpaprVioBus *vio_bus; QLIST_HEAD(, SpaprPhbState) phbs; struct SpaprNvram *nvram; - ICSState *ics; SpaprRtcState rtc; SpaprResizeHpt resize_hpt; @@ -196,9 +195,11 @@ struct SpaprMachineState { int32_t irq_map_nr; unsigned long *irq_map; - SpaprXive *xive; SpaprIrq *irq; qemu_irq *qirqs; + SpaprInterruptController *active_intc; + ICSState *ics; + SpaprXive *xive; bool cmd_line_caps[SPAPR_CAP_NUM]; SpaprCapabilities def, eff, mig; diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index adfef0fcbe..593059eff5 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -44,6 +44,9 @@ typedef struct SpaprInterruptController SpaprInterruptController; typedef struct SpaprInterruptControllerClass { InterfaceClass parent; + int (*activate)(SpaprInterruptController *intc, Error **errp); + void (*deactivate)(SpaprInterruptController *intc); + /* * These methods will typically be called on all intcs, active and * inactive @@ -55,6 +58,8 @@ typedef struct SpaprInterruptControllerClass { void (*free_irq)(SpaprInterruptController *intc, int irq); } SpaprInterruptControllerClass; +void spapr_irq_update_active_intc(SpaprMachineState *spapr); + int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, PowerPCCPU *cpu, Error **errp); -- cgit v1.2.3 From 7bcdbcca2f498b8c93f33241488305a3694a941c Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 16:09:46 +1000 Subject: spapr, xics, xive: Move set_irq from SpaprIrq to SpaprInterruptController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This method depends only on the active irq controller. Now that we've formalized the notion of active controller we can dispatch directly through that, rather than dispatching via SpaprIrq with the dual version having to do a second conditional dispatch. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 12 ++++++++++++ hw/intc/xics_spapr.c | 9 +++++++++ hw/ppc/spapr_irq.c | 41 ++++++++++------------------------------- include/hw/ppc/spapr_irq.h | 4 +++- 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index ff1a175b44..52d5e71793 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -553,6 +553,17 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc, return 0; } +static void spapr_xive_set_irq(SpaprInterruptController *intc, int irq, int val) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + + if (kvm_irqchip_in_kernel()) { + kvmppc_xive_source_set_irq(&xive->source, irq, val); + } else { + xive_source_set_irq(&xive->source, irq, val); + } +} + static void spapr_xive_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -574,6 +585,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) sicc->cpu_intc_create = spapr_xive_cpu_intc_create; sicc->claim_irq = spapr_xive_claim_irq; sicc->free_irq = spapr_xive_free_irq; + sicc->set_irq = spapr_xive_set_irq; } static const TypeInfo spapr_xive_info = { diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 224fe1efcd..02372697f6 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -373,6 +373,14 @@ static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) memset(&ics->irqs[srcno], 0, sizeof(ICSIRQState)); } +static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) +{ + ICSState *ics = ICS_SPAPR(intc); + uint32_t srcno = irq - ics->offset; + + ics_set_irq(ics, srcno, val); +} + static void ics_spapr_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -384,6 +392,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) sicc->cpu_intc_create = xics_spapr_cpu_intc_create; sicc->claim_irq = xics_spapr_claim_irq; sicc->free_irq = xics_spapr_free_irq; + sicc->set_irq = xics_spapr_set_irq; } static const TypeInfo ics_spapr_info = { diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 249a2688ac..bfccb815ed 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -123,14 +123,6 @@ static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) return 0; } -static void spapr_irq_set_irq_xics(void *opaque, int irq, int val) -{ - SpaprMachineState *spapr = opaque; - uint32_t srcno = irq - spapr->ics->offset; - - ics_set_irq(spapr->ics, srcno, val); -} - static void spapr_irq_reset_xics(SpaprMachineState *spapr, Error **errp) { Error *local_err = NULL; @@ -159,7 +151,6 @@ SpaprIrq spapr_irq_xics = { .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, - .set_irq = spapr_irq_set_irq_xics, .init_kvm = spapr_irq_init_kvm_xics, }; @@ -208,17 +199,6 @@ static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) spapr_xive_mmio_set_enabled(spapr->xive, true); } -static void spapr_irq_set_irq_xive(void *opaque, int irq, int val) -{ - SpaprMachineState *spapr = opaque; - - if (kvm_irqchip_in_kernel()) { - kvmppc_xive_source_set_irq(&spapr->xive->source, irq, val); - } else { - xive_source_set_irq(&spapr->xive->source, irq, val); - } -} - static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp) { if (kvm_enabled()) { @@ -236,7 +216,6 @@ SpaprIrq spapr_irq_xive = { .dt_populate = spapr_dt_xive, .post_load = spapr_irq_post_load_xive, .reset = spapr_irq_reset_xive, - .set_irq = spapr_irq_set_irq_xive, .init_kvm = spapr_irq_init_kvm_xive, }; @@ -316,13 +295,6 @@ static void spapr_irq_reset_dual(SpaprMachineState *spapr, Error **errp) spapr_irq_current(spapr)->reset(spapr, errp); } -static void spapr_irq_set_irq_dual(void *opaque, int irq, int val) -{ - SpaprMachineState *spapr = opaque; - - spapr_irq_current(spapr)->set_irq(spapr, irq, val); -} - /* * Define values in sync with the XIVE and XICS backend */ @@ -336,7 +308,6 @@ SpaprIrq spapr_irq_dual = { .dt_populate = spapr_irq_dt_populate_dual, .post_load = spapr_irq_post_load_dual, .reset = spapr_irq_reset_dual, - .set_irq = spapr_irq_set_irq_dual, .init_kvm = NULL, /* should not be used */ }; @@ -424,6 +395,15 @@ int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, return 0; } +static void spapr_set_irq(void *opaque, int irq, int level) +{ + SpaprMachineState *spapr = SPAPR_MACHINE(opaque); + SpaprInterruptControllerClass *sicc + = SPAPR_INTC_GET_CLASS(spapr->active_intc); + + sicc->set_irq(spapr->active_intc, irq, level); +} + void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -513,7 +493,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) spapr_xive_hcall_init(spapr); } - spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr, + spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr, spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); } @@ -737,7 +717,6 @@ SpaprIrq spapr_irq_xics_legacy = { .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, - .set_irq = spapr_irq_set_irq_xics, .init_kvm = spapr_irq_init_kvm_xics, }; diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 593059eff5..ece8d2ea48 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -56,6 +56,9 @@ typedef struct SpaprInterruptControllerClass { int (*claim_irq)(SpaprInterruptController *intc, int irq, bool lsi, Error **errp); void (*free_irq)(SpaprInterruptController *intc, int irq); + + /* These methods should only be called on the active intc */ + void (*set_irq)(SpaprInterruptController *intc, int irq, int val); } SpaprInterruptControllerClass; void spapr_irq_update_active_intc(SpaprMachineState *spapr); @@ -80,7 +83,6 @@ typedef struct SpaprIrq { void *fdt, uint32_t phandle); int (*post_load)(SpaprMachineState *spapr, int version_id); void (*reset)(SpaprMachineState *spapr, Error **errp); - void (*set_irq)(void *opaque, int srcno, int val); void (*init_kvm)(SpaprMachineState *spapr, Error **errp); } SpaprIrq; -- cgit v1.2.3 From 328d8eb24db8ec415260ee7243adf2e3d7e81bad Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 16:12:05 +1000 Subject: spapr, xics, xive: Move print_info from SpaprIrq to SpaprInterruptController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This method depends only on the active irq controller. Now that we've formalized the notion of active controller we can dispatch directly through that, rather than dispatching via SpaprIrq with the dual version having to do a second conditional dispatch. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 15 +++++++++++++++ hw/intc/xics_spapr.c | 15 +++++++++++++++ hw/ppc/spapr.c | 2 +- hw/ppc/spapr_irq.c | 44 ++++++++------------------------------------ include/hw/ppc/spapr_irq.h | 4 ++-- 5 files changed, 41 insertions(+), 39 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 52d5e71793..700ec5c9c1 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -564,6 +564,20 @@ static void spapr_xive_set_irq(SpaprInterruptController *intc, int irq, int val) } } +static void spapr_xive_print_info(SpaprInterruptController *intc, Monitor *mon) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + CPUState *cs; + + CPU_FOREACH(cs) { + PowerPCCPU *cpu = POWERPC_CPU(cs); + + xive_tctx_pic_print_info(spapr_cpu_state(cpu)->tctx, mon); + } + + spapr_xive_pic_print_info(xive, mon); +} + static void spapr_xive_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -586,6 +600,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) sicc->claim_irq = spapr_xive_claim_irq; sicc->free_irq = spapr_xive_free_irq; sicc->set_irq = spapr_xive_set_irq; + sicc->print_info = spapr_xive_print_info; } static const TypeInfo spapr_xive_info = { diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 02372697f6..415defe394 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -381,6 +381,20 @@ static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) ics_set_irq(ics, srcno, val); } +static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) +{ + ICSState *ics = ICS_SPAPR(intc); + CPUState *cs; + + CPU_FOREACH(cs) { + PowerPCCPU *cpu = POWERPC_CPU(cs); + + icp_pic_print_info(spapr_cpu_state(cpu)->icp, mon); + } + + ics_pic_print_info(ics, mon); +} + static void ics_spapr_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -393,6 +407,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) sicc->claim_irq = xics_spapr_claim_irq; sicc->free_irq = xics_spapr_free_irq; sicc->set_irq = xics_spapr_set_irq; + sicc->print_info = xics_spapr_print_info; } static const TypeInfo ics_spapr_info = { diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 428b834f30..24fe12b244 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4266,7 +4266,7 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj, { SpaprMachineState *spapr = SPAPR_MACHINE(obj); - spapr->irq->print_info(spapr, mon); + spapr_irq_print_info(spapr, mon); monitor_printf(mon, "irqchip: %s\n", kvm_irqchip_in_kernel() ? "in-kernel" : "emulated"); } diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index bfccb815ed..a29b527232 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -98,19 +98,6 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr, * XICS IRQ backend. */ -static void spapr_irq_print_info_xics(SpaprMachineState *spapr, Monitor *mon) -{ - CPUState *cs; - - CPU_FOREACH(cs) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - - icp_pic_print_info(spapr_cpu_state(cpu)->icp, mon); - } - - ics_pic_print_info(spapr->ics, mon); -} - static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) { if (!kvm_irqchip_in_kernel()) { @@ -147,7 +134,6 @@ SpaprIrq spapr_irq_xics = { .xics = true, .xive = false, - .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, @@ -158,20 +144,6 @@ SpaprIrq spapr_irq_xics = { * XIVE IRQ backend. */ -static void spapr_irq_print_info_xive(SpaprMachineState *spapr, - Monitor *mon) -{ - CPUState *cs; - - CPU_FOREACH(cs) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - - xive_tctx_pic_print_info(spapr_cpu_state(cpu)->tctx, mon); - } - - spapr_xive_pic_print_info(spapr->xive, mon); -} - static int spapr_irq_post_load_xive(SpaprMachineState *spapr, int version_id) { return spapr_xive_post_load(spapr->xive, version_id); @@ -212,7 +184,6 @@ SpaprIrq spapr_irq_xive = { .xics = false, .xive = true, - .print_info = spapr_irq_print_info_xive, .dt_populate = spapr_dt_xive, .post_load = spapr_irq_post_load_xive, .reset = spapr_irq_reset_xive, @@ -238,11 +209,6 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) &spapr_irq_xive : &spapr_irq_xics; } -static void spapr_irq_print_info_dual(SpaprMachineState *spapr, Monitor *mon) -{ - spapr_irq_current(spapr)->print_info(spapr, mon); -} - static void spapr_irq_dt_populate_dual(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle) @@ -304,7 +270,6 @@ SpaprIrq spapr_irq_dual = { .xics = true, .xive = true, - .print_info = spapr_irq_print_info_dual, .dt_populate = spapr_irq_dt_populate_dual, .post_load = spapr_irq_post_load_dual, .reset = spapr_irq_reset_dual, @@ -404,6 +369,14 @@ static void spapr_set_irq(void *opaque, int irq, int level) sicc->set_irq(spapr->active_intc, irq, level); } +void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon) +{ + SpaprInterruptControllerClass *sicc + = SPAPR_INTC_GET_CLASS(spapr->active_intc); + + sicc->print_info(spapr->active_intc, mon); +} + void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -713,7 +686,6 @@ SpaprIrq spapr_irq_xics_legacy = { .xics = true, .xive = false, - .print_info = spapr_irq_print_info_xics, .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index ece8d2ea48..bdfeb3b107 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -59,13 +59,14 @@ typedef struct SpaprInterruptControllerClass { /* These methods should only be called on the active intc */ void (*set_irq)(SpaprInterruptController *intc, int irq, int val); + void (*print_info)(SpaprInterruptController *intc, Monitor *mon); } SpaprInterruptControllerClass; void spapr_irq_update_active_intc(SpaprMachineState *spapr); int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, PowerPCCPU *cpu, Error **errp); - +void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon); void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, @@ -78,7 +79,6 @@ typedef struct SpaprIrq { bool xics; bool xive; - void (*print_info)(SpaprMachineState *spapr, Monitor *mon); void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); int (*post_load)(SpaprMachineState *spapr, int version_id); -- cgit v1.2.3 From 05289273c06de4bc6ece85a8bf672e588e34f36b Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 30 Sep 2019 12:35:06 +1000 Subject: spapr, xics, xive: Move dt_populate from SpaprIrq to SpaprInterruptController MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This method depends only on the active irq controller. Now that we've formalized the notion of active controller we can dispatch directly through that, rather than dispatching via SpaprIrq with the dual version having to do a second conditional dispatch. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 125 ++++++++++++++++++++++---------------------- hw/intc/xics_spapr.c | 5 +- hw/ppc/spapr.c | 3 +- hw/ppc/spapr_irq.c | 20 ++++--- include/hw/ppc/spapr_irq.h | 6 ++- include/hw/ppc/spapr_xive.h | 2 - include/hw/ppc/xics_spapr.h | 2 - 7 files changed, 80 insertions(+), 83 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 700ec5c9c1..37ffb74ca5 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -578,6 +578,68 @@ static void spapr_xive_print_info(SpaprInterruptController *intc, Monitor *mon) spapr_xive_pic_print_info(xive, mon); } +static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers, + void *fdt, uint32_t phandle) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + int node; + uint64_t timas[2 * 2]; + /* Interrupt number ranges for the IPIs */ + uint32_t lisn_ranges[] = { + cpu_to_be32(0), + cpu_to_be32(nr_servers), + }; + /* + * EQ size - the sizes of pages supported by the system 4K, 64K, + * 2M, 16M. We only advertise 64K for the moment. + */ + uint32_t eq_sizes[] = { + cpu_to_be32(16), /* 64K */ + }; + /* + * The following array is in sync with the reserved priorities + * defined by the 'spapr_xive_priority_is_reserved' routine. + */ + uint32_t plat_res_int_priorities[] = { + cpu_to_be32(7), /* start */ + cpu_to_be32(0xf8), /* count */ + }; + + /* Thread Interrupt Management Area : User (ring 3) and OS (ring 2) */ + timas[0] = cpu_to_be64(xive->tm_base + + XIVE_TM_USER_PAGE * (1ull << TM_SHIFT)); + timas[1] = cpu_to_be64(1ull << TM_SHIFT); + timas[2] = cpu_to_be64(xive->tm_base + + XIVE_TM_OS_PAGE * (1ull << TM_SHIFT)); + timas[3] = cpu_to_be64(1ull << TM_SHIFT); + + _FDT(node = fdt_add_subnode(fdt, 0, xive->nodename)); + + _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); + _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); + + _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); + _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, + sizeof(eq_sizes))); + _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, + sizeof(lisn_ranges))); + + /* For Linux to link the LSIs to the interrupt controller. */ + _FDT(fdt_setprop(fdt, node, "interrupt-controller", NULL, 0)); + _FDT(fdt_setprop_cell(fdt, node, "#interrupt-cells", 2)); + + /* For SLOF */ + _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); + _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); + + /* + * The "ibm,plat-res-int-priorities" property defines the priority + * ranges reserved by the hypervisor + */ + _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities", + plat_res_int_priorities, sizeof(plat_res_int_priorities))); +} + static void spapr_xive_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -601,6 +663,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) sicc->free_irq = spapr_xive_free_irq; sicc->set_irq = spapr_xive_set_irq; sicc->print_info = spapr_xive_print_info; + sicc->dt = spapr_xive_dt; } static const TypeInfo spapr_xive_info = { @@ -1601,65 +1664,3 @@ void spapr_xive_hcall_init(SpaprMachineState *spapr) spapr_register_hypercall(H_INT_SYNC, h_int_sync); spapr_register_hypercall(H_INT_RESET, h_int_reset); } - -void spapr_dt_xive(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, - uint32_t phandle) -{ - SpaprXive *xive = spapr->xive; - int node; - uint64_t timas[2 * 2]; - /* Interrupt number ranges for the IPIs */ - uint32_t lisn_ranges[] = { - cpu_to_be32(0), - cpu_to_be32(nr_servers), - }; - /* - * EQ size - the sizes of pages supported by the system 4K, 64K, - * 2M, 16M. We only advertise 64K for the moment. - */ - uint32_t eq_sizes[] = { - cpu_to_be32(16), /* 64K */ - }; - /* - * The following array is in sync with the reserved priorities - * defined by the 'spapr_xive_priority_is_reserved' routine. - */ - uint32_t plat_res_int_priorities[] = { - cpu_to_be32(7), /* start */ - cpu_to_be32(0xf8), /* count */ - }; - - /* Thread Interrupt Management Area : User (ring 3) and OS (ring 2) */ - timas[0] = cpu_to_be64(xive->tm_base + - XIVE_TM_USER_PAGE * (1ull << TM_SHIFT)); - timas[1] = cpu_to_be64(1ull << TM_SHIFT); - timas[2] = cpu_to_be64(xive->tm_base + - XIVE_TM_OS_PAGE * (1ull << TM_SHIFT)); - timas[3] = cpu_to_be64(1ull << TM_SHIFT); - - _FDT(node = fdt_add_subnode(fdt, 0, xive->nodename)); - - _FDT(fdt_setprop_string(fdt, node, "device_type", "power-ivpe")); - _FDT(fdt_setprop(fdt, node, "reg", timas, sizeof(timas))); - - _FDT(fdt_setprop_string(fdt, node, "compatible", "ibm,power-ivpe")); - _FDT(fdt_setprop(fdt, node, "ibm,xive-eq-sizes", eq_sizes, - sizeof(eq_sizes))); - _FDT(fdt_setprop(fdt, node, "ibm,xive-lisn-ranges", lisn_ranges, - sizeof(lisn_ranges))); - - /* For Linux to link the LSIs to the interrupt controller. */ - _FDT(fdt_setprop(fdt, node, "interrupt-controller", NULL, 0)); - _FDT(fdt_setprop_cell(fdt, node, "#interrupt-cells", 2)); - - /* For SLOF */ - _FDT(fdt_setprop_cell(fdt, node, "linux,phandle", phandle)); - _FDT(fdt_setprop_cell(fdt, node, "phandle", phandle)); - - /* - * The "ibm,plat-res-int-priorities" property defines the priority - * ranges reserved by the hypervisor - */ - _FDT(fdt_setprop(fdt, 0, "ibm,plat-res-int-priorities", - plat_res_int_priorities, sizeof(plat_res_int_priorities))); -} diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 415defe394..4eabafc7e1 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -308,8 +308,8 @@ static void ics_spapr_realize(DeviceState *dev, Error **errp) spapr_register_hypercall(H_IPOLL, h_ipoll); } -void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, - uint32_t phandle) +static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, + void *fdt, uint32_t phandle) { uint32_t interrupt_server_ranges_prop[] = { 0, cpu_to_be32(nr_servers), @@ -408,6 +408,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) sicc->free_irq = xics_spapr_free_irq; sicc->set_irq = xics_spapr_set_irq; sicc->print_info = xics_spapr_print_info; + sicc->dt = xics_spapr_dt; } static const TypeInfo ics_spapr_info = { diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 24fe12b244..c9623600c2 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1247,8 +1247,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr) _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2)); /* /interrupt controller */ - spapr->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt, - PHANDLE_INTC); + spapr_irq_dt(spapr, spapr_max_server_number(spapr), fdt, PHANDLE_INTC); ret = spapr_populate_memory(spapr, fdt); if (ret < 0) { diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index a29b527232..a8005072e6 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -134,7 +134,6 @@ SpaprIrq spapr_irq_xics = { .xics = true, .xive = false, - .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, .init_kvm = spapr_irq_init_kvm_xics, @@ -184,7 +183,6 @@ SpaprIrq spapr_irq_xive = { .xics = false, .xive = true, - .dt_populate = spapr_dt_xive, .post_load = spapr_irq_post_load_xive, .reset = spapr_irq_reset_xive, .init_kvm = spapr_irq_init_kvm_xive, @@ -209,13 +207,6 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) &spapr_irq_xive : &spapr_irq_xics; } -static void spapr_irq_dt_populate_dual(SpaprMachineState *spapr, - uint32_t nr_servers, void *fdt, - uint32_t phandle) -{ - spapr_irq_current(spapr)->dt_populate(spapr, nr_servers, fdt, phandle); -} - static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id) { /* @@ -270,7 +261,6 @@ SpaprIrq spapr_irq_dual = { .xics = true, .xive = true, - .dt_populate = spapr_irq_dt_populate_dual, .post_load = spapr_irq_post_load_dual, .reset = spapr_irq_reset_dual, .init_kvm = NULL, /* should not be used */ @@ -377,6 +367,15 @@ void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon) sicc->print_info(spapr->active_intc, mon); } +void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, + void *fdt, uint32_t phandle) +{ + SpaprInterruptControllerClass *sicc + = SPAPR_INTC_GET_CLASS(spapr->active_intc); + + sicc->dt(spapr->active_intc, nr_servers, fdt, phandle); +} + void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -686,7 +685,6 @@ SpaprIrq spapr_irq_xics_legacy = { .xics = true, .xive = false, - .dt_populate = spapr_dt_xics, .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, .init_kvm = spapr_irq_init_kvm_xics, diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index bdfeb3b107..0df95e1b5a 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -60,6 +60,8 @@ typedef struct SpaprInterruptControllerClass { /* These methods should only be called on the active intc */ void (*set_irq)(SpaprInterruptController *intc, int irq, int val); void (*print_info)(SpaprInterruptController *intc, Monitor *mon); + void (*dt)(SpaprInterruptController *intc, uint32_t nr_servers, + void *fdt, uint32_t phandle); } SpaprInterruptControllerClass; void spapr_irq_update_active_intc(SpaprMachineState *spapr); @@ -67,6 +69,8 @@ void spapr_irq_update_active_intc(SpaprMachineState *spapr); int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, PowerPCCPU *cpu, Error **errp); void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon); +void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, + void *fdt, uint32_t phandle); void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, @@ -79,8 +83,6 @@ typedef struct SpaprIrq { bool xics; bool xive; - void (*dt_populate)(SpaprMachineState *spapr, uint32_t nr_servers, - void *fdt, uint32_t phandle); int (*post_load)(SpaprMachineState *spapr, int version_id); void (*reset)(SpaprMachineState *spapr, Error **errp); void (*init_kvm)(SpaprMachineState *spapr, Error **errp); diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index 8f875673f5..ebe156eb30 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -58,8 +58,6 @@ void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon); int spapr_xive_post_load(SpaprXive *xive, int version_id); void spapr_xive_hcall_init(SpaprMachineState *spapr); -void spapr_dt_xive(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, - uint32_t phandle); void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx); void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable); void spapr_xive_map_mmio(SpaprXive *xive); diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h index 0b35e85c26..8e4fb6adce 100644 --- a/include/hw/ppc/xics_spapr.h +++ b/include/hw/ppc/xics_spapr.h @@ -32,8 +32,6 @@ #define TYPE_ICS_SPAPR "ics-spapr" #define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) -void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, - uint32_t phandle); int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); -- cgit v1.2.3 From 98a39a7927b510fcdd29f8237b67368a66121c84 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 23:23:51 +1000 Subject: spapr, xics, xive: Match signatures for XICS and XIVE KVM connect routines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both XICS and XIVE have routines to connect and disconnect KVM with similar but not identical signatures. This adjusts them to match exactly, which will be useful for further cleanups later. While we're there, we add an explicit return value to the connect path to streamline error reporting in the callers. We remove error reporting the disconnect path. In the XICS case this wasn't used at all. In the XIVE case the only error case was if the KVM device was set up, but KVM didn't have the capability to do so which is pretty obviously impossible. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive_kvm.c | 22 ++++++++++------------ hw/intc/xics_kvm.c | 9 +++++---- hw/ppc/spapr_irq.c | 22 +++++----------------- include/hw/ppc/spapr_xive.h | 4 ++-- include/hw/ppc/xics_spapr.h | 4 ++-- 5 files changed, 24 insertions(+), 37 deletions(-) diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c index 51b334b676..08012ac7cd 100644 --- a/hw/intc/spapr_xive_kvm.c +++ b/hw/intc/spapr_xive_kvm.c @@ -740,8 +740,9 @@ static void *kvmppc_xive_mmap(SpaprXive *xive, int pgoff, size_t len, * All the XIVE memory regions are now backed by mappings from the KVM * XIVE device. */ -void kvmppc_xive_connect(SpaprXive *xive, Error **errp) +int kvmppc_xive_connect(SpaprInterruptController *intc, Error **errp) { + SpaprXive *xive = SPAPR_XIVE(intc); XiveSource *xsrc = &xive->source; Error *local_err = NULL; size_t esb_len = (1ull << xsrc->esb_shift) * xsrc->nr_irqs; @@ -753,19 +754,19 @@ void kvmppc_xive_connect(SpaprXive *xive, Error **errp) * rebooting under the XIVE-only interrupt mode. */ if (xive->fd != -1) { - return; + return 0; } if (!kvmppc_has_cap_xive()) { error_setg(errp, "IRQ_XIVE capability must be present for KVM"); - return; + return -1; } /* 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"); - return; + return -1; } /* @@ -821,15 +822,17 @@ void kvmppc_xive_connect(SpaprXive *xive, Error **errp) kvm_kernel_irqchip = true; kvm_msi_via_irqfd_allowed = true; kvm_gsi_direct_mapping = true; - return; + return 0; fail: error_propagate(errp, local_err); - kvmppc_xive_disconnect(xive, NULL); + kvmppc_xive_disconnect(intc); + return -1; } -void kvmppc_xive_disconnect(SpaprXive *xive, Error **errp) +void kvmppc_xive_disconnect(SpaprInterruptController *intc) { + SpaprXive *xive = SPAPR_XIVE(intc); XiveSource *xsrc; size_t esb_len; @@ -838,11 +841,6 @@ void kvmppc_xive_disconnect(SpaprXive *xive, Error **errp) return; } - if (!kvmppc_has_cap_xive()) { - error_setg(errp, "IRQ_XIVE capability must be present for KVM"); - return; - } - /* Clear the KVM mapping */ xsrc = &xive->source; esb_len = (1ull << xsrc->esb_shift) * xsrc->nr_irqs; diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index ba90d6dc96..954c424b36 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -342,8 +342,9 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val) } } -int xics_kvm_connect(SpaprMachineState *spapr, Error **errp) +int xics_kvm_connect(SpaprInterruptController *intc, Error **errp) { + ICSState *ics = ICS_SPAPR(intc); int rc; CPUState *cs; Error *local_err = NULL; @@ -413,7 +414,7 @@ int xics_kvm_connect(SpaprMachineState *spapr, Error **errp) } /* Update the KVM sources */ - ics_set_kvm_state(spapr->ics, &local_err); + ics_set_kvm_state(ics, &local_err); if (local_err) { goto fail; } @@ -431,11 +432,11 @@ int xics_kvm_connect(SpaprMachineState *spapr, Error **errp) fail: error_propagate(errp, local_err); - xics_kvm_disconnect(spapr, NULL); + xics_kvm_disconnect(intc); return -1; } -void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp) +void xics_kvm_disconnect(SpaprInterruptController *intc) { /* * Only on P9 using the XICS-on XIVE KVM device: diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index a8005072e6..5c8ffb27da 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -124,7 +124,7 @@ static void spapr_irq_reset_xics(SpaprMachineState *spapr, Error **errp) static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp) { if (kvm_enabled()) { - xics_kvm_connect(spapr, errp); + xics_kvm_connect(SPAPR_INTC(spapr->ics), errp); } } @@ -173,7 +173,7 @@ static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp) { if (kvm_enabled()) { - kvmppc_xive_connect(spapr->xive, errp); + kvmppc_xive_connect(SPAPR_INTC(spapr->xive), errp); } } @@ -215,7 +215,7 @@ static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id) */ if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) { if (kvm_irqchip_in_kernel()) { - xics_kvm_disconnect(spapr, &error_fatal); + xics_kvm_disconnect(SPAPR_INTC(spapr->ics)); } spapr_irq_xive.reset(spapr, &error_fatal); } @@ -225,8 +225,6 @@ static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id) static void spapr_irq_reset_dual(SpaprMachineState *spapr, Error **errp) { - Error *local_err = NULL; - /* * Deactivate the XIVE MMIOs. The XIVE backend will reenable them * if selected. @@ -235,18 +233,8 @@ static void spapr_irq_reset_dual(SpaprMachineState *spapr, Error **errp) /* Destroy all KVM devices */ if (kvm_irqchip_in_kernel()) { - xics_kvm_disconnect(spapr, &local_err); - if (local_err) { - error_propagate(errp, local_err); - error_prepend(errp, "KVM XICS disconnect failed: "); - return; - } - kvmppc_xive_disconnect(spapr->xive, &local_err); - if (local_err) { - error_propagate(errp, local_err); - error_prepend(errp, "KVM XIVE disconnect failed: "); - return; - } + xics_kvm_disconnect(SPAPR_INTC(spapr->ics)); + kvmppc_xive_disconnect(SPAPR_INTC(spapr->xive)); } spapr_irq_current(spapr)->reset(spapr, errp); diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index ebe156eb30..64972754f9 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -68,8 +68,8 @@ int spapr_xive_end_to_target(uint8_t end_blk, uint32_t end_idx, /* * KVM XIVE device helpers */ -void kvmppc_xive_connect(SpaprXive *xive, Error **errp); -void kvmppc_xive_disconnect(SpaprXive *xive, Error **errp); +int kvmppc_xive_connect(SpaprInterruptController *intc, Error **errp); +void kvmppc_xive_disconnect(SpaprInterruptController *intc); void kvmppc_xive_reset(SpaprXive *xive, Error **errp); void kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS *eas, Error **errp); diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h index 8e4fb6adce..28b87038c8 100644 --- a/include/hw/ppc/xics_spapr.h +++ b/include/hw/ppc/xics_spapr.h @@ -32,8 +32,8 @@ #define TYPE_ICS_SPAPR "ics-spapr" #define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR) -int xics_kvm_connect(SpaprMachineState *spapr, Error **errp); -void xics_kvm_disconnect(SpaprMachineState *spapr, Error **errp); +int xics_kvm_connect(SpaprInterruptController *intc, Error **errp); +void xics_kvm_disconnect(SpaprInterruptController *intc); bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr); #endif /* XICS_SPAPR_H */ -- cgit v1.2.3 From 0a17e0c39f73080e6d5c4d9a2033a5bd475c416c Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 23:02:41 +1000 Subject: spapr: Remove SpaprIrq::init_kvm hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This hook is a bit odd. The only caller is spapr_irq_init_kvm(), but it explicitly takes an SpaprIrq *, so it's never really called through the current SpaprIrq. Essentially this is just a way of passing through a function pointer so that spapr_irq_init_kvm() can handle some configuration and error handling logic without duplicating it between the xics and xive reset paths. So, make it just take that function pointer. Because of earlier reworks to the KVM connect/disconnect code in the xics and xive backends we can also eliminate some wrapper functions and streamline error handling a bit. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr_irq.c | 74 ++++++++++++++++------------------------------ include/hw/ppc/spapr_irq.h | 1 - 2 files changed, 25 insertions(+), 50 deletions(-) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 5c8ffb27da..7cd18e5b15 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -65,33 +65,35 @@ void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num) bitmap_clear(spapr->irq_map, irq - SPAPR_IRQ_MSI, num); } -static void spapr_irq_init_kvm(SpaprMachineState *spapr, - SpaprIrq *irq, Error **errp) +static int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), + SpaprInterruptController *intc, + Error **errp) { - MachineState *machine = MACHINE(spapr); + MachineState *machine = MACHINE(qdev_get_machine()); Error *local_err = NULL; if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) { - irq->init_kvm(spapr, &local_err); - if (local_err && machine_kernel_irqchip_required(machine)) { - error_prepend(&local_err, - "kernel_irqchip requested but unavailable: "); - error_propagate(errp, local_err); - return; - } + if (fn(intc, &local_err) < 0) { + if (machine_kernel_irqchip_required(machine)) { + error_prepend(&local_err, + "kernel_irqchip requested but unavailable: "); + error_propagate(errp, local_err); + return -1; + } - if (!local_err) { - return; + /* + * We failed to initialize the KVM device, fallback to + * emulated mode + */ + error_prepend(&local_err, + "kernel_irqchip allowed but unavailable: "); + error_append_hint(&local_err, + "Falling back to kernel-irqchip=off\n"); + warn_report_err(local_err); } - - /* - * We failed to initialize the KVM device, fallback to - * emulated mode - */ - error_prepend(&local_err, "kernel_irqchip allowed but unavailable: "); - error_append_hint(&local_err, "Falling back to kernel-irqchip=off\n"); - warn_report_err(local_err); } + + return 0; } /* @@ -112,20 +114,7 @@ static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) static void spapr_irq_reset_xics(SpaprMachineState *spapr, Error **errp) { - Error *local_err = NULL; - - spapr_irq_init_kvm(spapr, &spapr_irq_xics, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } -} - -static void spapr_irq_init_kvm_xics(SpaprMachineState *spapr, Error **errp) -{ - if (kvm_enabled()) { - xics_kvm_connect(SPAPR_INTC(spapr->ics), errp); - } + spapr_irq_init_kvm(xics_kvm_connect, SPAPR_INTC(spapr->ics), errp); } SpaprIrq spapr_irq_xics = { @@ -136,7 +125,6 @@ SpaprIrq spapr_irq_xics = { .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, - .init_kvm = spapr_irq_init_kvm_xics, }; /* @@ -151,7 +139,6 @@ static int spapr_irq_post_load_xive(SpaprMachineState *spapr, int version_id) static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) { CPUState *cs; - Error *local_err = NULL; CPU_FOREACH(cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -160,9 +147,8 @@ static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx); } - spapr_irq_init_kvm(spapr, &spapr_irq_xive, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (spapr_irq_init_kvm(kvmppc_xive_connect, + SPAPR_INTC(spapr->xive), errp) < 0) { return; } @@ -170,13 +156,6 @@ static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) spapr_xive_mmio_set_enabled(spapr->xive, true); } -static void spapr_irq_init_kvm_xive(SpaprMachineState *spapr, Error **errp) -{ - if (kvm_enabled()) { - kvmppc_xive_connect(SPAPR_INTC(spapr->xive), errp); - } -} - SpaprIrq spapr_irq_xive = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, @@ -185,7 +164,6 @@ SpaprIrq spapr_irq_xive = { .post_load = spapr_irq_post_load_xive, .reset = spapr_irq_reset_xive, - .init_kvm = spapr_irq_init_kvm_xive, }; /* @@ -251,7 +229,6 @@ SpaprIrq spapr_irq_dual = { .post_load = spapr_irq_post_load_dual, .reset = spapr_irq_reset_dual, - .init_kvm = NULL, /* should not be used */ }; @@ -675,7 +652,6 @@ SpaprIrq spapr_irq_xics_legacy = { .post_load = spapr_irq_post_load_xics, .reset = spapr_irq_reset_xics, - .init_kvm = spapr_irq_init_kvm_xics, }; static void spapr_irq_register_types(void) diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 0df95e1b5a..06179b271f 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -85,7 +85,6 @@ typedef struct SpaprIrq { int (*post_load)(SpaprMachineState *spapr, int version_id); void (*reset)(SpaprMachineState *spapr, Error **errp); - void (*init_kvm)(SpaprMachineState *spapr, Error **errp); } SpaprIrq; extern SpaprIrq spapr_irq_xics; -- cgit v1.2.3 From 567192d486cc3073eb097246acc98b200fa3d198 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Thu, 26 Sep 2019 23:58:36 +1000 Subject: spapr, xics, xive: Move SpaprIrq::reset hook logic into activate/deactivate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It turns out that all the logic in the SpaprIrq::reset hooks (and some in the SpaprIrq::post_load hooks) isn't really related to resetting the irq backend (that's handled by the backends' own reset routines). Rather its about getting the backend ready to be the active interrupt controller or stopping being the active interrupt controller - reset (and post_load) is just the only time that changes at present. To make this flow clearer, move the logic into the explicit backend activate and deactivate hooks. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 38 ++++++++++++++++++++++++++ hw/intc/xics_spapr.c | 17 ++++++++++++ hw/ppc/spapr_irq.c | 67 +++------------------------------------------- include/hw/ppc/spapr_irq.h | 4 ++- 4 files changed, 61 insertions(+), 65 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 37ffb74ca5..1811653aac 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -640,6 +640,42 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers, plat_res_int_priorities, sizeof(plat_res_int_priorities))); } +static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + CPUState *cs; + + CPU_FOREACH(cs) { + PowerPCCPU *cpu = POWERPC_CPU(cs); + + /* (TCG) Set the OS CAM line of the thread interrupt context. */ + spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx); + } + + if (kvm_enabled()) { + int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp); + if (rc < 0) { + return rc; + } + } + + /* Activate the XIVE MMIOs */ + spapr_xive_mmio_set_enabled(xive, true); + + return 0; +} + +static void spapr_xive_deactivate(SpaprInterruptController *intc) +{ + SpaprXive *xive = SPAPR_XIVE(intc); + + spapr_xive_mmio_set_enabled(xive, false); + + if (kvm_irqchip_in_kernel()) { + kvmppc_xive_disconnect(intc); + } +} + static void spapr_xive_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -658,6 +694,8 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) xrc->write_nvt = spapr_xive_write_nvt; xrc->get_tctx = spapr_xive_get_tctx; + sicc->activate = spapr_xive_activate; + sicc->deactivate = spapr_xive_deactivate; sicc->cpu_intc_create = spapr_xive_cpu_intc_create; sicc->claim_irq = spapr_xive_claim_irq; sicc->free_irq = spapr_xive_free_irq; diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 4eabafc7e1..90b4d48877 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -395,6 +395,21 @@ static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) ics_pic_print_info(ics, mon); } +static int xics_spapr_activate(SpaprInterruptController *intc, Error **errp) +{ + if (kvm_enabled()) { + return spapr_irq_init_kvm(xics_kvm_connect, intc, errp); + } + return 0; +} + +static void xics_spapr_deactivate(SpaprInterruptController *intc) +{ + if (kvm_irqchip_in_kernel()) { + xics_kvm_disconnect(intc); + } +} + static void ics_spapr_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -403,6 +418,8 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) device_class_set_parent_realize(dc, ics_spapr_realize, &isc->parent_realize); + sicc->activate = xics_spapr_activate; + sicc->deactivate = xics_spapr_deactivate; sicc->cpu_intc_create = xics_spapr_cpu_intc_create; sicc->claim_irq = xics_spapr_claim_irq; sicc->free_irq = xics_spapr_free_irq; diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 7cd18e5b15..f70b331f44 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -65,9 +65,9 @@ void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num) bitmap_clear(spapr->irq_map, irq - SPAPR_IRQ_MSI, num); } -static int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), - SpaprInterruptController *intc, - Error **errp) +int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), + SpaprInterruptController *intc, + Error **errp) { MachineState *machine = MACHINE(qdev_get_machine()); Error *local_err = NULL; @@ -112,11 +112,6 @@ static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) return 0; } -static void spapr_irq_reset_xics(SpaprMachineState *spapr, Error **errp) -{ - spapr_irq_init_kvm(xics_kvm_connect, SPAPR_INTC(spapr->ics), errp); -} - SpaprIrq spapr_irq_xics = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, @@ -124,7 +119,6 @@ SpaprIrq spapr_irq_xics = { .xive = false, .post_load = spapr_irq_post_load_xics, - .reset = spapr_irq_reset_xics, }; /* @@ -136,26 +130,6 @@ static int spapr_irq_post_load_xive(SpaprMachineState *spapr, int version_id) return spapr_xive_post_load(spapr->xive, version_id); } -static void spapr_irq_reset_xive(SpaprMachineState *spapr, Error **errp) -{ - CPUState *cs; - - CPU_FOREACH(cs) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - - /* (TCG) Set the OS CAM line of the thread interrupt context. */ - spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx); - } - - if (spapr_irq_init_kvm(kvmppc_xive_connect, - SPAPR_INTC(spapr->xive), errp) < 0) { - return; - } - - /* Activate the XIVE MMIOs */ - spapr_xive_mmio_set_enabled(spapr->xive, true); -} - SpaprIrq spapr_irq_xive = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, @@ -163,7 +137,6 @@ SpaprIrq spapr_irq_xive = { .xive = true, .post_load = spapr_irq_post_load_xive, - .reset = spapr_irq_reset_xive, }; /* @@ -187,37 +160,9 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id) { - /* - * Force a reset of the XIVE backend after migration. The machine - * defaults to XICS at startup. - */ - if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) { - if (kvm_irqchip_in_kernel()) { - xics_kvm_disconnect(SPAPR_INTC(spapr->ics)); - } - spapr_irq_xive.reset(spapr, &error_fatal); - } - return spapr_irq_current(spapr)->post_load(spapr, version_id); } -static void spapr_irq_reset_dual(SpaprMachineState *spapr, Error **errp) -{ - /* - * Deactivate the XIVE MMIOs. The XIVE backend will reenable them - * if selected. - */ - spapr_xive_mmio_set_enabled(spapr->xive, false); - - /* Destroy all KVM devices */ - if (kvm_irqchip_in_kernel()) { - xics_kvm_disconnect(SPAPR_INTC(spapr->ics)); - kvmppc_xive_disconnect(SPAPR_INTC(spapr->xive)); - } - - spapr_irq_current(spapr)->reset(spapr, errp); -} - /* * Define values in sync with the XIVE and XICS backend */ @@ -228,7 +173,6 @@ SpaprIrq spapr_irq_dual = { .xive = true, .post_load = spapr_irq_post_load_dual, - .reset = spapr_irq_reset_dual, }; @@ -512,10 +456,6 @@ void spapr_irq_reset(SpaprMachineState *spapr, Error **errp) assert(!spapr->irq_map || bitmap_empty(spapr->irq_map, spapr->irq_map_nr)); spapr_irq_update_active_intc(spapr); - - if (spapr->irq->reset) { - spapr->irq->reset(spapr, errp); - } } int spapr_irq_get_phandle(SpaprMachineState *spapr, void *fdt, Error **errp) @@ -651,7 +591,6 @@ SpaprIrq spapr_irq_xics_legacy = { .xive = false, .post_load = spapr_irq_post_load_xics, - .reset = spapr_irq_reset_xics, }; static void spapr_irq_register_types(void) diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 06179b271f..e02e44624b 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -84,7 +84,6 @@ typedef struct SpaprIrq { bool xive; int (*post_load)(SpaprMachineState *spapr, int version_id); - void (*reset)(SpaprMachineState *spapr, Error **errp); } SpaprIrq; extern SpaprIrq spapr_irq_xics; @@ -99,6 +98,9 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq); int spapr_irq_post_load(SpaprMachineState *spapr, int version_id); void spapr_irq_reset(SpaprMachineState *spapr, Error **errp); int spapr_irq_get_phandle(SpaprMachineState *spapr, void *fdt, Error **errp); +int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), + SpaprInterruptController *intc, + Error **errp); /* * XICS legacy routines -- cgit v1.2.3 From 605994e5b7d17dcc275465b4f89816d29105b238 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 27 Sep 2019 10:53:53 +1000 Subject: spapr, xics, xive: Move SpaprIrq::post_load hook to backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining logic in the post_load hook really belongs to the interrupt controller backends, and just needs to be called on the active controller (after the active controller is set to the right thing based on the incoming migration in the generic spapr_irq_post_load() logic). Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/intc/spapr_xive.c | 5 +++-- hw/intc/xics_spapr.c | 13 +++++++++++++ hw/ppc/spapr_irq.c | 45 ++++----------------------------------------- include/hw/ppc/spapr_irq.h | 3 +-- include/hw/ppc/spapr_xive.h | 1 - 5 files changed, 21 insertions(+), 46 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 1811653aac..ba32d2cc5b 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -462,10 +462,10 @@ static int vmstate_spapr_xive_pre_save(void *opaque) * Called by the sPAPR IRQ backend 'post_load' method at the machine * level. */ -int spapr_xive_post_load(SpaprXive *xive, int version_id) +static int spapr_xive_post_load(SpaprInterruptController *intc, int version_id) { if (kvm_irqchip_in_kernel()) { - return kvmppc_xive_post_load(xive, version_id); + return kvmppc_xive_post_load(SPAPR_XIVE(intc), version_id); } return 0; @@ -702,6 +702,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) sicc->set_irq = spapr_xive_set_irq; sicc->print_info = spapr_xive_print_info; sicc->dt = spapr_xive_dt; + sicc->post_load = spapr_xive_post_load; } static const TypeInfo spapr_xive_info = { diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 90b4d48877..4f64b9a9fc 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -395,6 +395,18 @@ static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) ics_pic_print_info(ics, mon); } +static int xics_spapr_post_load(SpaprInterruptController *intc, int version_id) +{ + if (!kvm_irqchip_in_kernel()) { + CPUState *cs; + CPU_FOREACH(cs) { + PowerPCCPU *cpu = POWERPC_CPU(cs); + icp_resend(spapr_cpu_state(cpu)->icp); + } + } + return 0; +} + static int xics_spapr_activate(SpaprInterruptController *intc, Error **errp) { if (kvm_enabled()) { @@ -426,6 +438,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) sicc->set_irq = xics_spapr_set_irq; sicc->print_info = xics_spapr_print_info; sicc->dt = xics_spapr_dt; + sicc->post_load = xics_spapr_post_load; } static const TypeInfo ics_spapr_info = { diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index f70b331f44..f3d18b1dad 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -100,43 +100,22 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), * XICS IRQ backend. */ -static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id) -{ - if (!kvm_irqchip_in_kernel()) { - CPUState *cs; - CPU_FOREACH(cs) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - icp_resend(spapr_cpu_state(cpu)->icp); - } - } - return 0; -} - SpaprIrq spapr_irq_xics = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, .xics = true, .xive = false, - - .post_load = spapr_irq_post_load_xics, }; /* * XIVE IRQ backend. */ -static int spapr_irq_post_load_xive(SpaprMachineState *spapr, int version_id) -{ - return spapr_xive_post_load(spapr->xive, version_id); -} - SpaprIrq spapr_irq_xive = { .nr_xirqs = SPAPR_NR_XIRQS, .nr_msis = SPAPR_NR_MSIS, .xics = false, .xive = true, - - .post_load = spapr_irq_post_load_xive, }; /* @@ -148,21 +127,6 @@ SpaprIrq spapr_irq_xive = { * activated after an extra machine reset. */ -/* - * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the - * default. - */ -static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr) -{ - return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ? - &spapr_irq_xive : &spapr_irq_xics; -} - -static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id) -{ - return spapr_irq_current(spapr)->post_load(spapr, version_id); -} - /* * Define values in sync with the XIVE and XICS backend */ @@ -171,8 +135,6 @@ SpaprIrq spapr_irq_dual = { .nr_msis = SPAPR_NR_MSIS, .xics = true, .xive = true, - - .post_load = spapr_irq_post_load_dual, }; @@ -447,8 +409,11 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) int spapr_irq_post_load(SpaprMachineState *spapr, int version_id) { + SpaprInterruptControllerClass *sicc; + spapr_irq_update_active_intc(spapr); - return spapr->irq->post_load(spapr, version_id); + sicc = SPAPR_INTC_GET_CLASS(spapr->active_intc); + return sicc->post_load(spapr->active_intc, version_id); } void spapr_irq_reset(SpaprMachineState *spapr, Error **errp) @@ -589,8 +554,6 @@ SpaprIrq spapr_irq_xics_legacy = { .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, .xics = true, .xive = false, - - .post_load = spapr_irq_post_load_xics, }; static void spapr_irq_register_types(void) diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index e02e44624b..08173e714c 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -62,6 +62,7 @@ typedef struct SpaprInterruptControllerClass { void (*print_info)(SpaprInterruptController *intc, Monitor *mon); void (*dt)(SpaprInterruptController *intc, uint32_t nr_servers, void *fdt, uint32_t phandle); + int (*post_load)(SpaprInterruptController *intc, int version_id); } SpaprInterruptControllerClass; void spapr_irq_update_active_intc(SpaprMachineState *spapr); @@ -82,8 +83,6 @@ typedef struct SpaprIrq { uint32_t nr_msis; bool xics; bool xive; - - int (*post_load)(SpaprMachineState *spapr, int version_id); } SpaprIrq; extern SpaprIrq spapr_irq_xics; diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index 64972754f9..d84bd5c229 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -55,7 +55,6 @@ typedef struct SpaprXive { #define SPAPR_XIVE_BLOCK_ID 0x0 void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon); -int spapr_xive_post_load(SpaprXive *xive, int version_id); void spapr_xive_hcall_init(SpaprMachineState *spapr); void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx); -- cgit v1.2.3 From 8cbe71ecb8c1336b5ef96d64771608e02d88d4e3 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 27 Sep 2019 13:44:58 +1000 Subject: spapr: Remove SpaprIrq::nr_msis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nr_msis value we use here has to line up with whether we're using legacy or modern irq allocation. Therefore it's safer to derive it based on legacy_irq_allocation rather than having SpaprIrq contain a canned value. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr.c | 5 ++--- hw/ppc/spapr_irq.c | 26 +++++++++++++++++--------- hw/ppc/spapr_pci.c | 7 ++++--- include/hw/pci-host/spapr.h | 4 ++-- include/hw/ppc/spapr_irq.h | 4 +--- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index c9623600c2..99867b5e6d 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1267,7 +1267,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr) } QLIST_FOREACH(phb, &spapr->phbs, list) { - ret = spapr_dt_phb(phb, PHANDLE_INTC, fdt, spapr->irq->nr_msis, NULL); + ret = spapr_dt_phb(spapr, phb, PHANDLE_INTC, fdt, NULL); if (ret < 0) { error_report("couldn't setup PCI devices in fdt"); exit(1); @@ -3905,8 +3905,7 @@ int spapr_phb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, return -1; } - if (spapr_dt_phb(sphb, intc_phandle, fdt, spapr->irq->nr_msis, - fdt_start_offset)) { + if (spapr_dt_phb(spapr, sphb, intc_phandle, fdt, fdt_start_offset)) { error_setg(errp, "unable to create FDT node for PHB %d", sphb->index); return -1; } diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index f3d18b1dad..be90d36333 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -29,9 +29,14 @@ static const TypeInfo spapr_intc_info = { .class_size = sizeof(SpaprInterruptControllerClass), }; -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis) +static void spapr_irq_msi_init(SpaprMachineState *spapr) { - spapr->irq_map_nr = nr_msis; + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { + /* Legacy mode doesn't use this allocator */ + return; + } + + spapr->irq_map_nr = spapr_irq_nr_msis(spapr); spapr->irq_map = bitmap_new(spapr->irq_map_nr); } @@ -102,7 +107,6 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), SpaprIrq spapr_irq_xics = { .nr_xirqs = SPAPR_NR_XIRQS, - .nr_msis = SPAPR_NR_MSIS, .xics = true, .xive = false, }; @@ -113,7 +117,6 @@ SpaprIrq spapr_irq_xics = { SpaprIrq spapr_irq_xive = { .nr_xirqs = SPAPR_NR_XIRQS, - .nr_msis = SPAPR_NR_MSIS, .xics = false, .xive = true, }; @@ -132,7 +135,6 @@ SpaprIrq spapr_irq_xive = { */ SpaprIrq spapr_irq_dual = { .nr_xirqs = SPAPR_NR_XIRQS, - .nr_msis = SPAPR_NR_MSIS, .xics = true, .xive = true, }; @@ -247,6 +249,15 @@ void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, sicc->dt(spapr->active_intc, nr_servers, fdt, phandle); } +uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr) +{ + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { + return spapr->irq->nr_xirqs; + } else { + return SPAPR_XIRQ_BASE + spapr->irq->nr_xirqs - SPAPR_IRQ_MSI; + } +} + void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -267,9 +278,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) } /* Initialize the MSI IRQ allocator. */ - if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { - spapr_irq_msi_init(spapr, spapr->irq->nr_msis); - } + spapr_irq_msi_init(spapr); if (spapr->irq->xics) { Error *local_err = NULL; @@ -551,7 +560,6 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) SpaprIrq spapr_irq_xics_legacy = { .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, - .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, .xics = true, .xive = false, }; diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 01ff41d4c4..cc0e7829b6 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -2277,8 +2277,8 @@ static void spapr_phb_pci_enumerate(SpaprPhbState *phb) } -int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, - uint32_t nr_msis, int *node_offset) +int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb, + uint32_t intc_phandle, void *fdt, int *node_offset) { int bus_off, i, j, ret; uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) }; @@ -2343,7 +2343,8 @@ int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof_ranges)); _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg))); _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1)); - _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", nr_msis)); + _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", + spapr_irq_nr_msis(spapr))); /* Dynamic DMA window */ if (phb->ddw_enabled) { diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h index 23506f05d9..8877ff51fb 100644 --- a/include/hw/pci-host/spapr.h +++ b/include/hw/pci-host/spapr.h @@ -128,8 +128,8 @@ struct SpaprPhbState { #define SPAPR_PCI_NV2ATSD_WIN_SIZE (NVGPU_MAX_NUM * NVGPU_MAX_LINKS * \ 64 * KiB) -int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, - uint32_t nr_msis, int *node_offset); +int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb, + uint32_t intc_phandle, void *fdt, int *node_offset); void spapr_pci_rtas_init(void); diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 08173e714c..befe8e01dc 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -27,7 +27,6 @@ #define SPAPR_IRQ_MSI (SPAPR_XIRQ_BASE + 0x0300) #define SPAPR_NR_XIRQS 0x1000 -#define SPAPR_NR_MSIS (SPAPR_XIRQ_BASE + SPAPR_NR_XIRQS - SPAPR_IRQ_MSI) typedef struct SpaprMachineState SpaprMachineState; @@ -73,14 +72,13 @@ void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon); void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); +uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr); int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, Error **errp); void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); typedef struct SpaprIrq { uint32_t nr_xirqs; - uint32_t nr_msis; bool xics; bool xive; } SpaprIrq; -- cgit v1.2.3 From 54255c1f65e69a3f50121f2e37b89a84de2737a5 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 27 Sep 2019 13:54:23 +1000 Subject: spapr: Move SpaprIrq::nr_xirqs to SpaprMachineClass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the benefit of peripheral device allocation, the number of available irqs really wants to be the same on a given machine type version, regardless of what irq backends we are using. That's the case now, but only because we make sure the different SpaprIrq instances have the same value except for the special legacy one. Since this really only depends on machine type version, move the value to SpaprMachineClass instead of SpaprIrq. This also puts the code to set it to the lower value on old machine types right next to setting legacy_irq_allocation, which needs to go hand in hand. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/spapr.c | 2 ++ hw/ppc/spapr_irq.c | 33 ++++++++++++++++----------------- include/hw/ppc/spapr.h | 1 + include/hw/ppc/spapr_irq.h | 1 - 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 99867b5e6d..f9410d390a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4440,6 +4440,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) smc->dr_phb_enabled = true; smc->linux_pci_probe = true; smc->smp_threads_vsmt = true; + smc->nr_xirqs = SPAPR_NR_XIRQS; } static const TypeInfo spapr_machine_info = { @@ -4576,6 +4577,7 @@ static void spapr_machine_3_0_class_options(MachineClass *mc) compat_props_add(mc->compat_props, hw_compat_3_0, hw_compat_3_0_len); smc->legacy_irq_allocation = true; + smc->nr_xirqs = 0x400; smc->irq = &spapr_irq_xics_legacy; } diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index be90d36333..234d1073e5 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -106,7 +106,6 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), */ SpaprIrq spapr_irq_xics = { - .nr_xirqs = SPAPR_NR_XIRQS, .xics = true, .xive = false, }; @@ -116,7 +115,6 @@ SpaprIrq spapr_irq_xics = { */ SpaprIrq spapr_irq_xive = { - .nr_xirqs = SPAPR_NR_XIRQS, .xics = false, .xive = true, }; @@ -134,7 +132,6 @@ SpaprIrq spapr_irq_xive = { * Define values in sync with the XIVE and XICS backend */ SpaprIrq spapr_irq_dual = { - .nr_xirqs = SPAPR_NR_XIRQS, .xics = true, .xive = true, }; @@ -251,16 +248,19 @@ void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr) { - if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { - return spapr->irq->nr_xirqs; + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); + + if (smc->legacy_irq_allocation) { + return smc->nr_xirqs; } else { - return SPAPR_XIRQ_BASE + spapr->irq->nr_xirqs - SPAPR_IRQ_MSI; + return SPAPR_XIRQ_BASE + smc->nr_xirqs - SPAPR_IRQ_MSI; } } void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); if (machine_kernel_irqchip_split(machine)) { error_setg(errp, "kernel_irqchip split mode not supported on pseries"); @@ -298,8 +298,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) return; } - object_property_set_int(obj, spapr->irq->nr_xirqs, "nr-irqs", - &local_err); + object_property_set_int(obj, smc->nr_xirqs, "nr-irqs", &local_err); if (local_err) { error_propagate(errp, local_err); return; @@ -320,8 +319,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) int i; dev = qdev_create(NULL, TYPE_SPAPR_XIVE); - qdev_prop_set_uint32(dev, "nr-irqs", - spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); + qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_XIRQ_BASE); /* * 8 XIVE END structures per CPU. One for each available * priority @@ -346,17 +344,18 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) } spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr, - spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE); + smc->nr_xirqs + SPAPR_XIRQ_BASE); } int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp) { SpaprInterruptController *intcs[] = ALL_INTCS(spapr); int i; + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); int rc; assert(irq >= SPAPR_XIRQ_BASE); - assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); + assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE)); for (i = 0; i < ARRAY_SIZE(intcs); i++) { SpaprInterruptController *intc = intcs[i]; @@ -376,9 +375,10 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) { SpaprInterruptController *intcs[] = ALL_INTCS(spapr); int i, j; + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); assert(irq >= SPAPR_XIRQ_BASE); - assert((irq + num) <= (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); + assert((irq + num) <= (smc->nr_xirqs + SPAPR_XIRQ_BASE)); for (i = irq; i < (irq + num); i++) { for (j = 0; j < ARRAY_SIZE(intcs); j++) { @@ -395,6 +395,8 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num) qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) { + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); + /* * This interface is basically for VIO and PHB devices to find the * right qemu_irq to manipulate, so we only allow access to the @@ -403,7 +405,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) * interfaces, we can change this if we need to in future. */ assert(irq >= SPAPR_XIRQ_BASE); - assert(irq < (spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE)); + assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE)); if (spapr->ics) { assert(ics_valid_irq(spapr->ics, irq)); @@ -556,10 +558,7 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) return first + ics->offset; } -#define SPAPR_IRQ_XICS_LEGACY_NR_XIRQS 0x400 - SpaprIrq spapr_irq_xics_legacy = { - .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, .xics = true, .xive = false, }; diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 3b34cf5207..d5ab5ea7b2 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -119,6 +119,7 @@ struct SpaprMachineClass { bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */ bool pre_2_10_has_unused_icps; bool legacy_irq_allocation; + uint32_t nr_xirqs; bool broken_host_serial_model; /* present real host info to the guest */ bool pre_4_1_migration; /* don't migrate hpt-max-page-size */ bool linux_pci_probe; diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index befe8e01dc..5e150a6679 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -78,7 +78,6 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); typedef struct SpaprIrq { - uint32_t nr_xirqs; bool xics; bool xive; } SpaprIrq; -- cgit v1.2.3 From cb97526aa47e0590a04bf90579b76584fbc0d79f Mon Sep 17 00:00:00 2001 From: Alexey Kardashevskiy Date: Tue, 22 Oct 2019 15:05:36 +1100 Subject: pseries: Update SLOF firmware image This aims v4.2 and fixes: 1. full FDT rendering; 2. gcc9 -Waddress-of-packed-member. Signed-off-by: Alexey Kardashevskiy Signed-off-by: David Gibson --- pc-bios/README | 2 +- pc-bios/slof.bin | Bin 930640 -> 928552 bytes roms/SLOF | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pc-bios/README b/pc-bios/README index 0b0b5d42ad..830645c51f 100644 --- a/pc-bios/README +++ b/pc-bios/README @@ -17,7 +17,7 @@ - SLOF (Slimline Open Firmware) is a free IEEE 1275 Open Firmware implementation for certain IBM POWER hardware. The sources are at https://github.com/aik/SLOF, and the image currently in qemu is - built from git tag qemu-slof-20190911. + built from git tag qemu-slof-20191022. - sgabios (the Serial Graphics Adapter option ROM) provides a means for legacy x86 software to communicate with an attached serial console as diff --git a/pc-bios/slof.bin b/pc-bios/slof.bin index 2af0c5d5fc..0b93fe8c01 100644 Binary files a/pc-bios/slof.bin and b/pc-bios/slof.bin differ diff --git a/roms/SLOF b/roms/SLOF index bcc3c4e5c2..899d988365 160000 --- a/roms/SLOF +++ b/roms/SLOF @@ -1 +1 @@ -Subproject commit bcc3c4e5c21a015f4680894c4ec978a90d4a2d69 +Subproject commit 899d98836513bb3d6a4f4e48ef7cee887ee5f57b -- cgit v1.2.3 From 47c8c915b1628360d4d7d483e421e49b6bcfc371 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Wed, 23 Oct 2019 21:17:40 +0200 Subject: spapr: Don't request to unplug the same core twice We must not call spapr_drc_detach() on a detached DRC otherwise bad things can happen, ie. QEMU hangs or crashes. This is easily demonstrated with a CPU hotplug/unplug loop using QMP. Signed-off-by: Greg Kurz Message-Id: <157185826035.3073024.1664101000438499392.stgit@bahia.lan> Signed-off-by: David Gibson --- hw/ppc/spapr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f9410d390a..94f9d27096 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3741,9 +3741,10 @@ void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState *dev, spapr_vcpu_id(spapr, cc->core_id)); g_assert(drc); - spapr_drc_detach(drc); - - spapr_hotplug_req_remove_by_index(drc); + if (!spapr_drc_unplug_requested(drc)) { + spapr_drc_detach(drc); + spapr_hotplug_req_remove_by_index(drc); + } } int spapr_core_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, -- cgit v1.2.3 From 90f8db52bb7ea387ff45ac12bb73935b7fc27794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 22 Oct 2019 18:38:06 +0200 Subject: spapr: move CPU reset after presenter creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change prepares ground for future changes which will reset the interrupt presenter in the reset handler of the sPAPR and PowerNV cores. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Greg Kurz Signed-off-by: Cédric Le Goater Message-Id: <20191022163812.330-2-clg@kaod.org> Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 3e4302c7d5..2b21285d20 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -234,13 +234,16 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr, cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr)); kvmppc_set_papr(cpu); - qemu_register_reset(spapr_cpu_reset, cpu); - spapr_cpu_reset(cpu); - if (spapr_irq_cpu_intc_create(spapr, cpu, &local_err) < 0) { - goto error_unregister; + goto error_intc_create; } + /* + * FIXME: Hot-plugged CPUs are not reset. Do it at realize. + */ + qemu_register_reset(spapr_cpu_reset, cpu); + spapr_cpu_reset(cpu); + if (!sc->pre_3_0_migration) { vmstate_register(NULL, cs->cpu_index, &vmstate_spapr_cpu_state, cpu->machine_data); @@ -248,8 +251,7 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr, return; -error_unregister: - qemu_unregister_reset(spapr_cpu_reset, cpu); +error_intc_create: cpu_remove_sync(CPU(cpu)); error: error_propagate(errp, local_err); -- cgit v1.2.3 From d1f2b4691a268306a319ea76f67421431f529a07 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Tue, 22 Oct 2019 18:38:07 +0200 Subject: spapr_cpu_core: Implement DeviceClass::reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since vCPUs aren't plugged into a bus, we manually register a reset handler for each vCPU. We also call this handler at realize time to ensure hot plugged vCPUs are reset before being exposed to the guest. This results in vCPUs being reset twice at machine reset. It doesn't break anything but it is slightly suboptimal and above all confusing. The hotplug path in device_set_realized() already knows how to reset a hotplugged device if the device reset handler is present. Implement one for sPAPR CPU cores that resets all vCPUs under a core. While here rename spapr_cpu_reset() to spapr_reset_vcpu() for consistency with spapr_realize_vcpu() and spapr_unrealize_vcpu(). Signed-off-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé [clg: add documentation on the reset helper usage ] Signed-off-by: Cédric Le Goater Message-Id: <20191022163812.330-3-clg@kaod.org> Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 2b21285d20..2e34832d0e 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -25,9 +25,8 @@ #include "sysemu/hw_accel.h" #include "qemu/error-report.h" -static void spapr_cpu_reset(void *opaque) +static void spapr_reset_vcpu(PowerPCCPU *cpu) { - PowerPCCPU *cpu = opaque; CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); @@ -193,7 +192,6 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) if (!sc->pre_3_0_migration) { vmstate_unregister(NULL, &vmstate_spapr_cpu_state, cpu->machine_data); } - qemu_unregister_reset(spapr_cpu_reset, cpu); if (spapr_cpu_state(cpu)->icp) { object_unparent(OBJECT(spapr_cpu_state(cpu)->icp)); } @@ -204,12 +202,36 @@ static void spapr_unrealize_vcpu(PowerPCCPU *cpu, SpaprCpuCore *sc) object_unparent(OBJECT(cpu)); } +/* + * Called when CPUs are hot-plugged. + */ +static void spapr_cpu_core_reset(DeviceState *dev) +{ + CPUCore *cc = CPU_CORE(dev); + SpaprCpuCore *sc = SPAPR_CPU_CORE(dev); + int i; + + for (i = 0; i < cc->nr_threads; i++) { + spapr_reset_vcpu(sc->threads[i]); + } +} + +/* + * Called by the machine reset. + */ +static void spapr_cpu_core_reset_handler(void *opaque) +{ + spapr_cpu_core_reset(opaque); +} + static void spapr_cpu_core_unrealize(DeviceState *dev, Error **errp) { SpaprCpuCore *sc = SPAPR_CPU_CORE(OBJECT(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); } @@ -238,12 +260,6 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr, goto error_intc_create; } - /* - * FIXME: Hot-plugged CPUs are not reset. Do it at realize. - */ - qemu_register_reset(spapr_cpu_reset, cpu); - spapr_cpu_reset(cpu); - if (!sc->pre_3_0_migration) { vmstate_register(NULL, cs->cpu_index, &vmstate_spapr_cpu_state, cpu->machine_data); @@ -338,6 +354,8 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) goto err_unrealize; } } + + qemu_register_reset(spapr_cpu_core_reset_handler, sc); return; err_unrealize: @@ -366,6 +384,7 @@ static void spapr_cpu_core_class_init(ObjectClass *oc, void *data) dc->realize = spapr_cpu_core_realize; dc->unrealize = spapr_cpu_core_unrealize; + dc->reset = spapr_cpu_core_reset; dc->props = spapr_cpu_core_properties; scc->cpu_type = data; } -- cgit v1.2.3 From fa06541b5d24897a4833dfb2fe03635f07f36527 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 22 Oct 2019 18:38:08 +0200 Subject: ppc/pnv: Introduce a PnvCore reset handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit in which individual CPUs are reset. It will ease the introduction of future change reseting the interrupt presenter from the CPU reset handler. Signed-off-by: Cédric Le Goater Reviewed-by: Greg Kurz Message-Id: <20191022163812.330-4-clg@kaod.org> Signed-off-by: David Gibson --- hw/ppc/pnv_core.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index b1a7489e7a..9f981a4940 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -40,9 +40,8 @@ static const char *pnv_core_cpu_typename(PnvCore *pc) return cpu_type; } -static void pnv_cpu_reset(void *opaque) +static void pnv_core_cpu_reset(PowerPCCPU *cpu) { - PowerPCCPU *cpu = opaque; CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; @@ -192,8 +191,17 @@ static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip *chip, Error **errp) /* Set time-base frequency to 512 MHz */ cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ); +} + +static void pnv_core_reset(void *dev) +{ + CPUCore *cc = CPU_CORE(dev); + PnvCore *pc = PNV_CORE(dev); + int i; - qemu_register_reset(pnv_cpu_reset, cpu); + for (i = 0; i < cc->nr_threads; i++) { + pnv_core_cpu_reset(pc->threads[i]); + } } static void pnv_core_realize(DeviceState *dev, Error **errp) @@ -244,6 +252,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id); pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops, pc, name, PNV_XSCOM_EX_SIZE); + + qemu_register_reset(pnv_core_reset, pc); return; err: @@ -259,7 +269,6 @@ static void pnv_unrealize_vcpu(PowerPCCPU *cpu) { PnvCPUState *pnv_cpu = pnv_cpu_state(cpu); - qemu_unregister_reset(pnv_cpu_reset, cpu); object_unparent(OBJECT(pnv_cpu_state(cpu)->intc)); cpu_remove_sync(CPU(cpu)); cpu->machine_data = NULL; @@ -273,6 +282,8 @@ static void pnv_core_unrealize(DeviceState *dev, Error **errp) CPUCore *cc = CPU_CORE(dev); int i; + qemu_unregister_reset(pnv_core_reset, pc); + for (i = 0; i < cc->nr_threads; i++) { pnv_unrealize_vcpu(pc->threads[i]); } -- cgit v1.2.3 From aa5ac64b2394712b6269d0b15ba06c9c564dee92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 22 Oct 2019 18:38:09 +0200 Subject: ppc/pnv: Add a PnvChip pointer to PnvCore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We will use it to reset the interrupt presenter from the CPU reset handler. Signed-off-by: Cédric Le Goater Reviewed-by: Greg Kurz Message-Id: <20191022163812.330-5-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/pnv_core.c | 3 ++- include/hw/ppc/pnv_core.h | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index 9f981a4940..cc17bbfed8 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -222,6 +222,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) "required link 'chip' not found: "); return; } + pc->chip = PNV_CHIP(chip); pc->threads = g_new(PowerPCCPU *, cc->nr_threads); for (i = 0; i < cc->nr_threads; i++) { @@ -243,7 +244,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) } for (j = 0; j < cc->nr_threads; j++) { - pnv_realize_vcpu(pc->threads[j], PNV_CHIP(chip), &local_err); + pnv_realize_vcpu(pc->threads[j], pc->chip, &local_err); if (local_err) { goto err; } diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h index bfbd2ec42a..55eee95104 100644 --- a/include/hw/ppc/pnv_core.h +++ b/include/hw/ppc/pnv_core.h @@ -31,6 +31,8 @@ #define PNV_CORE_GET_CLASS(obj) \ OBJECT_GET_CLASS(PnvCoreClass, (obj), TYPE_PNV_CORE) +typedef struct PnvChip PnvChip; + typedef struct PnvCore { /*< private >*/ CPUCore parent_obj; @@ -38,6 +40,7 @@ typedef struct PnvCore { /*< public >*/ PowerPCCPU **threads; uint32_t pir; + PnvChip *chip; MemoryRegion xscom_regs; } PnvCore; -- cgit v1.2.3 From d49e8a9b46e4594223806ae622af462ff7bfa158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 22 Oct 2019 18:38:10 +0200 Subject: ppc: Reset the interrupt presenter from the CPU reset handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the sPAPR machine and PowerNV machine, the interrupt presenters are created by a machine handler at the core level and are reset independently. This is not consistent and it raises issues when it comes to handle hot-plugged CPUs. In that case, the presenters are not reset. This is less of an issue in XICS, although a zero MFFR could be a concern, but in XIVE, the OS CAM line is not set and this breaks the presenting algorithm. The current code has workarounds which need a global cleanup. Extend the sPAPR IRQ backend and the PowerNV Chip class with a new cpu_intc_reset() handler called by the CPU reset handler and remove the XiveTCTX reset handler which is now redundant. Signed-off-by: Cédric Le Goater Message-Id: <20191022163812.330-6-clg@kaod.org> Reviewed-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/intc/spapr_xive.c | 9 +++++++++ hw/intc/xics.c | 8 ++------ hw/intc/xics_spapr.c | 7 +++++++ hw/intc/xive.c | 12 +----------- hw/ppc/pnv.c | 18 ++++++++++++++++++ hw/ppc/pnv_core.c | 7 +++++-- hw/ppc/spapr_cpu_core.c | 5 ++++- hw/ppc/spapr_irq.c | 14 ++++++++++++++ include/hw/ppc/pnv.h | 1 + include/hw/ppc/spapr_irq.h | 2 ++ include/hw/ppc/xics.h | 1 + include/hw/ppc/xive.h | 1 + 12 files changed, 65 insertions(+), 20 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index ba32d2cc5b..20a8d8285f 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -553,6 +553,14 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc, return 0; } +static void spapr_xive_cpu_intc_reset(SpaprInterruptController *intc, + PowerPCCPU *cpu) +{ + XiveTCTX *tctx = spapr_cpu_state(cpu)->tctx; + + xive_tctx_reset(tctx); +} + static void spapr_xive_set_irq(SpaprInterruptController *intc, int irq, int val) { SpaprXive *xive = SPAPR_XIVE(intc); @@ -697,6 +705,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data) sicc->activate = spapr_xive_activate; sicc->deactivate = spapr_xive_deactivate; sicc->cpu_intc_create = spapr_xive_cpu_intc_create; + sicc->cpu_intc_reset = spapr_xive_cpu_intc_reset; sicc->claim_irq = spapr_xive_claim_irq; sicc->free_irq = spapr_xive_free_irq; sicc->set_irq = spapr_xive_set_irq; diff --git a/hw/intc/xics.c b/hw/intc/xics.c index b5ac408f7b..6da05763f9 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -274,10 +274,8 @@ static const VMStateDescription vmstate_icp_server = { }, }; -static void icp_reset_handler(void *dev) +void icp_reset(ICPState *icp) { - ICPState *icp = ICP(dev); - icp->xirr = 0; icp->pending_priority = 0xff; icp->mfrr = 0xff; @@ -288,7 +286,7 @@ static void icp_reset_handler(void *dev) if (kvm_irqchip_in_kernel()) { Error *local_err = NULL; - icp_set_kvm_state(ICP(dev), &local_err); + icp_set_kvm_state(icp, &local_err); if (local_err) { error_report_err(local_err); } @@ -351,7 +349,6 @@ static void icp_realize(DeviceState *dev, Error **errp) } } - qemu_register_reset(icp_reset_handler, dev); vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); } @@ -360,7 +357,6 @@ static void icp_unrealize(DeviceState *dev, Error **errp) ICPState *icp = ICP(dev); vmstate_unregister(NULL, &vmstate_icp_server, icp); - qemu_unregister_reset(icp_reset_handler, dev); } static void icp_class_init(ObjectClass *klass, void *data) diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 4f64b9a9fc..7418fb9f37 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -346,6 +346,12 @@ static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, return 0; } +static void xics_spapr_cpu_intc_reset(SpaprInterruptController *intc, + PowerPCCPU *cpu) +{ + icp_reset(spapr_cpu_state(cpu)->icp); +} + static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, bool lsi, Error **errp) { @@ -433,6 +439,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) sicc->activate = xics_spapr_activate; sicc->deactivate = xics_spapr_deactivate; sicc->cpu_intc_create = xics_spapr_cpu_intc_create; + sicc->cpu_intc_reset = xics_spapr_cpu_intc_reset; sicc->claim_irq = xics_spapr_claim_irq; sicc->free_irq = xics_spapr_free_irq; sicc->set_irq = xics_spapr_set_irq; diff --git a/hw/intc/xive.c b/hw/intc/xive.c index d420c6571e..f066be5eb5 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -547,10 +547,8 @@ void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon) } } -static void xive_tctx_reset(void *dev) +void xive_tctx_reset(XiveTCTX *tctx) { - XiveTCTX *tctx = XIVE_TCTX(dev); - memset(tctx->regs, 0, sizeof(tctx->regs)); /* Set some defaults */ @@ -607,13 +605,6 @@ static void xive_tctx_realize(DeviceState *dev, Error **errp) return; } } - - qemu_register_reset(xive_tctx_reset, dev); -} - -static void xive_tctx_unrealize(DeviceState *dev, Error **errp) -{ - qemu_unregister_reset(xive_tctx_reset, dev); } static int vmstate_xive_tctx_pre_save(void *opaque) @@ -668,7 +659,6 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data) dc->desc = "XIVE Interrupt Thread Context"; dc->realize = xive_tctx_realize; - dc->unrealize = xive_tctx_unrealize; dc->vmsd = &vmstate_xive_tctx; /* * Reason: part of XIVE interrupt controller, needs to be wired up diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 7cf64b6d25..4a51fb65a8 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -778,6 +778,13 @@ static void pnv_chip_power8_intc_create(PnvChip *chip, PowerPCCPU *cpu, pnv_cpu->intc = obj; } +static void pnv_chip_power8_intc_reset(PnvChip *chip, PowerPCCPU *cpu) +{ + PnvCPUState *pnv_cpu = pnv_cpu_state(cpu); + + icp_reset(ICP(pnv_cpu->intc)); +} + /* * 0:48 Reserved - Read as zeroes * 49:52 Node ID @@ -815,6 +822,13 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu, pnv_cpu->intc = obj; } +static void pnv_chip_power9_intc_reset(PnvChip *chip, PowerPCCPU *cpu) +{ + PnvCPUState *pnv_cpu = pnv_cpu_state(cpu); + + xive_tctx_reset(XIVE_TCTX(pnv_cpu->intc)); +} + /* * Allowed core identifiers on a POWER8 Processor Chip : * @@ -984,6 +998,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data) k->cores_mask = POWER8E_CORE_MASK; k->core_pir = pnv_chip_core_pir_p8; k->intc_create = pnv_chip_power8_intc_create; + k->intc_reset = pnv_chip_power8_intc_reset; k->isa_create = pnv_chip_power8_isa_create; k->dt_populate = pnv_chip_power8_dt_populate; k->pic_print_info = pnv_chip_power8_pic_print_info; @@ -1003,6 +1018,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data) k->cores_mask = POWER8_CORE_MASK; k->core_pir = pnv_chip_core_pir_p8; k->intc_create = pnv_chip_power8_intc_create; + k->intc_reset = pnv_chip_power8_intc_reset; k->isa_create = pnv_chip_power8_isa_create; k->dt_populate = pnv_chip_power8_dt_populate; k->pic_print_info = pnv_chip_power8_pic_print_info; @@ -1022,6 +1038,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data) k->cores_mask = POWER8_CORE_MASK; k->core_pir = pnv_chip_core_pir_p8; k->intc_create = pnv_chip_power8_intc_create; + k->intc_reset = pnv_chip_power8_intc_reset; k->isa_create = pnv_chip_power8nvl_isa_create; k->dt_populate = pnv_chip_power8_dt_populate; k->pic_print_info = pnv_chip_power8_pic_print_info; @@ -1191,6 +1208,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data) k->cores_mask = POWER9_CORE_MASK; k->core_pir = pnv_chip_core_pir_p9; k->intc_create = pnv_chip_power9_intc_create; + k->intc_reset = pnv_chip_power9_intc_reset; k->isa_create = pnv_chip_power9_isa_create; k->dt_populate = pnv_chip_power9_dt_populate; k->pic_print_info = pnv_chip_power9_pic_print_info; diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index cc17bbfed8..be0310ac03 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -40,10 +40,11 @@ static const char *pnv_core_cpu_typename(PnvCore *pc) return cpu_type; } -static void pnv_core_cpu_reset(PowerPCCPU *cpu) +static void pnv_core_cpu_reset(PowerPCCPU *cpu, PnvChip *chip) { CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip); cpu_reset(cs); @@ -54,6 +55,8 @@ static void pnv_core_cpu_reset(PowerPCCPU *cpu) env->gpr[3] = PNV_FDT_ADDR; env->nip = 0x10; env->msr |= MSR_HVB; /* Hypervisor mode */ + + pcc->intc_reset(chip, cpu); } /* @@ -200,7 +203,7 @@ static void pnv_core_reset(void *dev) int i; for (i = 0; i < cc->nr_threads; i++) { - pnv_core_cpu_reset(pc->threads[i]); + pnv_core_cpu_reset(pc->threads[i], pc->chip); } } diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 2e34832d0e..ef7b27a66d 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -32,6 +32,7 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu) PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); target_ulong lpcr; + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); cpu_reset(cs); @@ -76,9 +77,11 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu) spapr_cpu->dtl_addr = 0; spapr_cpu->dtl_size = 0; - spapr_caps_cpu_apply(SPAPR_MACHINE(qdev_get_machine()), cpu); + spapr_caps_cpu_apply(spapr, cpu); kvm_check_mmu(cpu, &error_fatal); + + spapr_irq_cpu_intc_reset(spapr, cpu); } void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip, target_ulong r3) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 234d1073e5..b941608b69 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -220,6 +220,20 @@ int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, return 0; } +void spapr_irq_cpu_intc_reset(SpaprMachineState *spapr, PowerPCCPU *cpu) +{ + SpaprInterruptController *intcs[] = ALL_INTCS(spapr); + int i; + + for (i = 0; i < ARRAY_SIZE(intcs); i++) { + SpaprInterruptController *intc = intcs[i]; + if (intc) { + SpaprInterruptControllerClass *sicc = SPAPR_INTC_GET_CLASS(intc); + sicc->cpu_intc_reset(intc, cpu); + } + } +} + static void spapr_set_irq(void *opaque, int irq, int level) { SpaprMachineState *spapr = SPAPR_MACHINE(opaque); diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index 1cdbe55bf8..2a780e633f 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -111,6 +111,7 @@ typedef struct PnvChipClass { uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id); void (*intc_create)(PnvChip *chip, PowerPCCPU *cpu, Error **errp); + void (*intc_reset)(PnvChip *chip, PowerPCCPU *cpu); ISABus *(*isa_create)(PnvChip *chip, Error **errp); void (*dt_populate)(PnvChip *chip, void *fdt); void (*pic_print_info)(PnvChip *chip, Monitor *mon); diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 5e150a6679..09232999b0 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -52,6 +52,7 @@ typedef struct SpaprInterruptControllerClass { */ int (*cpu_intc_create)(SpaprInterruptController *intc, PowerPCCPU *cpu, Error **errp); + void (*cpu_intc_reset)(SpaprInterruptController *intc, PowerPCCPU *cpu); int (*claim_irq)(SpaprInterruptController *intc, int irq, bool lsi, Error **errp); void (*free_irq)(SpaprInterruptController *intc, int irq); @@ -68,6 +69,7 @@ void spapr_irq_update_active_intc(SpaprMachineState *spapr); int spapr_irq_cpu_intc_create(SpaprMachineState *spapr, PowerPCCPU *cpu, Error **errp); +void spapr_irq_cpu_intc_reset(SpaprMachineState *spapr, PowerPCCPU *cpu); void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon); void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 1e6a9300eb..602173c122 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -161,6 +161,7 @@ void icp_set_mfrr(ICPState *icp, uint8_t mfrr); uint32_t icp_accept(ICPState *ss); uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr); void icp_eoi(ICPState *icp, uint32_t xirr); +void icp_reset(ICPState *icp); void ics_write_xive(ICSState *ics, int nr, int server, uint8_t priority, uint8_t saved_priority); diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h index fd3319bd32..99381639f5 100644 --- a/include/hw/ppc/xive.h +++ b/include/hw/ppc/xive.h @@ -415,6 +415,7 @@ uint64_t xive_tctx_tm_read(XiveTCTX *tctx, hwaddr offset, unsigned size); void xive_tctx_pic_print_info(XiveTCTX *tctx, Monitor *mon); Object *xive_tctx_create(Object *cpu, XiveRouter *xrtr, Error **errp); +void xive_tctx_reset(XiveTCTX *tctx); static inline uint32_t xive_nvt_cam_line(uint8_t nvt_blk, uint32_t nvt_idx) { -- cgit v1.2.3 From 00d6f4db604113a88afa0b823aaa50b6d91afb9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 22 Oct 2019 18:38:11 +0200 Subject: ppc/pnv: Fix naming of routines realizing the CPUs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'vcpu' suffix is inherited from the sPAPR machine. Use better names for PowerNV. Signed-off-by: Cédric Le Goater Reviewed-by: Greg Kurz Message-Id: <20191022163812.330-7-clg@kaod.org> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Gibson --- hw/ppc/pnv_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c index be0310ac03..e81cd3a3e0 100644 --- a/hw/ppc/pnv_core.c +++ b/hw/ppc/pnv_core.c @@ -162,7 +162,7 @@ static const MemoryRegionOps pnv_core_power9_xscom_ops = { .endianness = DEVICE_BIG_ENDIAN, }; -static void pnv_realize_vcpu(PowerPCCPU *cpu, PnvChip *chip, Error **errp) +static void pnv_core_cpu_realize(PowerPCCPU *cpu, PnvChip *chip, Error **errp) { CPUPPCState *env = &cpu->env; int core_pir; @@ -247,7 +247,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) } for (j = 0; j < cc->nr_threads; j++) { - pnv_realize_vcpu(pc->threads[j], pc->chip, &local_err); + pnv_core_cpu_realize(pc->threads[j], pc->chip, &local_err); if (local_err) { goto err; } @@ -269,7 +269,7 @@ err: error_propagate(errp, local_err); } -static void pnv_unrealize_vcpu(PowerPCCPU *cpu) +static void pnv_core_cpu_unrealize(PowerPCCPU *cpu) { PnvCPUState *pnv_cpu = pnv_cpu_state(cpu); @@ -289,7 +289,7 @@ static void pnv_core_unrealize(DeviceState *dev, Error **errp) qemu_unregister_reset(pnv_core_reset, pc); for (i = 0; i < cc->nr_threads; i++) { - pnv_unrealize_vcpu(pc->threads[i]); + pnv_core_cpu_unrealize(pc->threads[i]); } g_free(pc->threads); } -- cgit v1.2.3 From 97c00c54449b4ff349f85c6ce409dadd1b935a7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Tue, 22 Oct 2019 18:38:12 +0200 Subject: spapr/xive: Set the OS CAM line at reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a Virtual Processor is scheduled to run on a HW thread, the hypervisor pushes its identifier in the OS CAM line. When running with kernel_irqchip=off, QEMU needs to emulate the same behavior. Set the OS CAM line when the interrupt presenter of the sPAPR core is reset. This will also cover the case of hot-plugged CPUs. This change also has the benefit to remove the use of CPU_FOREACH() which can be unsafe. Signed-off-by: Cédric Le Goater Reviewed-by: Greg Kurz Message-Id: <20191022163812.330-8-clg@kaod.org> Signed-off-by: David Gibson --- hw/intc/spapr_xive.c | 48 ++++++++++++++++----------------------------- include/hw/ppc/spapr_xive.h | 1 - 2 files changed, 17 insertions(+), 32 deletions(-) diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index 20a8d8285f..d8e1291905 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -205,23 +205,6 @@ void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable) memory_region_set_enabled(&xive->end_source.esb_mmio, false); } -/* - * When a Virtual Processor is scheduled to run on a HW thread, the - * hypervisor pushes its identifier in the OS CAM line. Emulate the - * same behavior under QEMU. - */ -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx) -{ - uint8_t nvt_blk; - uint32_t nvt_idx; - uint32_t nvt_cam; - - spapr_xive_cpu_to_nvt(POWERPC_CPU(tctx->cs), &nvt_blk, &nvt_idx); - - nvt_cam = cpu_to_be32(TM_QW1W2_VO | xive_nvt_cam_line(nvt_blk, nvt_idx)); - memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4); -} - static void spapr_xive_end_reset(XiveEND *end) { memset(end, 0, sizeof(*end)); @@ -544,21 +527,32 @@ static int spapr_xive_cpu_intc_create(SpaprInterruptController *intc, } spapr_cpu->tctx = XIVE_TCTX(obj); - - /* - * (TCG) Early setting the OS CAM line for hotplugged CPUs as they - * don't beneficiate from the reset of the XIVE IRQ backend - */ - spapr_xive_set_tctx_os_cam(spapr_cpu->tctx); return 0; } +static void xive_tctx_set_os_cam(XiveTCTX *tctx, uint32_t os_cam) +{ + uint32_t qw1w2 = cpu_to_be32(TM_QW1W2_VO | os_cam); + memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &qw1w2, 4); +} + static void spapr_xive_cpu_intc_reset(SpaprInterruptController *intc, PowerPCCPU *cpu) { XiveTCTX *tctx = spapr_cpu_state(cpu)->tctx; + uint8_t nvt_blk; + uint32_t nvt_idx; xive_tctx_reset(tctx); + + /* + * When a Virtual Processor is scheduled to run on a HW thread, + * the hypervisor pushes its identifier in the OS CAM line. + * Emulate the same behavior under QEMU. + */ + spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx); + + xive_tctx_set_os_cam(tctx, xive_nvt_cam_line(nvt_blk, nvt_idx)); } static void spapr_xive_set_irq(SpaprInterruptController *intc, int irq, int val) @@ -651,14 +645,6 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers, static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp) { SpaprXive *xive = SPAPR_XIVE(intc); - CPUState *cs; - - CPU_FOREACH(cs) { - PowerPCCPU *cpu = POWERPC_CPU(cs); - - /* (TCG) Set the OS CAM line of the thread interrupt context. */ - spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx); - } if (kvm_enabled()) { int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp); diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h index d84bd5c229..742b7e834f 100644 --- a/include/hw/ppc/spapr_xive.h +++ b/include/hw/ppc/spapr_xive.h @@ -57,7 +57,6 @@ typedef struct SpaprXive { void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon); void spapr_xive_hcall_init(SpaprMachineState *spapr); -void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx); void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool enable); void spapr_xive_map_mmio(SpaprXive *xive); -- cgit v1.2.3