| Message ID | 20180315074356.31473-1-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hdata: Move 'HRMOR_BIT' macro to header file | expand |
On Thu, Mar 15, 2018 at 6:43 PM, Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: > Its already defined twice. Lets move it to header file. > > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > --- > hdata/memory.c | 2 -- > hdata/spira.c | 5 ++--- > hdata/spira.h | 6 ++++++ > 3 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/hdata/memory.c b/hdata/memory.c > index 27dc559f5..a83898955 100644 > --- a/hdata/memory.c > +++ b/hdata/memory.c > @@ -602,8 +602,6 @@ static struct dt_node *add_hb_reserve_node(const char *name, u64 start, u64 end) > return node; > } > > -#define HRMOR_BIT (1ul << 63) > - > static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) > { > const struct msvpd_hb_reserved_mem *hb_resv_mem; > diff --git a/hdata/spira.c b/hdata/spira.c > index 0724dcc42..19f456a1e 100644 > --- a/hdata/spira.c > +++ b/hdata/spira.c > @@ -88,16 +88,15 @@ __section(".cpuctrl.data") struct cpu_ctl_init_data cpu_ctl_init_data = { > * To help the FSP distinguishing between TCE tokens and actual physical > * addresses, we set the top bit to 1 on physical addresses > */ > -#define ADDR_TOP_BIT (1ul << 63) > > __section(".mdst.data") struct dump_mdst_table init_mdst_table[2] = { > { > - .addr = CPU_TO_BE64(INMEM_CON_START | ADDR_TOP_BIT), > + .addr = CPU_TO_BE64(INMEM_CON_START | HRMOR_BIT), > .type = CPU_TO_BE32(DUMP_REGION_CONSOLE), > .size = CPU_TO_BE32(INMEM_CON_LEN), > }, > { > - .addr = CPU_TO_BE64(HBRT_CON_START | ADDR_TOP_BIT), > + .addr = CPU_TO_BE64(HBRT_CON_START | HRMOR_BIT), > .type = CPU_TO_BE32(DUMP_REGION_HBRT_LOG), > .size = CPU_TO_BE32(HBRT_CON_LEN), > }, > diff --git a/hdata/spira.h b/hdata/spira.h > index 39e0e3333..d8421dcb6 100644 > --- a/hdata/spira.h > +++ b/hdata/spira.h > @@ -19,6 +19,12 @@ > > #include "hdif.h" > > +/* > + * To help the FSP/hostboot distinguishing between physical address and relative > + * address/TCE tokens, we set the top bit to 1 on physical addresses. > + */ > +#define HRMOR_BIT (1ul << 63) I don't understand this comment at all. On the host having bit 63 (ppc bit 0 set) causes the core to ignore the HRMOR setting when in hypervisor real mode. As far as I know this is why hostboot sets it. In OPAL we always run with HRMOR set to zero so it doesn't do anything for us (hence removing it), but if the FSP treats it as some kind of flag then that usage needs to be more clearly documented here. > /* > * The SPIRA structure > * > -- > 2.14.3 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
On 03/19/2018 07:59 AM, Oliver wrote: > On Thu, Mar 15, 2018 at 6:43 PM, Vasant Hegde > <hegdevasant@linux.vnet.ibm.com> wrote: >> Its already defined twice. Lets move it to header file. >> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >> --- >> hdata/memory.c | 2 -- >> hdata/spira.c | 5 ++--- >> hdata/spira.h | 6 ++++++ >> 3 files changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/hdata/memory.c b/hdata/memory.c >> index 27dc559f5..a83898955 100644 >> --- a/hdata/memory.c >> +++ b/hdata/memory.c >> @@ -602,8 +602,6 @@ static struct dt_node *add_hb_reserve_node(const char *name, u64 start, u64 end) >> return node; >> } >> >> -#define HRMOR_BIT (1ul << 63) >> - >> static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) >> { >> const struct msvpd_hb_reserved_mem *hb_resv_mem; >> diff --git a/hdata/spira.c b/hdata/spira.c >> index 0724dcc42..19f456a1e 100644 >> --- a/hdata/spira.c >> +++ b/hdata/spira.c >> @@ -88,16 +88,15 @@ __section(".cpuctrl.data") struct cpu_ctl_init_data cpu_ctl_init_data = { >> * To help the FSP distinguishing between TCE tokens and actual physical >> * addresses, we set the top bit to 1 on physical addresses >> */ >> -#define ADDR_TOP_BIT (1ul << 63) >> >> __section(".mdst.data") struct dump_mdst_table init_mdst_table[2] = { >> { >> - .addr = CPU_TO_BE64(INMEM_CON_START | ADDR_TOP_BIT), >> + .addr = CPU_TO_BE64(INMEM_CON_START | HRMOR_BIT), >> .type = CPU_TO_BE32(DUMP_REGION_CONSOLE), >> .size = CPU_TO_BE32(INMEM_CON_LEN), >> }, >> { >> - .addr = CPU_TO_BE64(HBRT_CON_START | ADDR_TOP_BIT), >> + .addr = CPU_TO_BE64(HBRT_CON_START | HRMOR_BIT), >> .type = CPU_TO_BE32(DUMP_REGION_HBRT_LOG), >> .size = CPU_TO_BE32(HBRT_CON_LEN), >> }, >> diff --git a/hdata/spira.h b/hdata/spira.h >> index 39e0e3333..d8421dcb6 100644 >> --- a/hdata/spira.h >> +++ b/hdata/spira.h >> @@ -19,6 +19,12 @@ >> >> #include "hdif.h" >> > >> +/* >> + * To help the FSP/hostboot distinguishing between physical address and relative >> + * address/TCE tokens, we set the top bit to 1 on physical addresses. >> + */ >> +#define HRMOR_BIT (1ul << 63) > > I don't understand this comment at all. On the host having bit 63 (ppc > bit 0 set) causes > the core to ignore the HRMOR setting when in hypervisor real mode. As > far as I know this is > why hostboot sets it. AFAIK if we set this bit hostboot thinks its real address. Otherwise it uses HRMOR and payload_base to get actual address (At least that's how its working in dump collection path). I want to move this macro to header file ... So that I can use this bit while updating MDST and MDDT table for MPIPL case. > In OPAL we always run with HRMOR set to zero so > it doesn't do anything > for us (hence removing it), but if the FSP treats it as some kind of > flag then that usage needs > to be more clearly documented here. IIRC if top bit is set, then FSP thinks its real address. Otherwise its TCE mapped address. -Vasant
On 03/21/2018 01:33 PM, Vasant Hegde wrote: > On 03/19/2018 07:59 AM, Oliver wrote: >> On Thu, Mar 15, 2018 at 6:43 PM, Vasant Hegde >> <hegdevasant@linux.vnet.ibm.com> wrote: >>> Its already defined twice. Lets move it to header file. >>> >>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> >>> --- >>> hdata/memory.c | 2 -- >>> hdata/spira.c | 5 ++--- >>> hdata/spira.h | 6 ++++++ >>> 3 files changed, 8 insertions(+), 5 deletions(-) >>> >>> diff --git a/hdata/memory.c b/hdata/memory.c >>> index 27dc559f5..a83898955 100644 >>> --- a/hdata/memory.c >>> +++ b/hdata/memory.c >>> @@ -602,8 +602,6 @@ static struct dt_node *add_hb_reserve_node(const char >>> *name, u64 start, u64 end) >>> return node; >>> } >>> >>> -#define HRMOR_BIT (1ul << 63) >>> - >>> static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) >>> { >>> const struct msvpd_hb_reserved_mem *hb_resv_mem; >>> diff --git a/hdata/spira.c b/hdata/spira.c >>> index 0724dcc42..19f456a1e 100644 >>> --- a/hdata/spira.c >>> +++ b/hdata/spira.c >>> @@ -88,16 +88,15 @@ __section(".cpuctrl.data") struct cpu_ctl_init_data >>> cpu_ctl_init_data = { >>> * To help the FSP distinguishing between TCE tokens and actual physical >>> * addresses, we set the top bit to 1 on physical addresses >>> */ >>> -#define ADDR_TOP_BIT (1ul << 63) >>> >>> __section(".mdst.data") struct dump_mdst_table init_mdst_table[2] = { >>> { >>> - .addr = CPU_TO_BE64(INMEM_CON_START | ADDR_TOP_BIT), >>> + .addr = CPU_TO_BE64(INMEM_CON_START | HRMOR_BIT), >>> .type = CPU_TO_BE32(DUMP_REGION_CONSOLE), >>> .size = CPU_TO_BE32(INMEM_CON_LEN), >>> }, >>> { >>> - .addr = CPU_TO_BE64(HBRT_CON_START | ADDR_TOP_BIT), >>> + .addr = CPU_TO_BE64(HBRT_CON_START | HRMOR_BIT), >>> .type = CPU_TO_BE32(DUMP_REGION_HBRT_LOG), >>> .size = CPU_TO_BE32(HBRT_CON_LEN), >>> }, >>> diff --git a/hdata/spira.h b/hdata/spira.h >>> index 39e0e3333..d8421dcb6 100644 >>> --- a/hdata/spira.h >>> +++ b/hdata/spira.h >>> @@ -19,6 +19,12 @@ >>> >>> #include "hdif.h" >>> >> >>> +/* >>> + * To help the FSP/hostboot distinguishing between physical address and >>> relative >>> + * address/TCE tokens, we set the top bit to 1 on physical addresses. >>> + */ >>> +#define HRMOR_BIT (1ul << 63) >> >> I don't understand this comment at all. On the host having bit 63 (ppc >> bit 0 set) causes >> the core to ignore the HRMOR setting when in hypervisor real mode. As >> far as I know this is >> why hostboot sets it. > > AFAIK if we set this bit hostboot thinks its real address. Otherwise it uses > HRMOR and payload_base to get actual address (At least that's how its working in > dump collection path). > > I want to move this macro to header file ... So that I can use this bit while > updating MDST > and MDDT table for MPIPL case. > > >> In OPAL we always run with HRMOR set to zero so >> it doesn't do anything >> for us (hence removing it), but if the FSP treats it as some kind of >> flag then that usage needs >> to be more clearly documented here. > > IIRC if top bit is set, then FSP thinks its real address. Otherwise its TCE > mapped address. Confirmed with FSP folks. What I mentioned here is correct. May be I will update comment and resend the patch. -Vasant
diff --git a/hdata/memory.c b/hdata/memory.c index 27dc559f5..a83898955 100644 --- a/hdata/memory.c +++ b/hdata/memory.c @@ -602,8 +602,6 @@ static struct dt_node *add_hb_reserve_node(const char *name, u64 start, u64 end) return node; } -#define HRMOR_BIT (1ul << 63) - static void get_hb_reserved_mem(struct HDIF_common_hdr *ms_vpd) { const struct msvpd_hb_reserved_mem *hb_resv_mem; diff --git a/hdata/spira.c b/hdata/spira.c index 0724dcc42..19f456a1e 100644 --- a/hdata/spira.c +++ b/hdata/spira.c @@ -88,16 +88,15 @@ __section(".cpuctrl.data") struct cpu_ctl_init_data cpu_ctl_init_data = { * To help the FSP distinguishing between TCE tokens and actual physical * addresses, we set the top bit to 1 on physical addresses */ -#define ADDR_TOP_BIT (1ul << 63) __section(".mdst.data") struct dump_mdst_table init_mdst_table[2] = { { - .addr = CPU_TO_BE64(INMEM_CON_START | ADDR_TOP_BIT), + .addr = CPU_TO_BE64(INMEM_CON_START | HRMOR_BIT), .type = CPU_TO_BE32(DUMP_REGION_CONSOLE), .size = CPU_TO_BE32(INMEM_CON_LEN), }, { - .addr = CPU_TO_BE64(HBRT_CON_START | ADDR_TOP_BIT), + .addr = CPU_TO_BE64(HBRT_CON_START | HRMOR_BIT), .type = CPU_TO_BE32(DUMP_REGION_HBRT_LOG), .size = CPU_TO_BE32(HBRT_CON_LEN), }, diff --git a/hdata/spira.h b/hdata/spira.h index 39e0e3333..d8421dcb6 100644 --- a/hdata/spira.h +++ b/hdata/spira.h @@ -19,6 +19,12 @@ #include "hdif.h" +/* + * To help the FSP/hostboot distinguishing between physical address and relative + * address/TCE tokens, we set the top bit to 1 on physical addresses. + */ +#define HRMOR_BIT (1ul << 63) + /* * The SPIRA structure *
Its already defined twice. Lets move it to header file. Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- hdata/memory.c | 2 -- hdata/spira.c | 5 ++--- hdata/spira.h | 6 ++++++ 3 files changed, 8 insertions(+), 5 deletions(-)