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

[Vasant Hegde]

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'm fine with making above changes.

@Nick, any thoughts on above changes?


-Vasant