[Skiboot] [PATCH] hdata: Move 'HRMOR_BIT' macro to header file

[Vasant Hegde]

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 at linux.vnet.ibm.com> wrote:
>>> Its already defined twice. Lets move it to header file.
>>>
>>> Signed-off-by: Vasant Hegde <hegdevasant at 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