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

[Vasant Hegde]

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.
   - Also if future if we support early OPAL crashes then we can't really wait 
for kernel to
     reserve memory.

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


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.


> 
>> diff --git a/core/opal-dump.c b/core/opal-dump.c
>> new file mode 100644
>> index 000000000..dc364fab1
>> --- /dev/null
>> +++ b/core/opal-dump.c
>> @@ -0,0 +1,143 @@
>> +/* Copyright 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.
>> + * You may obtain a copy of the License at
>> + *
>> + *	http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#define pr_fmt(fmt)	"DUMP: " fmt
>> +
>> +#include <device.h>
>> +#include <mem-map.h>
>> +#include <mem_region.h>
>> +#include <mem_region-malloc.h>
>> +#include <opal.h>
>> +#include <opal-dump.h>
>> +#include <opal-internal.h>
>> +#include <skiboot.h>
>> +
>> +#include <ccan/endian/endian.h>
>> +
>> +#include "hdata/spira.h"
>> +
>> +/* Actual address of MDST and MDDT table */
>> +#define MDST_TABLE_BASE		(SKIBOOT_BASE + MDST_TABLE_OFF)
>> +#define MDDT_TABLE_BASE		(SKIBOOT_BASE + MDDT_TABLE_OFF)
>> +
>> +static struct spira_ntuple *ntuple_mdst;
>> +static struct spira_ntuple *ntuple_mddt;
>> +static struct spira_ntuple *ntuple_mdrt;
>> +
>> +static int opal_mpipl_add_entry(u8 region, u64 src, u64 dest, u64 size)
>> +{
>> +	int i, max_cnt;
>> +	struct mdst_table *mdst;
>> +	struct mddt_table *mddt;
>> +
>> +	max_cnt = MDST_TABLE_SIZE / sizeof(struct mdst_table);
>> +	if (ntuple_mdst->act_cnt >= max_cnt) {
>> +		prlog(PR_DEBUG, "MDST table is full\n");
>> +		return OPAL_RESOURCE;
>> +	}
>> +
>> +	max_cnt = MDDT_TABLE_SIZE / sizeof(struct mddt_table);
>> +	if (ntuple_mdst->act_cnt >= max_cnt) {
>> +		prlog(PR_DEBUG, "MDDT table is full\n");
>> +		return OPAL_RESOURCE;
>> +	}
>> +
>> +	/* Use relocated memory address */
>> +	mdst = (void *)(MDST_TABLE_BASE);
>> +	mddt = (void *)(MDDT_TABLE_BASE);
>> +
>> +	/* Check for duplicate entry */
>> +	for (i = 0; i < ntuple_mdst->act_cnt; i++) {
>> +		if (mdst->addr == (src | HRMOR_BIT)) {
>> +			prlog(PR_DEBUG,
>> +			      "Duplicate source address : 0x%llx", src);
>> +			return OPAL_PARAMETER;
>> +		}
>> +		mdst++;
>> +	}
>> +	for (i = 0; i < ntuple_mddt->act_cnt; i++) {
>> +		if (mddt->addr == (dest | HRMOR_BIT)) {
>> +			prlog(PR_DEBUG,
>> +			      "Duplicate destination address : 0x%llx", dest);
>> +			return OPAL_PARAMETER;
>> +		}
>> +		mddt++;
>> +	}
>> +
>> +	/* Add OPAL source address to MDST entry */
>> +	mdst->addr = src | HRMOR_BIT;
>> +	mdst->data_region = region;
>> +	mdst->size = size;
>> +	ntuple_mdst->act_cnt++;
>> +
>> +	/* Add OPAL destination address to MDDT entry */
>> +	mddt->addr = dest | HRMOR_BIT;
>> +	mddt->data_region = region;
>> +	mddt->size = size;
>> +	ntuple_mddt->act_cnt++;
>> +
>> +	prlog(PR_TRACE, "Added new entry. src : 0x%llx, dest : 0x%llx,"
>> +	      " size : 0x%llx\n", src, dest, size);
>> +	return OPAL_SUCCESS;
>> +}
>> +
>> +/* Register for OPAL dump.  */
>> +static void opal_mpipl_register(void)
>> +{
>> +	u64 opal_dest, opal_size;
>> +
>> +	/* Get OPAL runtime size */
>> +	if (!dt_find_property(opal_node, "opal-runtime-size")) {
>> +		prlog(PR_DEBUG, "Could not get OPAL runtime size\n");
>> +		return;
>> +	}
> 
> As an aside, is it best practice to read your own device tree for this
> kind of thing? As opposed to just exporting it from the code which set
> the dt entry.

I think its fine to read from DT (we have been doing that) instead of creating
so many global variables.

-Vasant