| Message ID | 20190616171024.22799-2-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MPIPL support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (dbf27b6c4af84addb36bd3be34f96580aba9c873) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | fail | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
Vasant Hegde's on June 17, 2019 3:10 am: > Needed for creating OPAL core file. > > Sample output: > -------------- > sys/firmware/devicetree/base/ibm,opal # lsprop > ... > opal-boot-address > 00000000 30002560 > ... Why is this needed? We can't derive it from the symbol table? Thanks, Nick
On 06/28/2019 06:18 AM, Nicholas Piggin wrote: > Vasant Hegde's on June 17, 2019 3:10 am: >> Needed for creating OPAL core file. >> >> Sample output: >> -------------- >> sys/firmware/devicetree/base/ibm,opal # lsprop >> ... >> opal-boot-address >> 00000000 30002560 >> ... > > Why is this needed? We can't derive it from the symbol table? Kernel needs this address to generate proper OPAL dump. We can read it from symbol table. But then kernel has to read OPAL symbol table and parse it to get the data. Hence I have added this property so that kernel can directly read DT to get entry address. -Vasant
On Fri, 2019-06-28 at 14:34 +0530, Vasant Hegde wrote: > On 06/28/2019 06:18 AM, Nicholas Piggin wrote: > > Vasant Hegde's on June 17, 2019 3:10 am: > > > Needed for creating OPAL core file. > > > > > > Sample output: > > > -------------- > > > sys/firmware/devicetree/base/ibm,opal # lsprop > > > ... > > > opal-boot-address > > > 00000000 30002560 > > > ... > > > > Why is this needed? We can't derive it from the symbol table? > > Kernel needs this address to generate proper OPAL dump. I looked at Hari's patches to try work out what a "proper dump" is. Looks like we use this to create the AT_ENTRY note for the opalcore which file. Sounds important, but I can't find where the same happens for the Linux vmcore, so maybe it's not? > We can read it > from symbol table. But then kernel has to read OPAL symbol table and > parse it to get the data. You know... if we punted generating the opalcore to userspace rather than doing it in the kernel this wouldn't be an issue ;) > Hence I have added this property so that kernel can directly read DT > to get entry address. The value you're reading out of the DT is the value for the currently running skiboot rather than the one that crashed so this fishy to begin with. We have two architected entry points: fdt_entry at SKIBOOT_BASE+0x10 and hdat_entry at SKIBOOT_BASE+0x180. If you have to put this in the DT then use one of those. Oliver
On 07/09/2019 03:23 PM, Oliver O'Halloran wrote: > On Fri, 2019-06-28 at 14:34 +0530, Vasant Hegde wrote: >> On 06/28/2019 06:18 AM, Nicholas Piggin wrote: >>> Vasant Hegde's on June 17, 2019 3:10 am: >>>> Needed for creating OPAL core file. >>>> >>>> Sample output: >>>> -------------- >>>> sys/firmware/devicetree/base/ibm,opal # lsprop >>>> ... >>>> opal-boot-address >>>> 00000000 30002560 >>>> ... >>> >>> Why is this needed? We can't derive it from the symbol table? >> > >> Kernel needs this address to generate proper OPAL dump. > > I looked at Hari's patches to try work out what a "proper dump" > is. Looks like we use this to create the AT_ENTRY note for the > opalcore which file. Sounds important, but I can't find where > the same happens for the Linux vmcore, so maybe it's not? > vmcore is analyzed by crash.. which may not need AT_ENTRY. Hari can confirm. >> We can read it >> from symbol table. But then kernel has to read OPAL symbol table and >> parse it to get the data. > > You know... if we punted generating the opalcore to userspace rather > than doing it in the kernel this wouldn't be an issue ;) We did it in kernel so that it fits to existing dump flow. (kernel will generate dump -> kdump will offload it -> reboot system) > >> Hence I have added this property so that kernel can directly read DT >> to get entry address. > > The value you're reading out of the DT is the value for the currently > running skiboot rather than the one that crashed so this fishy to begin > with. AFAIK entry point is fixed for give skiboot LID. Now across MPIPL boot we don't expect to change skiboot LID (specially in production systems). So I think we are good here. > > We have two architected entry points: fdt_entry at SKIBOOT_BASE+0x10 > and hdat_entry at SKIBOOT_BASE+0x180. If you have to put this in the DT > then use one of those. These are hardcoded entry points used during boot depending on the type of entry (fdt_entry when hostboot passes DT, hdat_entry if it expect OPAL to generate DT). AFAIK this won't help in core generation. We need boot entry point. -Vasant
On 09/07/19 7:48 PM, Vasant Hegde wrote: > On 07/09/2019 03:23 PM, Oliver O'Halloran wrote: >> On Fri, 2019-06-28 at 14:34 +0530, Vasant Hegde wrote: >>> On 06/28/2019 06:18 AM, Nicholas Piggin wrote: >>>> Vasant Hegde's on June 17, 2019 3:10 am: >>>>> Needed for creating OPAL core file. >>>>> >>>>> Sample output: >>>>> -------------- >>>>> sys/firmware/devicetree/base/ibm,opal # lsprop >>>>> ... >>>>> opal-boot-address >>>>> 00000000 30002560 >>>>> ... >>>> >>>> Why is this needed? We can't derive it from the symbol table? >>> >> >>> Kernel needs this address to generate proper OPAL dump. >> >> I looked at Hari's patches to try work out what a "proper dump" >> is. Looks like we use this to create the AT_ENTRY note for the >> opalcore which file. Sounds important, but I can't find where >> the same happens for the Linux vmcore, so maybe it's not? >> > > vmcore is analyzed by crash.. which may not need AT_ENTRY. > Hari can confirm. > Yeah. opalcore needs to rely on GDB while we have a utility called crash built on top of GDB to handle vmcore. GDB needs this note to understand a typical core while crash utility handles it for vmcore... Thanks Hari
diff --git a/core/opal.c b/core/opal.c index 3a2fbb95b..17d14a966 100644 --- a/core/opal.c +++ b/core/opal.c @@ -1,4 +1,4 @@ -/* Copyright 2013-2014 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. @@ -390,6 +390,7 @@ void add_opal_node(void) { uint64_t base, entry, size; extern uint32_t opal_entry; + extern uint32_t boot_entry; struct dt_node *opal_event; /* XXX TODO: Reorg this. We should create the base OPAL @@ -419,6 +420,7 @@ void add_opal_node(void) dt_add_property_cells(opal_node, "opal-msg-size", OPAL_MSG_SIZE); dt_add_property_u64(opal_node, "opal-base-address", base); dt_add_property_u64(opal_node, "opal-entry-address", entry); + dt_add_property_u64(opal_node, "opal-boot-address", (uint64_t)&boot_entry); dt_add_property_u64(opal_node, "opal-runtime-size", size); /* Add irqchip interrupt controller */
Needed for creating OPAL core file. Sample output: -------------- sys/firmware/devicetree/base/ibm,opal # lsprop ... opal-boot-address 00000000 30002560 ... Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- core/opal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)