| Message ID | 1492729197.25766.156.camel@kernel.crashing.org |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, 2017-04-21 at 08:59 +1000, Benjamin Herrenschmidt wrote: > We don't trigger the IPI in set_mfrr() if the CPPR of the > destination is more favored than the MFRR. However, we fail > to re-evaluate it when the CPPR later changes. > > This fixes it. The way this is implemented can lead to > spurious IPIs but these are harmless. > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > hw/xive.c | 35 +++++++++++++++++++++-------------- > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/hw/xive.c b/hw/xive.c > index 1e2648c..eb75f5f 100644 > --- a/hw/xive.c > +++ b/hw/xive.c > @@ -3066,6 +3066,24 @@ static inline uint8_t opal_xive_check_pending(struct > xive_cpu_state *xs, > return xs->pending & mask; > } > > +static void opal_xive_update_cppr(struct xive_cpu_state *xs, u8 cppr) > +{ > + /* Peform the update */ > + xs->cppr = cppr; > + out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); > + > + /* Trigger the IPI if it's still more favored than the CPPR > + * > + * This can lead to a bunch of spurrious retriggers if the > + * IPI is queued up behind other interrupts but that's not > + * a big deal and keeps the code simpler > + */ > + if (xs->mfrr < cppr) { > + xive_ipi_trigger(xs->xive, GIRQ_TO_IDX(xs->ipi_irq)); > + xs->ipi_sent = true; What tree is this against? mainline gets this. [CC] hw/xive.o hw/xive.c: In function ‘opal_xive_update_cppr’: hw/xive.c:3083:5: error: ‘struct xive_cpu_state’ has no member named ‘ipi_sent’; did you mean ‘ipi_irq’? xs->ipi_sent = true; ^~ > + } > +} > + > static int64_t opal_xive_eoi(uint32_t xirr) > { > struct cpu_thread *c = this_cpu(); > @@ -3150,13 +3168,6 @@ static int64_t opal_xive_eoi(uint32_t xirr) > /* Is it an IPI ? */ > if (special_ipi) { > xive_ipi_eoi(src_x, idx); > - > - /* Check mfrr and eventually re-trigger. We check > - * against the new CPPR since we are about to update > - * the HW. > - */ > - if (xs->mfrr < cppr) > - xive_ipi_trigger(src_x, idx); > } else { > /* Otherwise go through the source mechanism */ > xive_vdbg(src_x, "EOI of IDX %x in EXT range\n", > idx); > @@ -3167,8 +3178,7 @@ static int64_t opal_xive_eoi(uint32_t xirr) > } > > /* Finally restore CPPR */ > - xs->cppr = cppr; > - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); > + opal_xive_update_cppr(xs, cppr); > > xive_cpu_vdbg(c, " pending=0x%x cppr=%d\n", xs->pending, cppr); > > @@ -3297,8 +3307,7 @@ static int64_t opal_xive_get_xirr(uint32_t *out_xirr, > bool just_poll) > > } else { > /* Nothing was active, this is a fluke, restore CPPR */ > - xs->cppr = old_cppr; > - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, old_cppr); > + opal_xive_update_cppr(xs, old_cppr); > xive_cpu_vdbg(c, " nothing active, restored CPPR to %d\n", > old_cppr); > } > @@ -3328,9 +3337,7 @@ static int64_t opal_xive_set_cppr(uint8_t cppr) > xive_cpu_vdbg(c, "CPPR setting to %d\n", cppr); > > lock(&xs->lock); > - c->xstate->cppr = cppr; > - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); > - > + opal_xive_update_cppr(xs, cppr); > unlock(&xs->lock); > > return OPAL_SUCCESS; >
This is upstream as of 50e1921f98 On Fri, 2017-04-21 at 08:59 +1000, Benjamin Herrenschmidt wrote: > We don't trigger the IPI in set_mfrr() if the CPPR of the > destination is more favored than the MFRR. However, we fail > to re-evaluate it when the CPPR later changes. > > This fixes it. The way this is implemented can lead to > spurious IPIs but these are harmless. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > hw/xive.c | 35 +++++++++++++++++++++-------------- > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/hw/xive.c b/hw/xive.c > index 1e2648c..eb75f5f 100644 > --- a/hw/xive.c > +++ b/hw/xive.c > @@ -3066,6 +3066,24 @@ static inline uint8_t opal_xive_check_pending(struct > xive_cpu_state *xs, > return xs->pending & mask; > } > > +static void opal_xive_update_cppr(struct xive_cpu_state *xs, u8 cppr) > +{ > + /* Peform the update */ > + xs->cppr = cppr; > + out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); > + > + /* Trigger the IPI if it's still more favored than the CPPR > + * > + * This can lead to a bunch of spurrious retriggers if the > + * IPI is queued up behind other interrupts but that's not > + * a big deal and keeps the code simpler > + */ > + if (xs->mfrr < cppr) { > + xive_ipi_trigger(xs->xive, GIRQ_TO_IDX(xs->ipi_irq)); > + xs->ipi_sent = true; > + } > +} > + > static int64_t opal_xive_eoi(uint32_t xirr) > { > struct cpu_thread *c = this_cpu(); > @@ -3150,13 +3168,6 @@ static int64_t opal_xive_eoi(uint32_t xirr) > /* Is it an IPI ? */ > if (special_ipi) { > xive_ipi_eoi(src_x, idx); > - > - /* Check mfrr and eventually re-trigger. We check > - * against the new CPPR since we are about to update > - * the HW. > - */ > - if (xs->mfrr < cppr) > - xive_ipi_trigger(src_x, idx); > } else { > /* Otherwise go through the source mechanism */ > xive_vdbg(src_x, "EOI of IDX %x in EXT range\n", > idx); > @@ -3167,8 +3178,7 @@ static int64_t opal_xive_eoi(uint32_t xirr) > } > > /* Finally restore CPPR */ > - xs->cppr = cppr; > - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); > + opal_xive_update_cppr(xs, cppr); > > xive_cpu_vdbg(c, " pending=0x%x cppr=%d\n", xs->pending, cppr); > > @@ -3297,8 +3307,7 @@ static int64_t opal_xive_get_xirr(uint32_t *out_xirr, > bool just_poll) > > } else { > /* Nothing was active, this is a fluke, restore CPPR */ > - xs->cppr = old_cppr; > - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, old_cppr); > + opal_xive_update_cppr(xs, old_cppr); > xive_cpu_vdbg(c, " nothing active, restored CPPR to %d\n", > old_cppr); > } > @@ -3328,9 +3337,7 @@ static int64_t opal_xive_set_cppr(uint8_t cppr) > xive_cpu_vdbg(c, "CPPR setting to %d\n", cppr); > > lock(&xs->lock); > - c->xstate->cppr = cppr; > - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); > - > + opal_xive_update_cppr(xs, cppr); > unlock(&xs->lock); > > return OPAL_SUCCESS; > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
diff --git a/hw/xive.c b/hw/xive.c index 1e2648c..eb75f5f 100644 --- a/hw/xive.c +++ b/hw/xive.c @@ -3066,6 +3066,24 @@ static inline uint8_t opal_xive_check_pending(struct xive_cpu_state *xs, return xs->pending & mask; } +static void opal_xive_update_cppr(struct xive_cpu_state *xs, u8 cppr) +{ + /* Peform the update */ + xs->cppr = cppr; + out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); + + /* Trigger the IPI if it's still more favored than the CPPR + * + * This can lead to a bunch of spurrious retriggers if the + * IPI is queued up behind other interrupts but that's not + * a big deal and keeps the code simpler + */ + if (xs->mfrr < cppr) { + xive_ipi_trigger(xs->xive, GIRQ_TO_IDX(xs->ipi_irq)); + xs->ipi_sent = true; + } +} + static int64_t opal_xive_eoi(uint32_t xirr) { struct cpu_thread *c = this_cpu(); @@ -3150,13 +3168,6 @@ static int64_t opal_xive_eoi(uint32_t xirr) /* Is it an IPI ? */ if (special_ipi) { xive_ipi_eoi(src_x, idx); - - /* Check mfrr and eventually re-trigger. We check - * against the new CPPR since we are about to update - * the HW. - */ - if (xs->mfrr < cppr) - xive_ipi_trigger(src_x, idx); } else { /* Otherwise go through the source mechanism */ xive_vdbg(src_x, "EOI of IDX %x in EXT range\n", idx); @@ -3167,8 +3178,7 @@ static int64_t opal_xive_eoi(uint32_t xirr) } /* Finally restore CPPR */ - xs->cppr = cppr; - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); + opal_xive_update_cppr(xs, cppr); xive_cpu_vdbg(c, " pending=0x%x cppr=%d\n", xs->pending, cppr); @@ -3297,8 +3307,7 @@ static int64_t opal_xive_get_xirr(uint32_t *out_xirr, bool just_poll) } else { /* Nothing was active, this is a fluke, restore CPPR */ - xs->cppr = old_cppr; - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, old_cppr); + opal_xive_update_cppr(xs, old_cppr); xive_cpu_vdbg(c, " nothing active, restored CPPR to %d\n", old_cppr); } @@ -3328,9 +3337,7 @@ static int64_t opal_xive_set_cppr(uint8_t cppr) xive_cpu_vdbg(c, "CPPR setting to %d\n", cppr); lock(&xs->lock); - c->xstate->cppr = cppr; - out_8(xs->tm_ring1 + TM_QW3_HV_PHYS + TM_CPPR, cppr); - + opal_xive_update_cppr(xs, cppr); unlock(&xs->lock); return OPAL_SUCCESS;
We don't trigger the IPI in set_mfrr() if the CPPR of the destination is more favored than the MFRR. However, we fail to re-evaluate it when the CPPR later changes. This fixes it. The way this is implemented can lead to spurious IPIs but these are harmless. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- hw/xive.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)