[Skiboot] [RFC PATCH v5 09/16] hdata/memory.c: Parse HDAT for secure memory

[Oliver O'Halloran]

On Wed, Mar 4, 2020 at 6:25 PM Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
>
>
>
> On 28/02/2020 07:40, Ryan Grimm wrote:
> > The secure memory ranges are provided by the hostboot through HDAT.
> > Skiboot parses HDAT and creates secure-memory@ device tree nodes.
> >
> > Check bit 15 when checking for reserves that are too big so we reserve
> > regions from HB that are in secure memory.
> >
> > Signed-off-by: Ryan Grimm <grimm at linux.ibm.com>
> > ---
> >  hdata/memory.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/hdata/memory.c b/hdata/memory.c
> > index 9c588ff6..c25bd9c0 100644
> > --- a/hdata/memory.c
> > +++ b/hdata/memory.c
> > @@ -32,7 +32,7 @@ struct HDIF_ms_area_address_range {
> >       __be64 start;
> >       __be64 end;
> >       __be32 chip;
> > -     __be32 mirror_attr;
> > +     __be32 memory_attr;
>
> Why is this renamed?
>
> >       __be64 mirror_start;
>
> and this one is not?
>
> >       __be32 controller_id;
> >       __be32 phys_attr;
> > @@ -59,6 +59,9 @@ struct HDIF_ms_area_address_range {
> >  #define MS_CONTROLLER_MCS_ID(id)     GETFIELD(PPC_BITMASK32(4, 7), id)
> >  #define MS_CONTROLLER_MCA_ID(id)     GETFIELD(PPC_BITMASK32(8, 15), id)
> >
> > +#define MS_ATTR_PEF                  PPC_BIT32(23)
> > +#define UV_SECURE_MEM_BIT            PPC_BIT(15)
> > +
> >  struct HDIF_ms_area_id {
> >       __be16 id;
> >  #define MS_PTYPE_RISER_CARD  0x8000
> > @@ -129,10 +132,10 @@ static bool add_address_range(struct dt_node *root,
> >       chip_id = pcid_to_chip_id(be32_to_cpu(arange->chip));
> >
> >       prlog(PR_DEBUG, "  Range: 0x%016llx..0x%016llx "
> > -           "on Chip 0x%x mattr: 0x%x pattr: 0x%x status:0x%x\n",
> > +           "on Chip 0x%x memattr: 0x%08x pattr: 0x%x status:0x%x\n",
> >             (long long)be64_to_cpu(arange->start),
> >             (long long)be64_to_cpu(arange->end),
> > -           chip_id, be32_to_cpu(arange->mirror_attr),
> > +           chip_id, be32_to_cpu(arange->memory_attr),
> >             mem_type, mem_status);
> >
> >       /* reg contains start and length */
> > @@ -161,6 +164,13 @@ static bool add_address_range(struct dt_node *root,
> >               return false;
> >       }
> >
> > +     if (be32_to_cpu(arange->memory_attr) & MS_ATTR_PEF) {
> > +             prlog(PR_DEBUG, "HDAT: Found secure memory\n");
> > +             name = "secure-memory";
> > +             dev_type = "secure-memory";
> > +             compat = "ibm,secure-memory";
> > +     }
> > +
> >       if (be16_to_cpu(id->flags) & MS_AREA_SHARED) {
> >               mem = dt_find_by_name_addr(dt_root, name, reg[0]);
> >               if (mem) {
> > @@ -674,9 +684,10 @@ static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd)
> >
> >               /*
> >                * Workaround broken HDAT reserve regions which are
> > -              * bigger than 512MB
> > +              * bigger than 512MB and not secure memory
> >                */
> > -             if ((end_addr - start_addr) > 0x20000000) {
> > +             if (((end_addr - start_addr) > 0x20000000) &&
> > +                     !(start_addr & UV_SECURE_MEM_BIT)) {
>
>
> Is this a hostboot bug? Where does this broken HDAT come from and can we
> fix it there?

It was a hostboot bug that turned up some time early in p9 bringup
that never seemed to get fixed so I worked around it instead. Maybe
it's been fixed by now, but honestly I don't really care.

> Anyway this bit seems unrelated to the other half of the
> patch about "secure-memory".

I think it's a carry over from earlier versions of this patchset which
put the secure memory ranges into /reserved-memory/ rather than having
actual memory nodes. I don't think there's any real need to support
giant reservations in secure memory so this should probably be
dropped.

> >                       prlog(PR_ERR, "MEM: Ignoring Bad HDAT reserve: too big\n");
> >                       continue;
> >               }
> >
>
> --
> Alexey