| Message ID | 20191115162436.30548-2-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | ppc/pnv: add XIVE support for KVM guests | expand |
On Fri, 15 Nov 2019 17:24:14 +0100 Cédric Le Goater <clg@kaod.org> wrote: > When an interrupt can not be presented to a vCPU, because it is not > running on any of the HW treads, the XIVE presenter updates the > Interrupt Pending Buffer register of the associated XIVE NVT > structure. This is only done if backlog is activated in the END but > this is generally the case. > > The current code assumes that the fields of the NVT structure is > architected with the same layout of the thread interrupt context > registers. Fix this assumption and define an offset for the IPB > register backup value in the NVT. > Does this fix a visible bug in the powernv machine ? If so, maybe worth describing the symptoms. Anyway, this seems conforment to the XIVE spec I have, so FWIW: Reviewed-by: Greg Kurz <groug@kaod.org> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > include/hw/ppc/xive_regs.h | 1 + > hw/intc/xive.c | 11 +++++++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h > index 55307cd1533c..530f232b04f8 100644 > --- a/include/hw/ppc/xive_regs.h > +++ b/include/hw/ppc/xive_regs.h > @@ -255,6 +255,7 @@ typedef struct XiveNVT { > uint32_t w2; > uint32_t w3; > uint32_t w4; > +#define NVT_W4_IPB PPC_BITMASK32(16, 23) > uint32_t w5; > uint32_t w6; > uint32_t w7; > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 3d472e29c858..177663d2b43e 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, > * - logical server : forward request to IVPE (not supported) > */ > if (xive_end_is_backlog(&end)) { > + uint8_t ipb; > + > if (format == 1) { > qemu_log_mask(LOG_GUEST_ERROR, > "XIVE: END %x/%x invalid config: F1 & backlog\n", > end_blk, end_idx); > return; > } > - /* Record the IPB in the associated NVT structure */ > - ipb_update((uint8_t *) &nvt.w4, priority); > + /* > + * Record the IPB in the associated NVT structure for later > + * use. The presenter will resend the interrupt when the vCPU > + * is dispatched again on a HW thread. > + */ > + ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority); > + nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb); > xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4); > > /*
On 18/11/2019 16:44, Greg Kurz wrote: > On Fri, 15 Nov 2019 17:24:14 +0100 > Cédric Le Goater <clg@kaod.org> wrote: > >> When an interrupt can not be presented to a vCPU, because it is not >> running on any of the HW treads, the XIVE presenter updates the >> Interrupt Pending Buffer register of the associated XIVE NVT >> structure. This is only done if backlog is activated in the END but >> this is generally the case. >> >> The current code assumes that the fields of the NVT structure is >> architected with the same layout of the thread interrupt context >> registers. Fix this assumption and define an offset for the IPB >> register backup value in the NVT. >> > > Does this fix a visible bug in the powernv machine ? If so, > maybe worth describing the symptoms. no. this is a preliminary work for escalation suppport and to dump the contents of the NVT table. > Anyway, this seems conforment to the XIVE spec I have, so FWIW: FYI, the NVT structure evolves in the XIVE2 specs. C. > Reviewed-by: Greg Kurz <groug@kaod.org> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> include/hw/ppc/xive_regs.h | 1 + >> hw/intc/xive.c | 11 +++++++++-- >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h >> index 55307cd1533c..530f232b04f8 100644 >> --- a/include/hw/ppc/xive_regs.h >> +++ b/include/hw/ppc/xive_regs.h >> @@ -255,6 +255,7 @@ typedef struct XiveNVT { >> uint32_t w2; >> uint32_t w3; >> uint32_t w4; >> +#define NVT_W4_IPB PPC_BITMASK32(16, 23) >> uint32_t w5; >> uint32_t w6; >> uint32_t w7; >> diff --git a/hw/intc/xive.c b/hw/intc/xive.c >> index 3d472e29c858..177663d2b43e 100644 >> --- a/hw/intc/xive.c >> +++ b/hw/intc/xive.c >> @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, >> * - logical server : forward request to IVPE (not supported) >> */ >> if (xive_end_is_backlog(&end)) { >> + uint8_t ipb; >> + >> if (format == 1) { >> qemu_log_mask(LOG_GUEST_ERROR, >> "XIVE: END %x/%x invalid config: F1 & backlog\n", >> end_blk, end_idx); >> return; >> } >> - /* Record the IPB in the associated NVT structure */ >> - ipb_update((uint8_t *) &nvt.w4, priority); >> + /* >> + * Record the IPB in the associated NVT structure for later >> + * use. The presenter will resend the interrupt when the vCPU >> + * is dispatched again on a HW thread. >> + */ >> + ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority); >> + nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb); >> xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4); >> >> /* >
On Fri, Nov 15, 2019 at 05:24:14PM +0100, Cédric Le Goater wrote: > When an interrupt can not be presented to a vCPU, because it is not > running on any of the HW treads, the XIVE presenter updates the > Interrupt Pending Buffer register of the associated XIVE NVT > structure. This is only done if backlog is activated in the END but > this is generally the case. > > The current code assumes that the fields of the NVT structure is > architected with the same layout of the thread interrupt context > registers. Fix this assumption and define an offset for the IPB > register backup value in the NVT. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Applied to ppc-for-5.0, thanks. > --- > include/hw/ppc/xive_regs.h | 1 + > hw/intc/xive.c | 11 +++++++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h > index 55307cd1533c..530f232b04f8 100644 > --- a/include/hw/ppc/xive_regs.h > +++ b/include/hw/ppc/xive_regs.h > @@ -255,6 +255,7 @@ typedef struct XiveNVT { > uint32_t w2; > uint32_t w3; > uint32_t w4; > +#define NVT_W4_IPB PPC_BITMASK32(16, 23) > uint32_t w5; > uint32_t w6; > uint32_t w7; > diff --git a/hw/intc/xive.c b/hw/intc/xive.c > index 3d472e29c858..177663d2b43e 100644 > --- a/hw/intc/xive.c > +++ b/hw/intc/xive.c > @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, > * - logical server : forward request to IVPE (not supported) > */ > if (xive_end_is_backlog(&end)) { > + uint8_t ipb; > + > if (format == 1) { > qemu_log_mask(LOG_GUEST_ERROR, > "XIVE: END %x/%x invalid config: F1 & backlog\n", > end_blk, end_idx); > return; > } > - /* Record the IPB in the associated NVT structure */ > - ipb_update((uint8_t *) &nvt.w4, priority); > + /* > + * Record the IPB in the associated NVT structure for later > + * use. The presenter will resend the interrupt when the vCPU > + * is dispatched again on a HW thread. > + */ > + ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority); > + nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb); > xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4); > > /*
diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h index 55307cd1533c..530f232b04f8 100644 --- a/include/hw/ppc/xive_regs.h +++ b/include/hw/ppc/xive_regs.h @@ -255,6 +255,7 @@ typedef struct XiveNVT { uint32_t w2; uint32_t w3; uint32_t w4; +#define NVT_W4_IPB PPC_BITMASK32(16, 23) uint32_t w5; uint32_t w6; uint32_t w7; diff --git a/hw/intc/xive.c b/hw/intc/xive.c index 3d472e29c858..177663d2b43e 100644 --- a/hw/intc/xive.c +++ b/hw/intc/xive.c @@ -1607,14 +1607,21 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk, * - logical server : forward request to IVPE (not supported) */ if (xive_end_is_backlog(&end)) { + uint8_t ipb; + if (format == 1) { qemu_log_mask(LOG_GUEST_ERROR, "XIVE: END %x/%x invalid config: F1 & backlog\n", end_blk, end_idx); return; } - /* Record the IPB in the associated NVT structure */ - ipb_update((uint8_t *) &nvt.w4, priority); + /* + * Record the IPB in the associated NVT structure for later + * use. The presenter will resend the interrupt when the vCPU + * is dispatched again on a HW thread. + */ + ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority); + nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb); xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4); /*
When an interrupt can not be presented to a vCPU, because it is not running on any of the HW treads, the XIVE presenter updates the Interrupt Pending Buffer register of the associated XIVE NVT structure. This is only done if backlog is activated in the END but this is generally the case. The current code assumes that the fields of the NVT structure is architected with the same layout of the thread interrupt context registers. Fix this assumption and define an offset for the IPB register backup value in the NVT. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- include/hw/ppc/xive_regs.h | 1 + hw/intc/xive.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-)