[Skiboot] [PATCH v8 20/24] MPIPL: Add OPAL API to query saved tags

[Vasant Hegde]

On 07/10/2019 03:20 PM, Vasant Hegde wrote:
> On 07/10/2019 09:22 AM, Michael Neuling wrote:
>> On Tue, 2019-07-09 at 20:53 +0530, Vasant Hegde wrote:
>>> On 07/09/2019 03:33 PM, Oliver O'Halloran wrote:
>>>> On Sun, 2019-06-16 at 22:40 +0530, Vasant Hegde wrote:
>>>>> Pre-MPIPL kernel saves various information required to create vmcore in
>>>>> metadata area and passes metadata area pointer to OPAL. OPAL will preserve
>>>>> this pointer across MPIPL. Post MPIPL kernel will request for saved tags
>>>>> via this API. Kernel also needs below tags:
>>>>>     - Saved CPU registers data to access CPU registers
>>>>>     - OPAL metadata area to create opalcore
>>>>>
> 
> .../...
> 
>>>> We already communicate some of that
>>>> information via the fw-load-area DT property, but that only contains the
>>>> ranges where skiboot loads the kernel and initrd (0x2000_000...0x2fff_ffff).
>>>
>>> OS needs to know about these memory ranges *only*...as first kernel may use
>>> these
>>> memory. If it uses it should take care of reserving destination memory to
>>> preserve these
>>> memory and pass it during MPIPL registration.
>>>
>>>> Odds are hostboot doesn't use *that* much memory, but the petitboot kernel
>>>> can
>>>
>>> As mentioned above kernel need not worry about hostboot memory usage...as its
>>> part of reserved-memory ranges.
> 
> Mikey,
> 
>>
>> Vasant,
>>
>> I think you may have missed Oliver's point in the above.
> 
> Yes. I did miss the point of ABI -OR- endianess difference in petitboot and host 
> kernel.
> 
>>
>> The Petitboot kernel needs some amount space to operate. In your current patch
>> series, the amount of space "firmware" is allowed to use is hard-wired by the
>> crashing kernel. Olivers point is that this size should be defined dynamically
>> by firmware, not hard-wired into the crashing kernel.
> 
> Nope. Its not hard wired. But the problem with current patch series is petitboot 
> kernel
> goes and reads host Linux metadata area to find the memory it can use to boot... 
> which
> is not correct.
> 
>>
>> If we every change petitboot or some other component of the firmware so that it
>> needs to use more memory, then the crashing kernel won't have any way of
>> discovering that and it won't allocate enough space for this process to start.
> 
> We do not have problem with rest of the firmware stack.. as they have their own
> load area and its reserved. Only issue is with petitboot kernel.
> 
> How about something like below :
>    - First boot : Host kernel will tell OPAL about amount of memory firmware 
> (including
>                         petitboot) can use during MPIPL boot
>        opal_mpipl_update(OPAL_MPIPL_BOOT_MEM_SIZE,  <size of memory petitboot 
> can use>)
>     - OPAL will preserve this across MPIPL
>    - Post MPIPL : Petitboot kernel will make a query to get usable size
>       opal_mpipl_query_tag(type, *tag)
>       type = GET_BOOT_SIZE , *tag = size (assumption is that kernel will load at 
> 0th address)
> 
> This means we have to again change the opal_mpipl_query_tag() API. We will replace
> `index` field with `type` field.
>    opal_mpipl_query_tag(type, *tag)
>     type = OPAL, Kernel tag, CPU tag, boot mem size tag etg
> 
> Post MPIPL boot, kernel will make opal_mpipl_query_tag with specific type field 
> to tag the
> data.

I think it makes sense to have separate API for tag registration instead of 
overloading
opal_mpipl_update API. So we will have 3 APIs

1 - opal_mpipl_update(ops, src, dest, size)
     enum opal_mpipl_ops {
         OPAL_MPIPL_ADD_RANGE            = 0,
         OPAL_MPIPL_REMOVE_RANGE         = 1,
         OPAL_MPIPL_REMOVE_ALL           = 2,
         OPAL_MPIPL_FREE_PRESERVED_MEMORY= 3,
};

enum opal_mpipl_tags {
         OPAL_MPIPL_TAG_CPU      = 0,
         OPAL_MPIPL_TAG_OPAL     = 1,
         OPAL_MPIPL_TAG_KERNEL   = 2,
         OPAL_MPIPL_TAG_BOOT_MEM = 3,
};

2 - opal_mpipl_register_tag(tag, tag_val)
     kernel will pass tag = OPAL_MPIPL_TAG_KERNEL and OPAL_MPIPL_TAG_BOOT_MEM

3 - opal_mpipl_query_tag(tag, *tag_val)
    Kernel will make this call for all the tags listed in opal_mpipl_tags.


-Vasant

> 
> I'm fine with making above changes.
> 
> @Nick, any thoughts on above changes?
> 
> 
> -Vasant
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
>