| Message ID | 20180528035559.13422-3-joel@jms.id.au |
|---|---|
| State | Superseded |
| Headers | show |
| Series | BMC dumping | expand |
On 05/28/2018 09:25 AM, Joel Stanley wrote: > This provides an implementation of the OPAL_REGISTER_DUMP_REGION and > OPAL_UNREGISTER_DUMP_REGION calls that can be used by non-FSP systems, > and performs that registration for all astbmc platforms. > > This pointer will be used by eg. pdbg to fetch the host kernel's log > buffer over FSI. > > We unconditionally clear the value on a reboot (both fast-reboot and > reliable reboot) to reduce the chance of debug tools getting the wrong > buffer. > > Signed-off-by: Joel Stanley <joel@jms.id.au> Looks good to me. Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> -Vasant
On Mon, 2018-05-28 at 13:25 +0930, Joel Stanley wrote: > This provides an implementation of the OPAL_REGISTER_DUMP_REGION and > OPAL_UNREGISTER_DUMP_REGION calls that can be used by non-FSP systems, > and performs that registration for all astbmc platforms. > > This pointer will be used by eg. pdbg to fetch the host kernel's log > buffer over FSI. > > We unconditionally clear the value on a reboot (both fast-reboot and > reliable reboot) to reduce the chance of debug tools getting the wrong > buffer. I wouldn't make it BMC specific. Instead, I would suggest you: - Add "generic" OPAL calls that populate the debug descritor (ie, statically defined using OPAL_CALL). - Make the above call an optional machine.register_dump_region - Make FSP machines populate the above to *also* register with the FSP. That way we can also use/create similar tools (or just getmemproc) to get to the kernel dmesg via the debug descriptor on FSP systems. Cheers, Ben. > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > v2: > Rename dump.h to dump-region.h > Add comment to init.c > --- > core/Makefile.inc | 2 +- > core/dump-region.c | 61 +++++++++++++++++++++++++++++++++++++++ > core/init.c | 4 +++ > include/dump-region.h | 26 +++++++++++++++++ > platforms/astbmc/common.c | 4 +++ > 5 files changed, 96 insertions(+), 1 deletion(-) > create mode 100644 core/dump-region.c > create mode 100644 include/dump-region.h > > diff --git a/core/Makefile.inc b/core/Makefile.inc > index d36350590edb..2d2f74778f4c 100644 > --- a/core/Makefile.inc > +++ b/core/Makefile.inc > @@ -9,7 +9,7 @@ CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o > 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 += pci-dt-slot.o direct-controls.o cpufeatures.o dump-region.o > > ifeq ($(SKIBOOT_GCOV),1) > CORE_OBJS += gcov-profiling.o > diff --git a/core/dump-region.c b/core/dump-region.c > new file mode 100644 > index 000000000000..e42486f99ebf > --- /dev/null > +++ b/core/dump-region.c > @@ -0,0 +1,61 @@ > +/* Copyright 2018 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 <opal.h> > +#include <skiboot.h> > + > +#include <dump-region.h> > + > +static int64_t bmc_opal_register_dump_region(uint32_t id, > + uint64_t addr, uint64_t size) > +{ > + if (id != OPAL_DUMP_REGION_LOG_BUF) { > + prerror("Unsupported log region id %02x\n", id); > + return OPAL_UNSUPPORTED; > + } > + > + if (size <= 0) { > + prerror("Invalid log size %lld\n", size); > + return OPAL_PARAMETER; > + } > + > + prlog(PR_INFO, "Registered log buf at 0x%016llx\n", addr); > + debug_descriptor.log_buf_phys = addr; > + > + return OPAL_SUCCESS; > +} > + > +static int64_t bmc_opal_unregister_dump_region(uint32_t id) > +{ > + if (id != OPAL_DUMP_REGION_LOG_BUF) { > + prlog(PR_DEBUG, "Unsupported log region id %02x\n", id); > + return OPAL_UNSUPPORTED; > + } > + prlog(PR_INFO, "Unregistered log buf\n"); > + debug_descriptor.log_buf_phys = 0; > + > + return OPAL_SUCCESS; > +} > + > +void bmc_dump_region_init(void) > +{ > + opal_register(OPAL_REGISTER_DUMP_REGION, > + bmc_opal_register_dump_region, 3); > + opal_register(OPAL_UNREGISTER_DUMP_REGION, > + bmc_opal_unregister_dump_region, 1); > +}; > diff --git a/core/init.c b/core/init.c > index 3b887a24d11c..c1c352858e06 100644 > --- a/core/init.c > +++ b/core/init.c > @@ -563,6 +563,10 @@ void __noreturn load_and_boot_kernel(bool is_reboot) > > mem_dump_free(); > > + /* Zero out memory location before the next kernel starts, in case the > + * previous kernel did not unregister */ > + debug_descriptor.log_buf_phys = 0; > + > /* Take processours out of nap */ > cpu_set_sreset_enable(false); > cpu_set_ipi_enable(false); > diff --git a/include/dump-region.h b/include/dump-region.h > new file mode 100644 > index 000000000000..b629e66a85e6 > --- /dev/null > +++ b/include/dump-region.h > @@ -0,0 +1,26 @@ > +/* Copyright 2018 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. > + */ > + > +#ifndef __DUMP_H > +#define __DUMP_H > + > +/* > + * Initialise dump region registration OPAL calls for bmc systems. > + */ > +void bmc_dump_region_init(void); > + > +#endif > + > diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c > index 30c2714f94b9..ca8ec7e8c0ce 100644 > --- a/platforms/astbmc/common.c > +++ b/platforms/astbmc/common.c > @@ -26,6 +26,7 @@ > #include <bt.h> > #include <errorlog.h> > #include <lpc.h> > +#include <dump-region.h> > > #include "astbmc.h" > > @@ -156,6 +157,9 @@ void astbmc_init(void) > > /* Add BMC firmware info to device tree */ > ipmi_dt_add_bmc_info(); > + > + /* Enable dump region OPAL calls */ > + bmc_dump_region_init(); > } > > int64_t astbmc_ipmi_power_down(uint64_t request)
Digging this back up again as it would have been handy for some debugging yesterday. On Mon, 28 May 2018 at 13:19, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Mon, 2018-05-28 at 13:25 +0930, Joel Stanley wrote: > > This provides an implementation of the OPAL_REGISTER_DUMP_REGION and > > OPAL_UNREGISTER_DUMP_REGION calls that can be used by non-FSP systems, > > and performs that registration for all astbmc platforms. > > > > This pointer will be used by eg. pdbg to fetch the host kernel's log > > buffer over FSI. > > > > We unconditionally clear the value on a reboot (both fast-reboot and > > reliable reboot) to reduce the chance of debug tools getting the wrong > > buffer. > > I wouldn't make it BMC specific. > > Instead, I would suggest you: > > - Add "generic" OPAL calls that populate the debug descritor (ie, > statically defined using OPAL_CALL). > > - Make the above call an optional machine.register_dump_region > > - Make FSP machines populate the above to *also* register with the > FSP. > > That way we can also use/create similar tools (or just getmemproc) to > get to the kernel dmesg via the debug descriptor on FSP systems. Oliver, Vasant: are you happy with the patch as-is? Or shall we go down the route that benh suggested? The advantage of the patch as-is is existing kernels that boot on new skiboot will register the debug descriptor. Cheers, Joel > > Cheers, > Ben. > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > --- > > v2: > > Rename dump.h to dump-region.h > > Add comment to init.c > > --- > > core/Makefile.inc | 2 +- > > core/dump-region.c | 61 +++++++++++++++++++++++++++++++++++++++ > > core/init.c | 4 +++ > > include/dump-region.h | 26 +++++++++++++++++ > > platforms/astbmc/common.c | 4 +++ > > 5 files changed, 96 insertions(+), 1 deletion(-) > > create mode 100644 core/dump-region.c > > create mode 100644 include/dump-region.h > > > > diff --git a/core/Makefile.inc b/core/Makefile.inc > > index d36350590edb..2d2f74778f4c 100644 > > --- a/core/Makefile.inc > > +++ b/core/Makefile.inc > > @@ -9,7 +9,7 @@ CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o > > 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 += pci-dt-slot.o direct-controls.o cpufeatures.o dump-region.o > > > > ifeq ($(SKIBOOT_GCOV),1) > > CORE_OBJS += gcov-profiling.o > > diff --git a/core/dump-region.c b/core/dump-region.c > > new file mode 100644 > > index 000000000000..e42486f99ebf > > --- /dev/null > > +++ b/core/dump-region.c > > @@ -0,0 +1,61 @@ > > +/* Copyright 2018 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 <opal.h> > > +#include <skiboot.h> > > + > > +#include <dump-region.h> > > + > > +static int64_t bmc_opal_register_dump_region(uint32_t id, > > + uint64_t addr, uint64_t size) > > +{ > > + if (id != OPAL_DUMP_REGION_LOG_BUF) { > > + prerror("Unsupported log region id %02x\n", id); > > + return OPAL_UNSUPPORTED; > > + } > > + > > + if (size <= 0) { > > + prerror("Invalid log size %lld\n", size); > > + return OPAL_PARAMETER; > > + } > > + > > + prlog(PR_INFO, "Registered log buf at 0x%016llx\n", addr); > > + debug_descriptor.log_buf_phys = addr; > > + > > + return OPAL_SUCCESS; > > +} > > + > > +static int64_t bmc_opal_unregister_dump_region(uint32_t id) > > +{ > > + if (id != OPAL_DUMP_REGION_LOG_BUF) { > > + prlog(PR_DEBUG, "Unsupported log region id %02x\n", id); > > + return OPAL_UNSUPPORTED; > > + } > > + prlog(PR_INFO, "Unregistered log buf\n"); > > + debug_descriptor.log_buf_phys = 0; > > + > > + return OPAL_SUCCESS; > > +} > > + > > +void bmc_dump_region_init(void) > > +{ > > + opal_register(OPAL_REGISTER_DUMP_REGION, > > + bmc_opal_register_dump_region, 3); > > + opal_register(OPAL_UNREGISTER_DUMP_REGION, > > + bmc_opal_unregister_dump_region, 1); > > +}; > > diff --git a/core/init.c b/core/init.c > > index 3b887a24d11c..c1c352858e06 100644 > > --- a/core/init.c > > +++ b/core/init.c > > @@ -563,6 +563,10 @@ void __noreturn load_and_boot_kernel(bool is_reboot) > > > > mem_dump_free(); > > > > + /* Zero out memory location before the next kernel starts, in case the > > + * previous kernel did not unregister */ > > + debug_descriptor.log_buf_phys = 0; > > + > > /* Take processours out of nap */ > > cpu_set_sreset_enable(false); > > cpu_set_ipi_enable(false); > > diff --git a/include/dump-region.h b/include/dump-region.h > > new file mode 100644 > > index 000000000000..b629e66a85e6 > > --- /dev/null > > +++ b/include/dump-region.h > > @@ -0,0 +1,26 @@ > > +/* Copyright 2018 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. > > + */ > > + > > +#ifndef __DUMP_H > > +#define __DUMP_H > > + > > +/* > > + * Initialise dump region registration OPAL calls for bmc systems. > > + */ > > +void bmc_dump_region_init(void); > > + > > +#endif > > + > > diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c > > index 30c2714f94b9..ca8ec7e8c0ce 100644 > > --- a/platforms/astbmc/common.c > > +++ b/platforms/astbmc/common.c > > @@ -26,6 +26,7 @@ > > #include <bt.h> > > #include <errorlog.h> > > #include <lpc.h> > > +#include <dump-region.h> > > > > #include "astbmc.h" > > > > @@ -156,6 +157,9 @@ void astbmc_init(void) > > > > /* Add BMC firmware info to device tree */ > > ipmi_dt_add_bmc_info(); > > + > > + /* Enable dump region OPAL calls */ > > + bmc_dump_region_init(); > > } > > > > int64_t astbmc_ipmi_power_down(uint64_t request)
On Tue, May 7, 2019 at 10:46 AM Joel Stanley <joel@jms.id.au> wrote: > > Digging this back up again as it would have been handy for some > debugging yesterday. > > On Mon, 28 May 2018 at 13:19, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > > > On Mon, 2018-05-28 at 13:25 +0930, Joel Stanley wrote: > > > This provides an implementation of the OPAL_REGISTER_DUMP_REGION and > > > OPAL_UNREGISTER_DUMP_REGION calls that can be used by non-FSP systems, > > > and performs that registration for all astbmc platforms. > > > > > > This pointer will be used by eg. pdbg to fetch the host kernel's log > > > buffer over FSI. > > > > > > We unconditionally clear the value on a reboot (both fast-reboot and > > > reliable reboot) to reduce the chance of debug tools getting the wrong > > > buffer. > > > > I wouldn't make it BMC specific. > > > > Instead, I would suggest you: > > > > - Add "generic" OPAL calls that populate the debug descritor (ie, > > statically defined using OPAL_CALL). > > > > - Make the above call an optional machine.register_dump_region > > > > - Make FSP machines populate the above to *also* register with the > > FSP. > > > > That way we can also use/create similar tools (or just getmemproc) to > > get to the kernel dmesg via the debug descriptor on FSP systems. > > Oliver, Vasant: are you happy with the patch as-is? Or shall we go > down the route that benh suggested? Do what Ben suggested. Having a static opal call for REGISTER_DUMP makes most of the header file ceremony in this patch redundant and it's easier to follow what's going on if we don't runtime patch OPAL call handlers. > The advantage of the patch as-is is existing kernels that boot on new > skiboot will register the debug descriptor. This will be true for either approach. > > Cheers, > > Joel > > > > > > Cheers, > > Ben. > > > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > > --- > > > v2: > > > Rename dump.h to dump-region.h > > > Add comment to init.c > > > --- > > > core/Makefile.inc | 2 +- > > > core/dump-region.c | 61 +++++++++++++++++++++++++++++++++++++++ > > > core/init.c | 4 +++ > > > include/dump-region.h | 26 +++++++++++++++++ > > > platforms/astbmc/common.c | 4 +++ > > > 5 files changed, 96 insertions(+), 1 deletion(-) > > > create mode 100644 core/dump-region.c > > > create mode 100644 include/dump-region.h > > > > > > diff --git a/core/Makefile.inc b/core/Makefile.inc > > > index d36350590edb..2d2f74778f4c 100644 > > > --- a/core/Makefile.inc > > > +++ b/core/Makefile.inc > > > @@ -9,7 +9,7 @@ CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o > > > 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 += pci-dt-slot.o direct-controls.o cpufeatures.o dump-region.o > > > > > > ifeq ($(SKIBOOT_GCOV),1) > > > CORE_OBJS += gcov-profiling.o > > > diff --git a/core/dump-region.c b/core/dump-region.c > > > new file mode 100644 > > > index 000000000000..e42486f99ebf > > > --- /dev/null > > > +++ b/core/dump-region.c > > > @@ -0,0 +1,61 @@ > > > +/* Copyright 2018 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 <opal.h> > > > +#include <skiboot.h> > > > + > > > +#include <dump-region.h> > > > + > > > +static int64_t bmc_opal_register_dump_region(uint32_t id, > > > + uint64_t addr, uint64_t size) > > > +{ > > > + if (id != OPAL_DUMP_REGION_LOG_BUF) { > > > + prerror("Unsupported log region id %02x\n", id); > > > + return OPAL_UNSUPPORTED; > > > + } > > > + > > > + if (size <= 0) { > > > + prerror("Invalid log size %lld\n", size); > > > + return OPAL_PARAMETER; > > > + } > > > + > > > + prlog(PR_INFO, "Registered log buf at 0x%016llx\n", addr); > > > + debug_descriptor.log_buf_phys = addr; > > > + > > > + return OPAL_SUCCESS; > > > +} > > > + > > > +static int64_t bmc_opal_unregister_dump_region(uint32_t id) > > > +{ > > > + if (id != OPAL_DUMP_REGION_LOG_BUF) { > > > + prlog(PR_DEBUG, "Unsupported log region id %02x\n", id); > > > + return OPAL_UNSUPPORTED; > > > + } > > > + prlog(PR_INFO, "Unregistered log buf\n"); > > > + debug_descriptor.log_buf_phys = 0; > > > + > > > + return OPAL_SUCCESS; > > > +} > > > + > > > +void bmc_dump_region_init(void) > > > +{ > > > + opal_register(OPAL_REGISTER_DUMP_REGION, > > > + bmc_opal_register_dump_region, 3); > > > + opal_register(OPAL_UNREGISTER_DUMP_REGION, > > > + bmc_opal_unregister_dump_region, 1); > > > +}; > > > diff --git a/core/init.c b/core/init.c > > > index 3b887a24d11c..c1c352858e06 100644 > > > --- a/core/init.c > > > +++ b/core/init.c > > > @@ -563,6 +563,10 @@ void __noreturn load_and_boot_kernel(bool is_reboot) > > > > > > mem_dump_free(); > > > > > > + /* Zero out memory location before the next kernel starts, in case the > > > + * previous kernel did not unregister */ > > > + debug_descriptor.log_buf_phys = 0; > > > + > > > /* Take processours out of nap */ > > > cpu_set_sreset_enable(false); > > > cpu_set_ipi_enable(false); > > > diff --git a/include/dump-region.h b/include/dump-region.h > > > new file mode 100644 > > > index 000000000000..b629e66a85e6 > > > --- /dev/null > > > +++ b/include/dump-region.h > > > @@ -0,0 +1,26 @@ > > > +/* Copyright 2018 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. > > > + */ > > > + > > > +#ifndef __DUMP_H > > > +#define __DUMP_H > > > + > > > +/* > > > + * Initialise dump region registration OPAL calls for bmc systems. > > > + */ > > > +void bmc_dump_region_init(void); > > > + > > > +#endif > > > + > > > diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c > > > index 30c2714f94b9..ca8ec7e8c0ce 100644 > > > --- a/platforms/astbmc/common.c > > > +++ b/platforms/astbmc/common.c > > > @@ -26,6 +26,7 @@ > > > #include <bt.h> > > > #include <errorlog.h> > > > #include <lpc.h> > > > +#include <dump-region.h> > > > > > > #include "astbmc.h" > > > > > > @@ -156,6 +157,9 @@ void astbmc_init(void) > > > > > > /* Add BMC firmware info to device tree */ > > > ipmi_dt_add_bmc_info(); > > > + > > > + /* Enable dump region OPAL calls */ > > > + bmc_dump_region_init(); > > > } > > > > > > int64_t astbmc_ipmi_power_down(uint64_t request)
On 05/07/2019 06:16 AM, Joel Stanley wrote: > Digging this back up again as it would have been handy for some > debugging yesterday. > > On Mon, 28 May 2018 at 13:19, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: >> >> On Mon, 2018-05-28 at 13:25 +0930, Joel Stanley wrote: >>> This provides an implementation of the OPAL_REGISTER_DUMP_REGION and >>> OPAL_UNREGISTER_DUMP_REGION calls that can be used by non-FSP systems, >>> and performs that registration for all astbmc platforms. >>> >>> This pointer will be used by eg. pdbg to fetch the host kernel's log >>> buffer over FSI. >>> >>> We unconditionally clear the value on a reboot (both fast-reboot and >>> reliable reboot) to reduce the chance of debug tools getting the wrong >>> buffer. >> >> I wouldn't make it BMC specific. >> >> Instead, I would suggest you: >> >> - Add "generic" OPAL calls that populate the debug descritor (ie, >> statically defined using OPAL_CALL). >> >> - Make the above call an optional machine.register_dump_region >> >> - Make FSP machines populate the above to *also* register with the >> FSP. >> >> That way we can also use/create similar tools (or just getmemproc) to >> get to the kernel dmesg via the debug descriptor on FSP systems. > > Oliver, Vasant: are you happy with the patch as-is? Or shall we go > down the route that benh suggested? Benh suggestion is better. > > The advantage of the patch as-is is existing kernels that boot on new > skiboot will register the debug descriptor. Even with new approach we will not have kernel changes right. -Vasant
diff --git a/core/Makefile.inc b/core/Makefile.inc index d36350590edb..2d2f74778f4c 100644 --- a/core/Makefile.inc +++ b/core/Makefile.inc @@ -9,7 +9,7 @@ CORE_OBJS += vpd.o hostservices.o platform.o nvram.o nvram-format.o hmi.o 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 += pci-dt-slot.o direct-controls.o cpufeatures.o dump-region.o ifeq ($(SKIBOOT_GCOV),1) CORE_OBJS += gcov-profiling.o diff --git a/core/dump-region.c b/core/dump-region.c new file mode 100644 index 000000000000..e42486f99ebf --- /dev/null +++ b/core/dump-region.c @@ -0,0 +1,61 @@ +/* Copyright 2018 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 <opal.h> +#include <skiboot.h> + +#include <dump-region.h> + +static int64_t bmc_opal_register_dump_region(uint32_t id, + uint64_t addr, uint64_t size) +{ + if (id != OPAL_DUMP_REGION_LOG_BUF) { + prerror("Unsupported log region id %02x\n", id); + return OPAL_UNSUPPORTED; + } + + if (size <= 0) { + prerror("Invalid log size %lld\n", size); + return OPAL_PARAMETER; + } + + prlog(PR_INFO, "Registered log buf at 0x%016llx\n", addr); + debug_descriptor.log_buf_phys = addr; + + return OPAL_SUCCESS; +} + +static int64_t bmc_opal_unregister_dump_region(uint32_t id) +{ + if (id != OPAL_DUMP_REGION_LOG_BUF) { + prlog(PR_DEBUG, "Unsupported log region id %02x\n", id); + return OPAL_UNSUPPORTED; + } + prlog(PR_INFO, "Unregistered log buf\n"); + debug_descriptor.log_buf_phys = 0; + + return OPAL_SUCCESS; +} + +void bmc_dump_region_init(void) +{ + opal_register(OPAL_REGISTER_DUMP_REGION, + bmc_opal_register_dump_region, 3); + opal_register(OPAL_UNREGISTER_DUMP_REGION, + bmc_opal_unregister_dump_region, 1); +}; diff --git a/core/init.c b/core/init.c index 3b887a24d11c..c1c352858e06 100644 --- a/core/init.c +++ b/core/init.c @@ -563,6 +563,10 @@ void __noreturn load_and_boot_kernel(bool is_reboot) mem_dump_free(); + /* Zero out memory location before the next kernel starts, in case the + * previous kernel did not unregister */ + debug_descriptor.log_buf_phys = 0; + /* Take processours out of nap */ cpu_set_sreset_enable(false); cpu_set_ipi_enable(false); diff --git a/include/dump-region.h b/include/dump-region.h new file mode 100644 index 000000000000..b629e66a85e6 --- /dev/null +++ b/include/dump-region.h @@ -0,0 +1,26 @@ +/* Copyright 2018 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. + */ + +#ifndef __DUMP_H +#define __DUMP_H + +/* + * Initialise dump region registration OPAL calls for bmc systems. + */ +void bmc_dump_region_init(void); + +#endif + diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c index 30c2714f94b9..ca8ec7e8c0ce 100644 --- a/platforms/astbmc/common.c +++ b/platforms/astbmc/common.c @@ -26,6 +26,7 @@ #include <bt.h> #include <errorlog.h> #include <lpc.h> +#include <dump-region.h> #include "astbmc.h" @@ -156,6 +157,9 @@ void astbmc_init(void) /* Add BMC firmware info to device tree */ ipmi_dt_add_bmc_info(); + + /* Enable dump region OPAL calls */ + bmc_dump_region_init(); } int64_t astbmc_ipmi_power_down(uint64_t request)
This provides an implementation of the OPAL_REGISTER_DUMP_REGION and OPAL_UNREGISTER_DUMP_REGION calls that can be used by non-FSP systems, and performs that registration for all astbmc platforms. This pointer will be used by eg. pdbg to fetch the host kernel's log buffer over FSI. We unconditionally clear the value on a reboot (both fast-reboot and reliable reboot) to reduce the chance of debug tools getting the wrong buffer. Signed-off-by: Joel Stanley <joel@jms.id.au> --- v2: Rename dump.h to dump-region.h Add comment to init.c --- core/Makefile.inc | 2 +- core/dump-region.c | 61 +++++++++++++++++++++++++++++++++++++++ core/init.c | 4 +++ include/dump-region.h | 26 +++++++++++++++++ platforms/astbmc/common.c | 4 +++ 5 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 core/dump-region.c create mode 100644 include/dump-region.h