[Skiboot] [PATCH] core/cpu.c: Use a device-tree node to detect nest mmu presence

[Alistair Popple]

Thanks Balbir,

On Tue, 6 Dec 2016 10:39:25 PM Balbir Singh wrote:
> On Mon, Dec 5, 2016 at 2:56 PM, Alistair Popple <alistair at popple.id.au> wrote:
> > The nest mmu address scom was hardcoded which could lead to boot
> > failure on POWER9 systems without a nest mmu. For example Mambo
> > doesn't model the nest mmu which results in the following failure when
> > calling opal_nmmu_set_ptcr() during kernel load:
> >
> > WARNING: 20856759: (20856757): Invalid address 0x0000000028096258 in XSCOM range,  SCOM=0x00280962b
> > WARNING: 20856759: (20856757): Attempt to store non-existent address 0x00001A0028096258
> > 20856759: (20856757): 0x000000003002DA08 : stdcix  r26,r0,r3
> > FATAL ERROR: 20856759: (20856757): Check Stop for 0:0: Machine Check with ME bit of MSR off
> >
> > This patch instead reads the address from the device-tree and makes
> > opal_nmmu_set_ptcr return OPAL_UNSUPPORTED on systems without a nest
> > mmu.
> >
> > Signed-off-by: Alistair Popple <alistair at popple.id.au>
> > ---
> >  core/cpu.c | 36 ++++++++++++++++++++++--------------
> >  1 file changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/core/cpu.c b/core/cpu.c
> > index f83d910..5d76adf 100644
> > --- a/core/cpu.c
> > +++ b/core/cpu.c
> > @@ -1084,6 +1084,15 @@ static int64_t opal_reinit_cpus(uint64_t flags)
> >  }
> >  opal_call(OPAL_REINIT_CPUS, opal_reinit_cpus, 1);
> >
> > +#define NMMU_XLAT_CTL_PTCR 0xb
> > +static int64_t nmmu_set_ptcr(uint64_t chip_id, struct dt_node *node, uint64_t ptcr)
> > +{
> > +       uint32_t nmmu_base_addr;
> > +
> > +       nmmu_base_addr = dt_get_address(node, 0, NULL);
> > +       return xscom_write(chip_id, nmmu_base_addr + NMMU_XLAT_CTL_PTCR, ptcr);
> 
> Seems reasonable, do we need to check if nmmu_base_addr is valid?

How do we check if it is valid? Either the device-tree has a valid address in
the "ibm,power9-nest-mmu" or it doesn't. We have no way of working out if the
address is valid unless there is a "ibm,power9-nest-mmu" node with no "reg"
property, but in that case dt_get_address() will crash skiboot with an assert
failure as that means the device-tree is just plain wrong.

In summary I don't think we need to (nor can) check if it's valid.

> > +}
> > +
> >  /*
> >   * Setup the the Nest MMU PTCR register for all chips in the system or
> >   * the specified chip id.
> > @@ -1091,25 +1100,24 @@ opal_call(OPAL_REINIT_CPUS, opal_reinit_cpus, 1);
> >   * The PTCR value may be overwritten so long as all users have been
> >   * quiesced. If it is set to an invalid memory address the system will
> >   * checkstop if anything attempts to use it.
> > + *
> > + * Returns OPAL_UNSUPPORTED if no nest mmu was found.
> >   */
> > -#define NMMU_CFG_XLAT_CTL_PTCR 0x5012c4b
> >  static int64_t opal_nmmu_set_ptcr(uint64_t chip_id, uint64_t ptcr)
> >  {
> > -       struct proc_chip *chip;
> > -       int64_t rc = OPAL_PARAMETER;
> > -
> > -       if (proc_gen != proc_gen_p9)
> > -               return OPAL_UNSUPPORTED;
> > +       struct dt_node *node;
> > +       int64_t rc = OPAL_UNSUPPORTED;
> >
> >         if (chip_id == -1ULL)
> > -               for_each_chip(chip)
> > -                       rc = xscom_write(chip->id, NMMU_CFG_XLAT_CTL_PTCR, ptcr);
> > -       else {
> > -               if (!(chip = get_chip(chip_id)))
> > -                       return OPAL_PARAMETER;
> > -
> > -               rc = xscom_write(chip->id, NMMU_CFG_XLAT_CTL_PTCR, ptcr);
> > -       }
> > +               dt_for_each_compatible(dt_root, node, "ibm,power9-nest-mmu") {
> > +                       chip_id = dt_get_chip_id(node);
> > +                       if ((rc = nmmu_set_ptcr(chip_id, node, ptcr)))
> > +                               return rc;
> > +               }
> > +       else
> > +               dt_for_each_compatible_on_chip(dt_root, node, "ibm,power9-nest-mmu", chip_id)
> 
> I presume this property will be passed on to skiboot?

Yes, either it will need to be derived from the HDAT or passed in via some
other platform fixup.

- Alistair

> > +                       if ((rc = nmmu_set_ptcr(chip_id, node, ptcr)))
> > +                               return rc;
> >
> >         return rc;
> >  }
> 
> Balbir Singh.
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
>