On 11/30/2017 05:00 AM, David Gibson wrote: > On Thu, Nov 23, 2017 at 02:29:45PM +0100, Cédric Le Goater wrote: >> The Pending Interrupt Priority Register (PIPR) contains the priority >> of the most favored pending notification. It is calculated from the >> Interrupt Pending Buffer (IPB) which indicates a pending interrupt at >> the priority corresponding to the bit number. >> >> If the PIPR is more favored (1) than the Current Processor Priority >> Register (CPPR), the CPU interrupt line is raised and the EO bit of >> the Notification Source Register is updated to notify the presence of >> an exception for the O/S. The check needs to be done whenever the PIPR >> or the CPPR is changed. >> >> Then, the O/S Exception is raised and the O/S acknowledges the >> interrupt with a special read in the TIMA. If the EO bit of the >> Notification Source Register (NSR) is set (and it should), the Current >> Processor Priority Register (CPPR) takes the value of the Pending >> Interrupt Priority Register (PIPR). The bit number in the Interrupt >> Pending Buffer (IPB) corresponding to the priority of the pending >> interrupt is reseted and so is the EO bit of the NSR. >> >> (1) numerically less than >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> --- >> hw/intc/spapr_xive.c | 77 >> +++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 76 insertions(+), 1 deletion(-) >> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c >> index df14c5a88275..fead9c7031f3 100644 >> --- a/hw/intc/spapr_xive.c >> +++ b/hw/intc/spapr_xive.c >> @@ -39,9 +39,63 @@ struct sPAPRXiveICP { >> XiveEQ eqt[XIVE_PRIORITY_MAX + 1]; >> }; >> >> +/* Convert a priority number to an Interrupt Pending Buffer (IPB) >> + * register, which indicates a pending interrupt at the priority >> + * corresponding to the bit number >> + */ >> +static uint8_t priority_to_ipb(uint8_t priority) >> +{ >> + return priority > XIVE_PRIORITY_MAX ? >> + 0 : 1 << (XIVE_PRIORITY_MAX - priority); > > Does handling out of bounds values here make sense, or should you just > assert() they're not passed in?
Looking at the code, I think we could assert, yes. I need to check the SET_OS_PENDING command first. >> +} >> + >> +/* Convert an Interrupt Pending Buffer (IPB) register to a Pending >> + * Interrupt Priority Register (PIPR), which contains the priority of >> + * the most favored pending notification. >> + * >> + * TODO: >> + * >> + * PIPR is clamped to CPPR. So the value in the PIPR is: >> + * >> + * v = leftmost_bit_of(ipb) (or 0xff); >> + * pipr = v < cppr ? v : cppr; >> + * >> + * Ben says: "which means it's never actually 0xff ... surprise !". >> + * But, the CPPR can be set to 0xFF ... I am confused ... > > A resolution to this would be nice.. That's on my TODO list. Not a big issue. >> + */ >> +static uint8_t ipb_to_pipr(uint8_t ibp) >> +{ >> + return ibp ? clz32((uint32_t)ibp << 24) : 0xff; >> +} >> + >> static uint64_t spapr_xive_icp_accept(sPAPRXiveICP *icp) >> { >> - return 0; >> + uint8_t nsr = icp->tima_os[TM_NSR]; >> + >> + qemu_irq_lower(icp->output); >> + >> + if (icp->tima_os[TM_NSR] & TM_QW1_NSR_EO) { >> + uint8_t cppr = icp->tima_os[TM_PIPR]; >> + >> + icp->tima_os[TM_CPPR] = cppr; >> + >> + /* Reset the pending buffer bit */ >> + icp->tima_os[TM_IPB] &= ~priority_to_ipb(cppr); > > What if multiple irqs of the same priority were queued? When an interrupt is EOI'ed, the queue is scanned to check for any pending interrupts. If so, a replay is forced with a call to force_external_irq_replay() C. > >> + icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]); >> + >> + /* Drop Exception bit for OS */ >> + icp->tima_os[TM_NSR] &= ~TM_QW1_NSR_EO; >> + } >> + >> + return (nsr << 8) | icp->tima_os[TM_CPPR]; >> +} >> + >> +static void spapr_xive_icp_notify(sPAPRXiveICP *icp) >> +{ >> + if (icp->tima_os[TM_PIPR] < icp->tima_os[TM_CPPR]) { >> + icp->tima_os[TM_NSR] |= TM_QW1_NSR_EO; >> + qemu_irq_raise(icp->output); >> + } >> } >> >> static void spapr_xive_icp_set_cppr(sPAPRXiveICP *icp, uint8_t cppr) >> @@ -51,6 +105,9 @@ static void spapr_xive_icp_set_cppr(sPAPRXiveICP *icp, >> uint8_t cppr) >> } >> >> icp->tima_os[TM_CPPR] = cppr; >> + >> + /* CPPR has changed, inform the ICP which might raise an exception */ >> + spapr_xive_icp_notify(icp); >> } >> >> /* >> @@ -224,6 +281,8 @@ static void spapr_xive_irq(sPAPRXive *xive, int lisn) >> XiveEQ *eq; >> uint32_t eq_idx; >> uint8_t priority; >> + uint32_t server; >> + sPAPRXiveICP *icp; >> >> ive = spapr_xive_get_ive(xive, lisn); >> if (!ive || !(ive->w & IVE_VALID)) { >> @@ -253,6 +312,13 @@ static void spapr_xive_irq(sPAPRXive *xive, int lisn) >> qemu_log_mask(LOG_UNIMP, "XIVE: !UCOND_NOTIFY not implemented\n"); >> } >> >> + server = GETFIELD(EQ_W6_NVT_INDEX, eq->w6); >> + icp = spapr_xive_icp_get(xive, server); >> + if (!icp) { >> + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: No ICP for server %d\n", >> server); >> + return; >> + } >> + >> if (GETFIELD(EQ_W6_FORMAT_BIT, eq->w6) == 0) { >> priority = GETFIELD(EQ_W7_F0_PRIORITY, eq->w7); >> >> @@ -260,9 +326,18 @@ static void spapr_xive_irq(sPAPRXive *xive, int lisn) >> if (priority == 0xff) { >> g_assert_not_reached(); >> } >> + >> + /* Update the IPB (Interrupt Pending Buffer) with the priority >> + * of the new notification and inform the ICP, which will >> + * decide to raise the exception, or not, depending the CPPR. >> + */ >> + icp->tima_os[TM_IPB] |= priority_to_ipb(priority); >> + icp->tima_os[TM_PIPR] = ipb_to_pipr(icp->tima_os[TM_IPB]); >> } else { >> qemu_log_mask(LOG_UNIMP, "XIVE: w7 format1 not implemented\n"); >> } >> + >> + spapr_xive_icp_notify(icp); >> } >> >> /* >