| Message ID | 20200723163757.1878994-1-clg@kaod.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xive/p9: Enforce thread enablement | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (abe4c4799ffee4be12674ad59fc0bc521b0724f3) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | fail | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Fri, Jul 24, 2020 at 2:38 AM Cédric Le Goater <clg@kaod.org> wrote: > > Make sure that the enablement has completed to guarantee that the TIMA > accesses will see the latest state of the enable register. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/xive.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/hw/xive.c b/hw/xive.c > index 8d6095c02e7e..9a8d91482589 100644 > --- a/hw/xive.c > +++ b/hw/xive.c > @@ -2741,6 +2741,7 @@ static void xive_reset_enable_thread(struct cpu_thread *c) > struct proc_chip *chip = get_chip(c->chip_id); > struct xive *x = chip->xive; > uint32_t fc, bit; > + uint64_t enable; > > /* Get fused core number */ > fc = (c->pir >> 3) & 0xf; > @@ -2752,9 +2753,16 @@ static void xive_reset_enable_thread(struct cpu_thread *c) > if (fc < 8) { > xive_regw(x, PC_THREAD_EN_REG0_CLR, PPC_BIT(bit)); > xive_regw(x, PC_THREAD_EN_REG0_SET, PPC_BIT(bit)); > + > + enable = xive_regr(x, PC_THREAD_EN_REG0, PPC_BIT(bit)); This doesn't build with this patch applied. I assume the last argument isn't supposed to be there? > + if (!(enable & PPC_BIT(bit))) > + xive_cpu_err(c, "Failed to enable thread\n"); > } else { > xive_regw(x, PC_THREAD_EN_REG1_CLR, PPC_BIT(bit)); > xive_regw(x, PC_THREAD_EN_REG1_SET, PPC_BIT(bit)); > + enable = xive_regr(x, PC_THREAD_EN_REG1, PPC_BIT(bit)); same issue > + if (!(enable & PPC_BIT(bit))) > + xive_cpu_err(c, "Failed to enable thread\n"); > } > } > > -- > 2.25.4 >
On 8/4/20 8:33 AM, Oliver O'Halloran wrote: > On Fri, Jul 24, 2020 at 2:38 AM Cédric Le Goater <clg@kaod.org> wrote: >> >> Make sure that the enablement has completed to guarantee that the TIMA >> accesses will see the latest state of the enable register. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/xive.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/hw/xive.c b/hw/xive.c >> index 8d6095c02e7e..9a8d91482589 100644 >> --- a/hw/xive.c >> +++ b/hw/xive.c >> @@ -2741,6 +2741,7 @@ static void xive_reset_enable_thread(struct cpu_thread *c) >> struct proc_chip *chip = get_chip(c->chip_id); >> struct xive *x = chip->xive; >> uint32_t fc, bit; >> + uint64_t enable; >> >> /* Get fused core number */ >> fc = (c->pir >> 3) & 0xf; >> @@ -2752,9 +2753,16 @@ static void xive_reset_enable_thread(struct cpu_thread *c) >> if (fc < 8) { >> xive_regw(x, PC_THREAD_EN_REG0_CLR, PPC_BIT(bit)); >> xive_regw(x, PC_THREAD_EN_REG0_SET, PPC_BIT(bit)); >> + >> + enable = xive_regr(x, PC_THREAD_EN_REG0, PPC_BIT(bit)); > > This doesn't build with this patch applied. I assume the last argument > isn't supposed to be there? ouch. I sent you a crappy patch. v2 on his way. Sorry about that. C. > >> + if (!(enable & PPC_BIT(bit))) >> + xive_cpu_err(c, "Failed to enable thread\n"); >> } else { >> xive_regw(x, PC_THREAD_EN_REG1_CLR, PPC_BIT(bit)); >> xive_regw(x, PC_THREAD_EN_REG1_SET, PPC_BIT(bit)); >> + enable = xive_regr(x, PC_THREAD_EN_REG1, PPC_BIT(bit)); > > same issue > >> + if (!(enable & PPC_BIT(bit))) >> + xive_cpu_err(c, "Failed to enable thread\n"); >> } >> } >> >> -- >> 2.25.4 >>
diff --git a/hw/xive.c b/hw/xive.c index 8d6095c02e7e..9a8d91482589 100644 --- a/hw/xive.c +++ b/hw/xive.c @@ -2741,6 +2741,7 @@ static void xive_reset_enable_thread(struct cpu_thread *c) struct proc_chip *chip = get_chip(c->chip_id); struct xive *x = chip->xive; uint32_t fc, bit; + uint64_t enable; /* Get fused core number */ fc = (c->pir >> 3) & 0xf; @@ -2752,9 +2753,16 @@ static void xive_reset_enable_thread(struct cpu_thread *c) if (fc < 8) { xive_regw(x, PC_THREAD_EN_REG0_CLR, PPC_BIT(bit)); xive_regw(x, PC_THREAD_EN_REG0_SET, PPC_BIT(bit)); + + enable = xive_regr(x, PC_THREAD_EN_REG0, PPC_BIT(bit)); + if (!(enable & PPC_BIT(bit))) + xive_cpu_err(c, "Failed to enable thread\n"); } else { xive_regw(x, PC_THREAD_EN_REG1_CLR, PPC_BIT(bit)); xive_regw(x, PC_THREAD_EN_REG1_SET, PPC_BIT(bit)); + enable = xive_regr(x, PC_THREAD_EN_REG1, PPC_BIT(bit)); + if (!(enable & PPC_BIT(bit))) + xive_cpu_err(c, "Failed to enable thread\n"); } }
Make sure that the enablement has completed to guarantee that the TIMA accesses will see the latest state of the enable register. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/xive.c | 8 ++++++++ 1 file changed, 8 insertions(+)