| Message ID | 20210920173203.1800475-1-nathanl@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/pseries: delete scanlog | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
Nathan Lynch <nathanl@linux.ibm.com> writes: > Remove the pseries scanlog driver. > > This code supports functions from Power4-era servers that are not present > on targets currently supported by arch/powerpc. System manuals from this > time have this description: > > Scan Dump data is a set of chip data that the service processor gathers > after a system malfunction. It consists of chip scan rings, chip trace > arrays, and Scan COM (SCOM) registers. This data is stored in the > scan-log partition of the system’s Nonvolatile Random Access > Memory (NVRAM). > > PowerVM partition firmware development doesn't recognize the associated > function call or property, and they don't see any references to them in > their codebase. It seems to have been specific to non-virtualized > pseries. Just bumping this to see if there are any objections.
Nathan Lynch <nathanl@linux.ibm.com> writes: > Nathan Lynch <nathanl@linux.ibm.com> writes: >> Remove the pseries scanlog driver. >> >> This code supports functions from Power4-era servers that are not present >> on targets currently supported by arch/powerpc. System manuals from this >> time have this description: >> >> Scan Dump data is a set of chip data that the service processor gathers >> after a system malfunction. It consists of chip scan rings, chip trace >> arrays, and Scan COM (SCOM) registers. This data is stored in the >> scan-log partition of the system’s Nonvolatile Random Access >> Memory (NVRAM). >> >> PowerVM partition firmware development doesn't recognize the associated >> function call or property, and they don't see any references to them in >> their codebase. It seems to have been specific to non-virtualized >> pseries. > > Just bumping this to see if there are any objections. Not an objection, I like nothing better than dropping old unused cruft, but are we sure it's safe to remove the proc file? I see that rtas_errd still looks for it, have you checked that it will handle the absence of the file gracefully and continue doing whatever else it does? On further inspection it looks like the code that looks for it in rtas_errd is #if 0'ed out (??), so maybe it's dead. Anyway if you can test that rtas_errd still works that'd be good. Presumably there's no other code that cares about the proc file. cheers
Michael Ellerman <mpe@ellerman.id.au> writes: > Nathan Lynch <nathanl@linux.ibm.com> writes: >> Nathan Lynch <nathanl@linux.ibm.com> writes: >>> Remove the pseries scanlog driver. >>> >>> This code supports functions from Power4-era servers that are not present >>> on targets currently supported by arch/powerpc. System manuals from this >>> time have this description: >>> >>> Scan Dump data is a set of chip data that the service processor gathers >>> after a system malfunction. It consists of chip scan rings, chip trace >>> arrays, and Scan COM (SCOM) registers. This data is stored in the >>> scan-log partition of the system’s Nonvolatile Random Access >>> Memory (NVRAM). >>> >>> PowerVM partition firmware development doesn't recognize the associated >>> function call or property, and they don't see any references to them in >>> their codebase. It seems to have been specific to non-virtualized >>> pseries. >> >> Just bumping this to see if there are any objections. > > Not an objection, I like nothing better than dropping old unused cruft, > but are we sure it's safe to remove the proc file? > > I see that rtas_errd still looks for it, have you checked that it will > handle the absence of the file gracefully and continue doing whatever > else it does? Uhh. I will stop forgetting to check ppc64_diag when making such changes. Thanks for pointing this out. > On further inspection it looks like the code that looks for it in > rtas_errd is #if 0'ed out (??), so maybe it's dead. Yes it seems so. From rtas_errd's main(): #if 0 /* * Check to see if a new scanlog dump is available; if so, copy it to * the filesystem and associate the dump with the first error processed. */ check_scanlog_dump(); #endif And that's the only entry point into the code that collects the scanlog data. And that dead code appears to deal with the absence of /proc/ppc64/scan-log-dump gracefully. It has been like that since initial git import in 2013. > Anyway if you can test that rtas_errd still works that'd be good. I've verified that it starts normally and logs EPOW events associated with partition migration. > Presumably there's no other code that cares about the proc file. AFAIK this is right. powerpc-utils and librtas do not use it. librtas has a wrapper for the calling the associated RTAS function directly, but that's fine. I tried using GitHub's search to find instances of "scan-log-dump" that weren't from Linux or ppc64_diag (need to be logged in I think): https://github.com/search?q=%22scan-log-dump%22+-path%3Aarch%2Fpowerpc+-filename%3Ascanlog.c+-extension%3Apatch&type=Code&ref=advsearch&l=&l= This hasn't yielded any unexpected users. There may be better search terms but that's what a few minutes of fiddling with it got me.
Nathan Lynch <nathanl@linux.ibm.com> writes: > Michael Ellerman <mpe@ellerman.id.au> writes: >> Nathan Lynch <nathanl@linux.ibm.com> writes: >>> Nathan Lynch <nathanl@linux.ibm.com> writes: >>>> Remove the pseries scanlog driver. >>>> >>>> This code supports functions from Power4-era servers that are not present >>>> on targets currently supported by arch/powerpc. System manuals from this >>>> time have this description: >>>> >>>> Scan Dump data is a set of chip data that the service processor gathers >>>> after a system malfunction. It consists of chip scan rings, chip trace >>>> arrays, and Scan COM (SCOM) registers. This data is stored in the >>>> scan-log partition of the system’s Nonvolatile Random Access >>>> Memory (NVRAM). >>>> >>>> PowerVM partition firmware development doesn't recognize the associated >>>> function call or property, and they don't see any references to them in >>>> their codebase. It seems to have been specific to non-virtualized >>>> pseries. >>> >>> Just bumping this to see if there are any objections. >> >> Not an objection, I like nothing better than dropping old unused cruft, >> but are we sure it's safe to remove the proc file? >> >> I see that rtas_errd still looks for it, have you checked that it will >> handle the absence of the file gracefully and continue doing whatever >> else it does? > > Uhh. I will stop forgetting to check ppc64_diag when making such > changes. Thanks for pointing this out. No worries. >> On further inspection it looks like the code that looks for it in >> rtas_errd is #if 0'ed out (??), so maybe it's dead. > > Yes it seems so. From rtas_errd's main(): > > #if 0 > /* > * Check to see if a new scanlog dump is available; if so, copy it to > * the filesystem and associate the dump with the first error processed. > */ > check_scanlog_dump(); > #endif > > And that's the only entry point into the code that collects the scanlog > data. And that dead code appears to deal with the absence of > /proc/ppc64/scan-log-dump gracefully. It has been like that since > initial git import in 2013. OK, I guess it came from sourceforge before that. But I'm not going to start digging there, that's long enough ago that it shouldn't matter. >> Anyway if you can test that rtas_errd still works that'd be good. > > I've verified that it starts normally and logs EPOW events associated > with partition migration. Awesome. >> Presumably there's no other code that cares about the proc file. > > AFAIK this is right. powerpc-utils and librtas do not use it. librtas > has a wrapper for the calling the associated RTAS function directly, but > that's fine. > > I tried using GitHub's search to find instances of "scan-log-dump" that > weren't from Linux or ppc64_diag (need to be logged in I think): > > https://github.com/search?q=%22scan-log-dump%22+-path%3Aarch%2Fpowerpc+-filename%3Ascanlog.c+-extension%3Apatch&type=Code&ref=advsearch&l=&l= > > This hasn't yielded any unexpected users. There may be better search > terms but that's what a few minutes of fiddling with it got me. I had a look on sourcegraph too, same story, nothing interesting: https://sourcegraph.com/search?q=context:global+scan-log-dump+NOT+file:arch/powerpc/platforms/pseries/scanlog.c+NOT+file:arch/powerpc/kernel/rtas.c+NOT+file:arch/ppc64/kernel/scanlog.c+fork:yes+archived:yes&patternType=literal So that seems OK to me, I'll pick this up. cheers
On Mon, 20 Sep 2021 12:32:03 -0500, Nathan Lynch wrote: > Remove the pseries scanlog driver. > > This code supports functions from Power4-era servers that are not present > on targets currently supported by arch/powerpc. System manuals from this > time have this description: > > Scan Dump data is a set of chip data that the service processor gathers > after a system malfunction. It consists of chip scan rings, chip trace > arrays, and Scan COM (SCOM) registers. This data is stored in the > scan-log partition of the system’s Nonvolatile Random Access > Memory (NVRAM). > > [...] Applied to powerpc/next. [1/1] powerpc/pseries: delete scanlog https://git.kernel.org/powerpc/c/22887f319a39929e357810a1f964fcba7ae42c59 cheers
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig index 0ad2291337a7..846815007fef 100644 --- a/arch/powerpc/configs/ppc64_defconfig +++ b/arch/powerpc/configs/ppc64_defconfig @@ -26,7 +26,6 @@ CONFIG_PPC64=y CONFIG_NR_CPUS=2048 CONFIG_PPC_SPLPAR=y CONFIG_DTL=y -CONFIG_SCANLOG=m CONFIG_PPC_SMLPAR=y CONFIG_IBMEBUS=y CONFIG_PPC_SVM=y diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig index b183629f1bcf..42a72e6d5b35 100644 --- a/arch/powerpc/configs/pseries_defconfig +++ b/arch/powerpc/configs/pseries_defconfig @@ -38,7 +38,6 @@ CONFIG_MODULE_SRCVERSION_ALL=y CONFIG_PARTITION_ADVANCED=y CONFIG_PPC_SPLPAR=y CONFIG_DTL=y -CONFIG_SCANLOG=m CONFIG_PPC_SMLPAR=y CONFIG_IBMEBUS=y CONFIG_PAPR_SCM=m diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig index 5e037df2a3a1..bf9b612a929b 100644 --- a/arch/powerpc/platforms/pseries/Kconfig +++ b/arch/powerpc/platforms/pseries/Kconfig @@ -61,10 +61,6 @@ config PSERIES_ENERGY Provides: /sys/devices/system/cpu/pseries_(de)activation_hint_list and /sys/devices/system/cpu/cpuN/pseries_(de)activation_hint -config SCANLOG - tristate "Scanlog dump interface" - depends on RTAS_PROC && PPC_PSERIES - config IO_EVENT_IRQ bool "IO Event Interrupt support" depends on PPC_PSERIES diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 4cda0ef87be0..2f9ae0c113e3 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -8,7 +8,6 @@ obj-y := lpar.o hvCall.o nvram.o reconfig.o \ firmware.o power.o dlpar.o mobility.o rng.o \ pci.o pci_dlpar.o eeh_pseries.o msi.o obj-$(CONFIG_SMP) += smp.o -obj-$(CONFIG_SCANLOG) += scanlog.o obj-$(CONFIG_KEXEC_CORE) += kexec.o obj-$(CONFIG_PSERIES_ENERGY) += pseries_energy.o diff --git a/arch/powerpc/platforms/pseries/scanlog.c b/arch/powerpc/platforms/pseries/scanlog.c deleted file mode 100644 index 2879c4f0ceb7..000000000000 --- a/arch/powerpc/platforms/pseries/scanlog.c +++ /dev/null @@ -1,195 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -/* - * c 2001 PPC 64 Team, IBM Corp - * - * scan-log-data driver for PPC64 Todd Inglett <tinglett@vnet.ibm.com> - * - * When ppc64 hardware fails the service processor dumps internal state - * of the system. After a reboot the operating system can access a dump - * of this data using this driver. A dump exists if the device-tree - * /chosen/ibm,scan-log-data property exists. - * - * This driver exports /proc/powerpc/scan-log-dump which can be read. - * The driver supports only sequential reads. - * - * The driver looks at a write to the driver for the single word "reset". - * If given, the driver will reset the scanlog so the platform can free it. - */ - -#include <linux/module.h> -#include <linux/types.h> -#include <linux/errno.h> -#include <linux/proc_fs.h> -#include <linux/init.h> -#include <linux/delay.h> -#include <linux/slab.h> -#include <linux/uaccess.h> -#include <asm/rtas.h> -#include <asm/prom.h> - -#define MODULE_VERS "1.0" -#define MODULE_NAME "scanlog" - -/* Status returns from ibm,scan-log-dump */ -#define SCANLOG_COMPLETE 0 -#define SCANLOG_HWERROR -1 -#define SCANLOG_CONTINUE 1 - - -static unsigned int ibm_scan_log_dump; /* RTAS token */ -static unsigned int *scanlog_buffer; /* The data buffer */ - -static ssize_t scanlog_read(struct file *file, char __user *buf, - size_t count, loff_t *ppos) -{ - unsigned int *data = scanlog_buffer; - int status; - unsigned long len, off; - unsigned int wait_time; - - if (count > RTAS_DATA_BUF_SIZE) - count = RTAS_DATA_BUF_SIZE; - - if (count < 1024) { - /* This is the min supported by this RTAS call. Rather - * than do all the buffering we insist the user code handle - * larger reads. As long as cp works... :) - */ - printk(KERN_ERR "scanlog: cannot perform a small read (%ld)\n", count); - return -EINVAL; - } - - if (!access_ok(buf, count)) - return -EFAULT; - - for (;;) { - wait_time = 500; /* default wait if no data */ - spin_lock(&rtas_data_buf_lock); - memcpy(rtas_data_buf, data, RTAS_DATA_BUF_SIZE); - status = rtas_call(ibm_scan_log_dump, 2, 1, NULL, - (u32) __pa(rtas_data_buf), (u32) count); - memcpy(data, rtas_data_buf, RTAS_DATA_BUF_SIZE); - spin_unlock(&rtas_data_buf_lock); - - pr_debug("scanlog: status=%d, data[0]=%x, data[1]=%x, " \ - "data[2]=%x\n", status, data[0], data[1], data[2]); - switch (status) { - case SCANLOG_COMPLETE: - pr_debug("scanlog: hit eof\n"); - return 0; - case SCANLOG_HWERROR: - pr_debug("scanlog: hardware error reading data\n"); - return -EIO; - case SCANLOG_CONTINUE: - /* We may or may not have data yet */ - len = data[1]; - off = data[2]; - if (len > 0) { - if (copy_to_user(buf, ((char *)data)+off, len)) - return -EFAULT; - return len; - } - /* Break to sleep default time */ - break; - default: - /* Assume extended busy */ - wait_time = rtas_busy_delay_time(status); - if (!wait_time) { - printk(KERN_ERR "scanlog: unknown error " \ - "from rtas: %d\n", status); - return -EIO; - } - } - /* Apparently no data yet. Wait and try again. */ - msleep_interruptible(wait_time); - } - /*NOTREACHED*/ -} - -static ssize_t scanlog_write(struct file * file, const char __user * buf, - size_t count, loff_t *ppos) -{ - char stkbuf[20]; - int status; - - if (count > 19) count = 19; - if (copy_from_user (stkbuf, buf, count)) { - return -EFAULT; - } - stkbuf[count] = 0; - - if (buf) { - if (strncmp(stkbuf, "reset", 5) == 0) { - pr_debug("scanlog: reset scanlog\n"); - status = rtas_call(ibm_scan_log_dump, 2, 1, NULL, 0, 0); - pr_debug("scanlog: rtas returns %d\n", status); - } - } - return count; -} - -static int scanlog_open(struct inode * inode, struct file * file) -{ - unsigned int *data = scanlog_buffer; - - if (data[0] != 0) { - /* This imperfect test stops a second copy of the - * data (or a reset while data is being copied) - */ - return -EBUSY; - } - - data[0] = 0; /* re-init so we restart the scan */ - - return 0; -} - -static int scanlog_release(struct inode * inode, struct file * file) -{ - unsigned int *data = scanlog_buffer; - - data[0] = 0; - return 0; -} - -static const struct proc_ops scanlog_proc_ops = { - .proc_read = scanlog_read, - .proc_write = scanlog_write, - .proc_open = scanlog_open, - .proc_release = scanlog_release, - .proc_lseek = noop_llseek, -}; - -static int __init scanlog_init(void) -{ - struct proc_dir_entry *ent; - int err = -ENOMEM; - - ibm_scan_log_dump = rtas_token("ibm,scan-log-dump"); - if (ibm_scan_log_dump == RTAS_UNKNOWN_SERVICE) - return -ENODEV; - - /* Ideally we could allocate a buffer < 4G */ - scanlog_buffer = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL); - if (!scanlog_buffer) - goto err; - - ent = proc_create("powerpc/rtas/scan-log-dump", 0400, NULL, - &scanlog_proc_ops); - if (!ent) - goto err; - return 0; -err: - kfree(scanlog_buffer); - return err; -} - -static void __exit scanlog_cleanup(void) -{ - remove_proc_entry("powerpc/rtas/scan-log-dump", NULL); - kfree(scanlog_buffer); -} - -module_init(scanlog_init); -module_exit(scanlog_cleanup); -MODULE_LICENSE("GPL");