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

[Ryan Grimm]

On Wed, 2020-03-04 at 18:25 +1100, Alexey Kardashevskiy 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?

The latest version of the HDAT spec renames mirror attributes to memory
attributes including mirroring bits like before and also SMF.

mirror_start is unchanged, so, I'll try to document this better.

-Ryan

> 
> >  	__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? Anyway this bit seems unrelated to the other half of
> the
> patch about "secure-memory".
> 

I don't know about broken HDAT, we just piggybacked on this code.  Let
me check with HB team and make this look better.

-Ryan

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