[Skiboot] [PATCH v8 10/24] MPIPL: Register for OPAL dump

[Nicholas Piggin]

Vasant Hegde's on June 28, 2019 8:36 pm:
> On 06/28/2019 07:03 AM, Nicholas Piggin wrote:
>> Vasant Hegde's on June 17, 2019 3:10 am:
>>> This patch adds support to register for OPAL dump.
>>>    - Calculate memory required to capture OPAL dump
>>>    - Reserve OPAL dump destination memory
>>>    - Add OPAL dump details to MDST and MDDT table
>>>
>>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>>> ---
>>>   core/Makefile.inc   |   2 +-
>>>   core/init.c         |   6 ++-
>>>   core/opal-dump.c    | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/opal-dump.h |   4 ++
>>>   4 files changed, 153 insertions(+), 2 deletions(-)
>>>   create mode 100644 core/opal-dump.c
>>>
>>> diff --git a/core/Makefile.inc b/core/Makefile.inc
>>> index 21c12fb8d..c2c9731db 100644
>>> --- a/core/Makefile.inc
>>> +++ b/core/Makefile.inc
>>> @@ -10,7 +10,7 @@ CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o
>>>   CORE_OBJS += timer.o i2c.o rtc.o flash.o sensor.o ipmi-opal.o
>>>   CORE_OBJS += flash-subpartition.o bitmap.o buddy.o pci-quirk.o powercap.o psr.o
>>>   CORE_OBJS += pci-dt-slot.o direct-controls.o cpufeatures.o
>>> -CORE_OBJS += flash-firmware-versions.o
>>> +CORE_OBJS += flash-firmware-versions.o opal-dump.o
>>>   
>>>   ifeq ($(SKIBOOT_GCOV),1)
>>>   CORE_OBJS += gcov-profiling.o
>>> diff --git a/core/init.c b/core/init.c
>>> index 3db9df314..03776537e 100644
>>> --- a/core/init.c
>>> +++ b/core/init.c
>>> @@ -1,4 +1,4 @@
>>> -/* Copyright 2013-2016 IBM Corp.
>>> +/* Copyright 2013-2019 IBM Corp.
>>>    *
>>>    * Licensed under the Apache License, Version 2.0 (the "License");
>>>    * you may not use this file except in compliance with the License.
>>> @@ -55,6 +55,7 @@
>>>   #include <sbe-p9.h>
>>>   #include <debug_descriptor.h>
>>>   #include <occ.h>
>>> +#include <opal-dump.h>
>>>   
>>>   enum proc_gen proc_gen;
>>>   unsigned int pcie_max_link_speed;
>>> @@ -1173,6 +1174,9 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
>>>   	if (platform.init)
>>>   		platform.init();
>>>   
>>> +	/* init opal dump */
>>> +	opal_mpipl_init();
>>> +
>>>   	/* Read in NVRAM and set it up */
>>>   	nvram_init();
>>>   
>> 
>> So you're reserving memory and adding important OPAL memory to the MPIPL
>> ranges just in case, even if the host won't be using it?
> 
> Yes.
>    - This is to make sure each layer takes care of its reservation. OPAL should 
> take care of
>      memory reservation for OPAL/ kernel should take care of kernel memory 
> reservation.

Why though? The kernel is what has to activate the MPIPL and then put
together the dump after the reboot. Kernel knows whether or not it will
enable the feature. Why should OPAL take care of it?

>    - Also if future if we support early OPAL crashes then we can't really wait 
> for kernel to
>      reserve memory.

No, but that's not in this series.

>    - Kernel can always go and capture OPAL dump irrespective of dump method its 
> using.

I'm not sure what this means.

> 
> 
> Probably I should call opal_mpipl_init() bit late in the init path.
> 
>> 
>> It seems nicer if you would just advertise in the dt what the important
>> OPAL address ranges are, and the host can preserve them if it wants to
>> do OPAL dumps.
> 
> I don't think so.

Then most systems will just waste double memory on OPAL that they'll
never use.

Thanks,
Nick