Message ID | 20191024155539.12045-1-hegdevasant@linux.vnet.ibm.com |
---|---|
State | Accepted |
Headers | show |
Series | hdata: Fix reserved node label search | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (d75e82dbfbb9443efeb3f9a5921ac23605aab469) |
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Fri, Oct 25, 2019 at 2:55 AM Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: > > Soon hostboot/HDAT will provide consistent reserved node name. It will > just provide node name without starting string "ibm,". Commit 50d508c3 > made sure that all device tree nodes starts with "ibm,". > > But we use hostboot/HDAT provided name for `ibm,prd-label` property. So we > have to fix couple of our `ibm,prd-label` property based search/comparision > to accommodate this change. > > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> Merged as c3bfa320955967c943348dc526828de3d278fdbc, that said... > --- > external/opal-prd/opal-prd.c | 13 +++++++++---- > hdata/tpmrel.c | 10 +++++++--- > 2 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c > index b49d9ae6d..1dc7691a8 100644 > --- a/external/opal-prd/opal-prd.c > +++ b/external/opal-prd/opal-prd.c > @@ -122,7 +122,8 @@ static struct opal_prd_ctx *ctx; > > static const char *opal_prd_devnode = "/dev/opal-prd"; > static const char *opal_prd_socket = "/run/opal-prd-control"; > -static const char *hbrt_code_region_name = "ibm,hbrt-code-image"; > +static const char *hbrt_code_region_name = "hbrt-code-image"; > +static const char *hbrt_code_region_name_ibm = "ibm,hbrt-code-image"; > static const int opal_prd_version = 1; > static uint64_t opal_prd_ipoll = 0xf000000000000000; > > @@ -2193,9 +2194,13 @@ static int run_prd_daemon(struct opal_prd_ctx *ctx) > } else { > rc = map_hbrt_physmem(ctx, hbrt_code_region_name); > if (rc) { > - pr_log(LOG_ERR, "IMAGE: Can't access hbrt " > - "physical memory"); > - goto out_close; > + /* Fallback to old style ibm,prd-label */ > + rc = map_hbrt_physmem(ctx, hbrt_code_region_name_ibm); We should probably add the prefix to "hbrt-code-region" so we don't break existing opal-prd binaries. It should be safe since hbrt-code-region is consumed by opal-prd instead of HBRT itself.
On 11/8/19 10:31 AM, Oliver O'Halloran wrote: > On Fri, Oct 25, 2019 at 2:55 AM Vasant Hegde > <hegdevasant@linux.vnet.ibm.com> wrote: >> >> Soon hostboot/HDAT will provide consistent reserved node name. It will >> just provide node name without starting string "ibm,". Commit 50d508c3 >> made sure that all device tree nodes starts with "ibm,". >> >> But we use hostboot/HDAT provided name for `ibm,prd-label` property. So we >> have to fix couple of our `ibm,prd-label` property based search/comparision >> to accommodate this change. >> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > > Merged as c3bfa320955967c943348dc526828de3d278fdbc, that said... > >> --- >> external/opal-prd/opal-prd.c | 13 +++++++++---- >> hdata/tpmrel.c | 10 +++++++--- >> 2 files changed, 16 insertions(+), 7 deletions(-) >> >> diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c >> index b49d9ae6d..1dc7691a8 100644 >> --- a/external/opal-prd/opal-prd.c >> +++ b/external/opal-prd/opal-prd.c >> @@ -122,7 +122,8 @@ static struct opal_prd_ctx *ctx; >> >> static const char *opal_prd_devnode = "/dev/opal-prd"; >> static const char *opal_prd_socket = "/run/opal-prd-control"; >> -static const char *hbrt_code_region_name = "ibm,hbrt-code-image"; >> +static const char *hbrt_code_region_name = "hbrt-code-image"; >> +static const char *hbrt_code_region_name_ibm = "ibm,hbrt-code-image"; >> static const int opal_prd_version = 1; >> static uint64_t opal_prd_ipoll = 0xf000000000000000; >> >> @@ -2193,9 +2194,13 @@ static int run_prd_daemon(struct opal_prd_ctx *ctx) >> } else { >> rc = map_hbrt_physmem(ctx, hbrt_code_region_name); >> if (rc) { >> - pr_log(LOG_ERR, "IMAGE: Can't access hbrt " >> - "physical memory"); >> - goto out_close; >> + /* Fallback to old style ibm,prd-label */ >> + rc = map_hbrt_physmem(ctx, hbrt_code_region_name_ibm); > > We should probably add the prefix to "hbrt-code-region" so we don't > break existing opal-prd binaries. It should be safe since > hbrt-code-region is consumed by opal-prd instead of HBRT itself. > Sorry. I should have explained it better. If we add `ibm,*` to prd-label property then it becomes another workaround..which I wanted to avoid. For P9 lifetime, hostboot will continue to add `ibm,` for these reserved nodes so that we don't break backward compatibility with distros. This is more of preparation for next generation chips.. where hostboot will just provide consistent name without adding `ibm,`. -Vasant
diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c index b49d9ae6d..1dc7691a8 100644 --- a/external/opal-prd/opal-prd.c +++ b/external/opal-prd/opal-prd.c @@ -122,7 +122,8 @@ static struct opal_prd_ctx *ctx; static const char *opal_prd_devnode = "/dev/opal-prd"; static const char *opal_prd_socket = "/run/opal-prd-control"; -static const char *hbrt_code_region_name = "ibm,hbrt-code-image"; +static const char *hbrt_code_region_name = "hbrt-code-image"; +static const char *hbrt_code_region_name_ibm = "ibm,hbrt-code-image"; static const int opal_prd_version = 1; static uint64_t opal_prd_ipoll = 0xf000000000000000; @@ -2193,9 +2194,13 @@ static int run_prd_daemon(struct opal_prd_ctx *ctx) } else { rc = map_hbrt_physmem(ctx, hbrt_code_region_name); if (rc) { - pr_log(LOG_ERR, "IMAGE: Can't access hbrt " - "physical memory"); - goto out_close; + /* Fallback to old style ibm,prd-label */ + rc = map_hbrt_physmem(ctx, hbrt_code_region_name_ibm); + if (rc) { + pr_log(LOG_ERR, "IMAGE: Can't access hbrt " + "physical memory"); + goto out_close; + } } dump_hbrt_map(ctx); } diff --git a/hdata/tpmrel.c b/hdata/tpmrel.c index 1fe14c75e..8bfc0f8fc 100644 --- a/hdata/tpmrel.c +++ b/hdata/tpmrel.c @@ -153,10 +153,14 @@ static void tpmrel_cvc_init(struct HDIF_common_hdr *hdif_hdr) if (!node) return; - cvc_reserved_mem = get_hb_reserved_memory("ibm,secure-crypt-algo-code"); + cvc_reserved_mem = get_hb_reserved_memory("secure-crypt-algo-code"); if (!cvc_reserved_mem) { - prlog(PR_ERR, "CVC reserved memory not found\n"); - return; + /* Fallback to old style ibm,prd-label */ + cvc_reserved_mem = get_hb_reserved_memory("ibm,secure-crypt-algo-code"); + if (!cvc_reserved_mem) { + prlog(PR_ERR, "CVC reserved memory not found\n"); + return; + } } parent = dt_new(node, "ibm,cvc");
Soon hostboot/HDAT will provide consistent reserved node name. It will just provide node name without starting string "ibm,". Commit 50d508c3 made sure that all device tree nodes starts with "ibm,". But we use hostboot/HDAT provided name for `ibm,prd-label` property. So we have to fix couple of our `ibm,prd-label` property based search/comparision to accommodate this change. Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- external/opal-prd/opal-prd.c | 13 +++++++++---- hdata/tpmrel.c | 10 +++++++--- 2 files changed, 16 insertions(+), 7 deletions(-)