Re: [PATCH] macintosh: macio_asic: fix resource_size.cocci warnings
Yihao Han writes: > drivers/macintosh/macio_asic.c:219:26-29: WARNING: > Suspicious code. resource_size is maybe missing with res > drivers/macintosh/macio_asic.c:221:26-29: WARNING: > Suspicious code. resource_size is maybe missing with res > > Use resource_size function on resource object instead of > explicit computation. > > Generated by: scripts/coccinelle/api/resource_size.cocci > > Signed-off-by: Yihao Han > --- > drivers/macintosh/macio_asic.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c > index 1943a007e2d5..260fccb3863e 100644 > --- a/drivers/macintosh/macio_asic.c > +++ b/drivers/macintosh/macio_asic.c > @@ -216,9 +216,9 @@ static int macio_resource_quirks(struct device_node *np, > struct resource *res, > /* Some older IDE resources have bogus sizes */ > if (of_node_name_eq(np, "IDE") || of_node_name_eq(np, "ATA") || > of_node_is_type(np, "ide") || of_node_is_type(np, "ata")) { > - if (index == 0 && (res->end - res->start) > 0xfff) > + if (index == 0 && (resource_size(res)) > 0xfff) > res->end = res->start + 0xfff; > - if (index == 1 && (res->end - res->start) > 0xff) > + if (index == 1 && (resource_size(res)) > 0xff) Are you sure the conversion is correct? It's not exactly equivalent: static inline resource_size_t resource_size(const struct resource *res) { return res->end - res->start + 1; } cheers
Re: Apply d799769188529abc6cbf035a10087a51f7832b6b to 5.17 and 5.15?
Nathan Chancellor writes: > On Thu, Apr 21, 2022 at 05:46:52PM +1000, Michael Ellerman wrote: >> Nathan Chancellor writes: >> > Hi Greg, Sasha, and Michael, >> > >> > Commit d79976918852 ("powerpc/64: Add UADDR64 relocation support") fixes >> > a boot failure with CONFIG_RELOCATABLE=y kernels linked with recent >> > versions of ld.lld [1]. Additionally, it resolves a separate boot >> > failure that Paul Menzel reported [2] with ld.lld 13.0.0. Is this a >> > reasonable backport for 5.17 and 5.15? It applies cleanly, resolves both >> > problems, and does not appear to cause any other issues in my testing >> > for both trees but I was curious what Michael's opinion was, as I am far >> > from a PowerPC expert. >> > >> > This change does apply cleanly to 5.10 (I did not try earlier branches) >> > but there are other changes needed for ld.lld to link CONFIG_RELOCATABLE >> > kernels in that branch so to avoid any regressions, I think it is safe >> > to just focus on 5.15 and 5.17. >> >> I considered tagging it for stable, but I wanted it to get a bit of >> testing first, it's a reasonably big patch. >> >> I think we're reasonably confident it doesn't introduce any new bugs, >> but more testing time is always good. >> >> So I guess I'd be inclined to wait another week or so before requesting >> a stable backport? > > Sure, thanks for the response! I'll ping this thread on Monday, May 2nd, > so that we have two more RC releases to try and flush out any lingering > issues. If you do receive any reports of regressions, please let me > know. Sounds good, thanks. cheers
Re: [RFC v4 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
Eric DeVolder writes: > On 4/21/22 06:34, Michael Ellerman wrote: >> Sourabh Jain writes: >>> The option CRASH_HOTPLUG enables, in kernel update to kexec segments on >>> hotplug events. >>> >>> All the updates needed on the capture kernel load path in the kernel for >>> both kexec_load and kexec_file_load system will be kept under this config. >>> >>> Signed-off-by: Sourabh Jain >>> Reviewed-by: Eric DeVolder >>> --- >>> arch/powerpc/Kconfig | 11 +++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>> index b779603978e1..777db33f75b5 100644 >>> --- a/arch/powerpc/Kconfig >>> +++ b/arch/powerpc/Kconfig >>> @@ -623,6 +623,17 @@ config FA_DUMP >>> If unsure, say "y". Only special kernels like petitboot may >>> need to say "N" here. >>> >>> +config CRASH_HOTPLUG >>> + bool "kernel updates of crash kexec segments" >>> + depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE >>> + help >>> + An efficient way to keep the capture kernel up-to-date with CPU >>> + hotplug events. On CPU hotplug event the kexec segments of capture >>> + kernel becomes stale and need to be updated with latest CPU data. >>> + In this method the kernel performs minimal update to only relevant >>> + kexec segments on CPU hotplug event, instead of triggering full >>> + capture kernel reload from userspace using udev rule. >> >> Why would a user ever want to turn this off? >> >> Seems to me we should just make it always behave this way, and not have >> a CONFIG option at all. > > Sourabh, > > Borislav Petkov also requested I remove the config option, which will be the > case in upcoming v8. > > Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the > CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG. If you're having to spell "CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG" in many places then you can still add CONFIG_CRASH_HOTPLUG to represent the sum of all the dependencies, just don't make it user-selectable. cheers
[powerpc:next-test] BUILD SUCCESS 950cf957fe34d40d63dfa3bf3968210430b6491e
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test branch HEAD: 950cf957fe34d40d63dfa3bf3968210430b6491e misc: ocxl: fix possible double free in ocxl_file_register_afu elapsed time: 809m configs tested: 123 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm64 defconfig arm64allyesconfig arm allmodconfig arm defconfig arm allyesconfig i386 randconfig-c001 powerpc asp8347_defconfig sh se7712_defconfig powerpc ep8248e_defconfig sh sh7724_generic_defconfig powerpc ppc64_defconfig sh rsk7269_defconfig mips loongson1b_defconfig armmini2440_defconfig sparc sparc32_defconfig armkeystone_defconfig powerpc storcenter_defconfig powerpc64 defconfig parisc alldefconfig mips cobalt_defconfig mipsbcm47xx_defconfig sh polaris_defconfig armspear6xx_defconfig mipsar7_defconfig powerpc pq2fads_defconfig sparc64 defconfig arm iop32x_defconfig arm assabet_defconfig arm stm32_defconfig xtensasmp_lx200_defconfig mips xway_defconfig xtensa iss_defconfig microblaze defconfig xtensa cadence_csp_defconfig powerpc arches_defconfig x86_64randconfig-c001 arm randconfig-c002-20220421 ia64 allmodconfig ia64 allyesconfig ia64defconfig m68k allyesconfig m68k allmodconfig m68kdefconfig cskydefconfig nios2allyesconfig alpha defconfig alphaallyesconfig h8300allyesconfig xtensa allyesconfig arc defconfig sh allmodconfig nios2 defconfig arc allyesconfig s390defconfig s390 allmodconfig parisc defconfig parisc64defconfig parisc allyesconfig s390 allyesconfig sparc defconfig i386 allyesconfig sparcallyesconfig i386defconfig i386 debian-10.3-kselftests i386 debian-10.3 mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allnoconfig powerpc allmodconfig x86_64randconfig-a004 x86_64randconfig-a002 x86_64randconfig-a006 i386 randconfig-a001 i386 randconfig-a003 i386 randconfig-a005 x86_64randconfig-a013 x86_64randconfig-a011 x86_64randconfig-a015 i386 randconfig-a012 i386 randconfig-a014 i386 randconfig-a016 arc randconfig-r043-20220421 s390 randconfig-r044-20220421 riscvrandconfig-r042-20220421 riscv defconfig riscvnommu_virt_defconfig riscv rv32_defconfig riscvnommu_k210_defconfig riscv allnoconfig riscvallmodconfig riscvallyesconfig x86_64rhel-8.3-kselftests um x86_64_defconfig um i386_defconfig x86_64 rhel-8.3-func x86_64 kexec x86_64 defconfig x86_64 allyesconfig x86_64 rhel-8.3-kunit x86_64
[powerpc:fixes-test] BUILD SUCCESS bb82c574691daf8f7fa9a160264d15c5804cb769
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: bb82c574691daf8f7fa9a160264d15c5804cb769 powerpc/perf: Fix 32bit compile elapsed time: 810m configs tested: 109 configs skipped: 116 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm64 defconfig arm64allyesconfig arm allmodconfig arm defconfig arm allyesconfig i386 randconfig-c001 powerpc asp8347_defconfig sh se7712_defconfig powerpc ep8248e_defconfig sh sh7724_generic_defconfig powerpc ppc64_defconfig sh rsk7269_defconfig mips loongson1b_defconfig armmini2440_defconfig sparc sparc32_defconfig armkeystone_defconfig powerpc storcenter_defconfig powerpc64 defconfig parisc alldefconfig mips cobalt_defconfig mipsbcm47xx_defconfig sh polaris_defconfig armspear6xx_defconfig mipsar7_defconfig powerpc pq2fads_defconfig arm stm32_defconfig xtensasmp_lx200_defconfig mips xway_defconfig xtensa iss_defconfig microblaze defconfig xtensa cadence_csp_defconfig powerpc arches_defconfig x86_64randconfig-c001 arm randconfig-c002-20220421 ia64 allmodconfig ia64 allyesconfig ia64defconfig m68k allyesconfig m68k allmodconfig m68kdefconfig cskydefconfig nios2allyesconfig alpha defconfig alphaallyesconfig h8300allyesconfig xtensa allyesconfig arc defconfig sh allmodconfig nios2 defconfig arc allyesconfig s390defconfig s390 allmodconfig parisc defconfig parisc64defconfig parisc allyesconfig s390 allyesconfig sparc defconfig i386 allyesconfig sparcallyesconfig i386defconfig i386 debian-10.3-kselftests i386 debian-10.3 mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allnoconfig powerpc allmodconfig x86_64randconfig-a006 x86_64randconfig-a004 x86_64randconfig-a002 arc randconfig-r043-20220421 s390 randconfig-r044-20220421 riscvrandconfig-r042-20220421 riscv defconfig riscvnommu_virt_defconfig riscv rv32_defconfig riscvnommu_k210_defconfig riscv allnoconfig riscvallmodconfig riscvallyesconfig x86_64rhel-8.3-kselftests um x86_64_defconfig um i386_defconfig x86_64 rhel-8.3-func x86_64 kexec x86_64 defconfig x86_64 allyesconfig x86_64 rhel-8.3-kunit x86_64 rhel-8.3 clang tested configs: riscvrandconfig-c006-20220421 mips randconfig-c004-20220421 x86_64randconfig-c007 i386 randconfig-c001 arm randconfig-c002-20220421 powerpc randconfig-c003-20220421 powerpc ppc64e_defconfig mips rs90_defconfig powerpcfsp2_defconfig powerpc tqm8560_defconfig i386 randconfig-a002 i386
Re: [RFC v4 PATCH 4/5] powerpc/crash hp: add crash hotplug support for kexec_file_load
On 4/19/22 03:31, Sourabh Jain wrote: On 14/04/22 22:02, Laurent Dufour wrote: On 11/04/2022, 10:43:56, Sourabh Jain wrote: Two major changes are done to enable the crash CPU hotplug handler. Firstly, updated the kexec load path to prepare kimage for hotplug changes, and secondly, implemented the crash hotplug handler. On the kexec load path, the memsz allocation of the crash FDT segment is updated to ensure that it has sufficient buffer space to accommodate future hot add CPUs. Initialized the kimage members to track the kexec FDT segment. The crash hotplug handler updates the cpus node of crash FDT. While we update crash FDT the kexec_crash_image is marked invalid and restored after FDT update to avoid race. Since memory crash hotplug support is not there yet the crash hotplug the handler simply warns the user and returns. Signed-off-by: Sourabh Jain --- arch/powerpc/kexec/core_64.c | 46 ++ arch/powerpc/kexec/elf_64.c | 74 2 files changed, 120 insertions(+) diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c index 249d2632526d..62f77cc86407 100644 --- a/arch/powerpc/kexec/core_64.c +++ b/arch/powerpc/kexec/core_64.c @@ -466,6 +466,52 @@ int update_cpus_node(void *fdt) return ret; } +#ifdef CONFIG_CRASH_HOTPLUG +/** + * arch_crash_hotplug_handler() - Handle hotplug FDT changes + * @image: the active struct kimage + * @hp_action: the hot un/plug action being handled + * @a: first parameter dependent upon hp_action + * @b: first parameter dependent upon hp_action + * + * To accurately reflect CPU hot un/plug changes, the FDT + * must be updated with the new list of CPUs and memories. + */ +void arch_crash_hotplug_handler(struct kimage *image, unsigned int hp_action, + unsigned long a, unsigned long b) +{ + void *fdt; + + /* No action needed for CPU hot-unplug */ + if (hp_action == KEXEC_CRASH_HP_REMOVE_CPU) + return; I should have report since in the previous version too, why nothing is done when CPU are removed? Since CPU note addresses are already available for all possible CPUs (regardless they are online or not) the PHDR is allocated for all possible CPUs during elfcorehdr creation. At least for the kexec_load system call. Now on the crash path, the crashing CPU initiates an IPI call to update the CPU data of all online CPUs and jumps to the purgatory. Hence no action is needed for the remove case. With the above logic, we shouldn't be taking any action for the CPU add case too, right? But on PowerPC early boot path there is validation that checks the boot CPU is part of the Flattened Device Tree (FDT) or not. If the boot CPU is not found in FDT the boot fails. Hence FDT needs to be updated for every new CPU added to the system but not needed when CPU is removed. Thanks Sourabh Jain + + /* crash update on memory hotplug is not support yet */ + if (hp_action == KEXEC_CRASH_HP_REMOVE_MEMORY || hp_action == KEXEC_CRASH_HP_ADD_MEMORY) { + pr_info_once("crash hp: crash update is not supported with memory hotplug\n"); + return; + } Sourabh, Baoquan's preference is to do away with the hp_action (and a and b) parameters to this function. As x86_64 has no need for them, I have no reason to argue for keeping them. If you really need hp_action, then please respond to Baoquan's concern in the "[PATCH v7 4/8] crash: add generic infrastructure for crash hotplug support" thread. Thanks, eric + + /* Must have valid FDT index */ + if (!image->arch.fdt_index_valid) { + pr_err("crash hp: unable to locate FDT segment"); + return; + } + + fdt = __va((void *)image->segment[image->arch.fdt_index].mem); + + /* Temporarily invalidate the crash image while it is replaced */ + xchg(&kexec_crash_image, NULL); + + /* update FDT to refelect changes to CPU resrouces */ + if (update_cpus_node(fdt)) + pr_err("crash hp: failed to update crash FDT"); + + /* The crash image is now valid once again */ + xchg(&kexec_crash_image, image); +} +#endif /* CONFIG_CRASH_HOTPLUG */ + #ifdef CONFIG_PPC_64S_HASH_MMU /* Values we need to export to the second kernel via the device tree. */ static unsigned long htab_base; diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c index eeb258002d1e..9dc774548ce4 100644 --- a/arch/powerpc/kexec/elf_64.c +++ b/arch/powerpc/kexec/elf_64.c @@ -24,6 +24,67 @@ #include #include +#include +#include + +#ifdef CONFIG_CRASH_HOTPLUG + +/** + * get_cpu_node_sz() - Calculate the space needed to store a CPU device type node + * in FDT. The calculation is done based on the existing CPU + * node in unflatten device tree. Loop through all the + * properties of the very first CPU type device node found in + * unflatten device tree and returns the sum of the property + *
Re: [RFC v4 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
On 4/21/22 06:34, Michael Ellerman wrote: Sourabh Jain writes: The option CRASH_HOTPLUG enables, in kernel update to kexec segments on hotplug events. All the updates needed on the capture kernel load path in the kernel for both kexec_load and kexec_file_load system will be kept under this config. Signed-off-by: Sourabh Jain Reviewed-by: Eric DeVolder --- arch/powerpc/Kconfig | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index b779603978e1..777db33f75b5 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -623,6 +623,17 @@ config FA_DUMP If unsure, say "y". Only special kernels like petitboot may need to say "N" here. +config CRASH_HOTPLUG + bool "kernel updates of crash kexec segments" + depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE + help + An efficient way to keep the capture kernel up-to-date with CPU + hotplug events. On CPU hotplug event the kexec segments of capture + kernel becomes stale and need to be updated with latest CPU data. + In this method the kernel performs minimal update to only relevant + kexec segments on CPU hotplug event, instead of triggering full + capture kernel reload from userspace using udev rule. Why would a user ever want to turn this off? Seems to me we should just make it always behave this way, and not have a CONFIG option at all. Sourabh, Borislav Petkov also requested I remove the config option, which will be the case in upcoming v8. Where I was using CONFIG_CRASH_HOTPLUG, I've replaced it with the CONFIG_HOTPLUG_CPU || CONFIG_MEMORY_HOTPLUG. Eric cheers
Re: Apply d799769188529abc6cbf035a10087a51f7832b6b to 5.17 and 5.15?
On Thu, Apr 21, 2022 at 05:46:52PM +1000, Michael Ellerman wrote: > Nathan Chancellor writes: > > Hi Greg, Sasha, and Michael, > > > > Commit d79976918852 ("powerpc/64: Add UADDR64 relocation support") fixes > > a boot failure with CONFIG_RELOCATABLE=y kernels linked with recent > > versions of ld.lld [1]. Additionally, it resolves a separate boot > > failure that Paul Menzel reported [2] with ld.lld 13.0.0. Is this a > > reasonable backport for 5.17 and 5.15? It applies cleanly, resolves both > > problems, and does not appear to cause any other issues in my testing > > for both trees but I was curious what Michael's opinion was, as I am far > > from a PowerPC expert. > > > > This change does apply cleanly to 5.10 (I did not try earlier branches) > > but there are other changes needed for ld.lld to link CONFIG_RELOCATABLE > > kernels in that branch so to avoid any regressions, I think it is safe > > to just focus on 5.15 and 5.17. > > I considered tagging it for stable, but I wanted it to get a bit of > testing first, it's a reasonably big patch. > > I think we're reasonably confident it doesn't introduce any new bugs, > but more testing time is always good. > > So I guess I'd be inclined to wait another week or so before requesting > a stable backport? Sure, thanks for the response! I'll ping this thread on Monday, May 2nd, so that we have two more RC releases to try and flush out any lingering issues. If you do receive any reports of regressions, please let me know. Cheers, Nathan
[PATCH v2] macintosh: macio_asic: fix resource_size.cocci warnings
drivers/macintosh/macio_asic.c:219:26-29: WARNING:Suspicious code. resource_size is maybe missing with res drivers/macintosh/macio_asic.c:221:26-29: WARNING:Suspicious code. resource_size is maybe missing with res Use resource_size function on resource object instead of explicit computation. Generated by: scripts/coccinelle/api/resource_size.cocci Signed-off-by: Yihao Han --- v2: drop parenthesis around resource_size(res) and edit commit message --- drivers/macintosh/macio_asic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c index 1943a007e2d5..260fccb3863e 100644 --- a/drivers/macintosh/macio_asic.c +++ b/drivers/macintosh/macio_asic.c @@ -216,9 +216,9 @@ static int macio_resource_quirks(struct device_node *np, struct resource *res, /* Some older IDE resources have bogus sizes */ if (of_node_name_eq(np, "IDE") || of_node_name_eq(np, "ATA") || of_node_is_type(np, "ide") || of_node_is_type(np, "ata")) { - if (index == 0 && (res->end - res->start) > 0xfff) + if (index == 0 && resource_size(res) > 0xfff) res->end = res->start + 0xfff; - if (index == 1 && (res->end - res->start) > 0xff) + if (index == 1 && resource_size(res) > 0xff) res->end = res->start + 0xff; } return 0; -- 2.17.1
Re: [PATCH v2] powerpc/rtas: Keep MSR[RI] set when calling RTAS
On 13/04/2022, 07:58:42, Nicholas Piggin wrote: > Excerpts from Laurent Dufour's message of April 2, 2022 12:06 am: >> RTAS runs in real mode (MSR[DR] and MSR[IR] unset) and in 32bits >> mode (MSR[SF] unset). >> >> The change in MSR is done in enter_rtas() in a relatively complex way, >> since the MSR value could be hardcoded. >> >> Furthermore, a panic has been reported when hitting the watchdog interrupt >> while running in RTAS, this leads to the following stack trace: >> >> [69244.027433][ C24] watchdog: CPU 24 Hard LOCKUP >> [69244.027442][ C24] watchdog: CPU 24 TB:997512652051031, last heartbeat >> TB:997504470175378 (15980ms ago) >> [69244.027451][ C24] Modules linked in: chacha_generic(E) libchacha(E) >> xxhash_generic(E) wp512(E) sha3_generic(E) rmd160(E) poly1305_generic(E) >> libpoly1305(E) michael_mic(E) md4(E) crc32_generic(E) cmac(E) ccm(E) >> algif_rng(E) twofish_generic(E) twofish_common(E) serpent_generic(E) >> fcrypt(E) des_generic(E) libdes(E) cast6_generic(E) cast5_generic(E) >> cast_common(E) camellia_generic(E) blowfish_generic(E) blowfish_common(E) >> algif_skcipher(E) algif_hash(E) gcm(E) algif_aead(E) af_alg(E) tun(E) >> rpcsec_gss_krb5(E) auth_rpcgss(E) >> nfsv4(E) dns_resolver(E) rpadlpar_io(EX) rpaphp(EX) xsk_diag(E) tcp_diag(E) >> udp_diag(E) raw_diag(E) inet_diag(E) unix_diag(E) af_packet_diag(E) >> netlink_diag(E) nfsv3(E) nfs_acl(E) nfs(E) lockd(E) grace(E) sunrpc(E) >> fscache(E) netfs(E) af_packet(E) rfkill(E) bonding(E) tls(E) ibmveth(EX) >> crct10dif_vpmsum(E) rtc_generic(E) drm(E) drm_panel_orientation_quirks(E) >> fuse(E) configfs(E) backlight(E) ip_tables(E) x_tables(E) dm_service_time(E) >> sd_mod(E) t10_pi(E) >> [69244.027555][ C24] ibmvfc(EX) scsi_transport_fc(E) vmx_crypto(E) >> gf128mul(E) btrfs(E) blake2b_generic(E) libcrc32c(E) crc32c_vpmsum(E) xor(E) >> raid6_pq(E) dm_mirror(E) dm_region_hash(E) dm_log(E) sg(E) dm_multipath(E) >> dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) scsi_mod(E) >> [69244.027587][ C24] Supported: No, Unreleased kernel >> [69244.027600][ C24] CPU: 24 PID: 87504 Comm: drmgr Kdump: loaded Tainted: >> GE X5.14.21-150400.71.1.bz196362_2-default #1 SLE15-SP4 >> (unreleased) 0d821077ef4faa8dfaf370efb5fdca1fa35f4e2c >> [69244.027609][ C24] NIP: 1fb41050 LR: 1fb4104c CTR: >> >> [69244.027612][ C24] REGS: cfc33d60 TRAP: 0100 Tainted: G >> E X (5.14.21-150400.71.1.bz196362_2-default) >> [69244.027615][ C24] MSR: 82981000 CR: 4882 >> XER: 20040020 >> [69244.027625][ C24] CFAR: 011c IRQMASK: 1 >> [69244.027625][ C24] GPR00: 0003 >> 0001 50dc >> [69244.027625][ C24] GPR04: 1ffb6100 0020 >> 0001 1fb09010 >> [69244.027625][ C24] GPR08: 2000 >> >> [69244.027625][ C24] GPR12: 8004072a40a8 cff8b680 >> 0007 0034 >> [69244.027625][ C24] GPR16: 1fbf6e94 1fbf6d84 >> 1fbd1db0 1fb3f008 >> [69244.027625][ C24] GPR20: 1fb41018 >> 017f f68f >> [69244.027625][ C24] GPR24: 1fb18fe8 1fb3e000 >> 1fb1adc0 1fb1cf40 >> [69244.027625][ C24] GPR28: 1fb26000 1fb460f0 >> 1fb17f18 1fb17000 >> [69244.027663][ C24] NIP [1fb41050] 0x1fb41050 >> [69244.027696][ C24] LR [1fb4104c] 0x1fb4104c >> [69244.027699][ C24] Call Trace: >> [69244.027701][ C24] Instruction dump: >> [69244.027723][ C24] >> >> [69244.027728][ C24] >> >> [69244.027762][T87504] Oops: Unrecoverable System Reset, sig: 6 [#1] >> [69244.028044][T87504] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA >> pSeries >> [69244.028089][T87504] Modules linked in: chacha_generic(E) libchacha(E) >> xxhash_generic(E) wp512(E) sha3_generic(E) rmd160(E) poly1305_generic(E) >> libpoly1305(E) michael_mic(E) md4(E) crc32_generic(E) cmac(E) ccm(E) >> algif_rng(E) twofish_generic(E) twofish_common(E) serpent_generic(E) >> fcrypt(E) des_generic(E) libdes(E) cast6_generic(E) cast5_generic(E) >> cast_common(E) camellia_generic(E) blowfish_generic(E) blowfish_common(E) >> algif_skcipher(E) algif_hash(E) gcm(E) algif_aead(E) af_alg(E) tun(E) >> rpcsec_gss_krb5(E) auth_rpcgss(E) >> nfsv4(E) dns_resolver(E) rpadlpar_io(EX) rpaphp(EX) xsk_diag(E) tcp_diag(E) >> udp_diag(E) raw_diag(E) inet_diag(E) unix_diag(E) af_packet_diag(E) >> netlink_diag(E) nfsv3(E) nfs_acl(E) nfs(E) lockd(E) grace(E) sunrpc(E) >> fscache(E) netfs(E) af_packet(E) rfkill(E) bonding(E) tls(E) ibmveth(EX) >> crct10dif_vpmsum(E) rtc_generic(E) drm(E) drm_panel_orientation_quirks(E
Re: [RFC v4 PATCH 2/5] powerpc/crash hp: introduce a new config option CRASH_HOTPLUG
Sourabh Jain writes: > The option CRASH_HOTPLUG enables, in kernel update to kexec segments on > hotplug events. > > All the updates needed on the capture kernel load path in the kernel for > both kexec_load and kexec_file_load system will be kept under this config. > > Signed-off-by: Sourabh Jain > Reviewed-by: Eric DeVolder > --- > arch/powerpc/Kconfig | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index b779603978e1..777db33f75b5 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -623,6 +623,17 @@ config FA_DUMP > If unsure, say "y". Only special kernels like petitboot may > need to say "N" here. > > +config CRASH_HOTPLUG > + bool "kernel updates of crash kexec segments" > + depends on CRASH_DUMP && (HOTPLUG_CPU) && KEXEC_FILE > + help > + An efficient way to keep the capture kernel up-to-date with CPU > + hotplug events. On CPU hotplug event the kexec segments of capture > + kernel becomes stale and need to be updated with latest CPU data. > + In this method the kernel performs minimal update to only relevant > + kexec segments on CPU hotplug event, instead of triggering full > + capture kernel reload from userspace using udev rule. Why would a user ever want to turn this off? Seems to me we should just make it always behave this way, and not have a CONFIG option at all. cheers
Re: [PATCH RFC 2/8] arm64: stacktrace: Add arch_within_stack_frames
On 4/19/22 22:40, Mark Rutland wrote: > Hi, > > On Mon, Apr 18, 2022 at 09:22:11PM +0800, He Zhe wrote: >> This function checks if the given address range crosses frame boundary. > I don't think that's quite true, becuase arm64's procedure call standard > (AAPCS64) doesn't give us enough information to determine this without > additional metadata from the compiler, which we simply don't have today. > > Since there's a lot of confusion in this area, I've made a bit of an info dump > below, before review on the patch itself, but TBH I'm struggling to see that > this is all that useful. Thanks for the exhaustive explanation and info dump here. I've read through all your comments, very helpful. > > On arm64, we use a calling convention called AAPCS64, (in full: "Procedure > Call > Standard for the Arm® 64-bit Architecture (AArch64)"). That's maintained at: > > https://github.com/ARM-software/abi-aa > > ... with the latest release (as of today) at: > > > https://github.com/ARM-software/abi-aa/blob/60a8eb8c55e999d74dac5e368fc9d7e36e38dda4/aapcs64/aapcs64.rst > https://github.com/ARM-software/abi-aa/releases/download/2022Q1/aapcs64.pdf > > In AAPCS64, there are two related but distinct things to be aware of: > > * The "stack frame" of a function, which is the entire contiguous region of > stack memory used by a function. > > * The "frame record", which is the saved FP and LR placed *somewhere* within > the function's stack frame. The FP points at the most recent frame record on > the stack, and at function call boundaries points at the caller's frame > record. > > AAPCS64 doesn't say *where* a frame record is placed within a stack frame, and > there are reasons for compilers to place above and below it. So in genral, a > functionss stack frame looks like: > > +=+ > | above | > |-| > | FP | LR | > |-| > | below | > +=+ > > ... where the "above" or "below" portions might be any size (even 0 bytes). > > Typical code generation today means for most functions that the "below" > portion > is 0 bytes in size, but this is not guaranteed, and even today there are cases > where this is not true. > > When one function calls another without a stack transition, that looks like: > > +=+ ___ > | above |\ > |-|| > ,->| FP | LR |+-- Caller's stack frame > | |-|| > | | below | ___/ > | +=+ ___ > | | above |\ > | |-|| > '--| FP | LR |+-- Callee's stack frame > |-|| > | below | ___/ > +=+ > > Where there's a stack transition, and the new stack is at a *lower* VA than > the > old stack, that looks like: > > +=+ ___ > | above |\ > |-|| > ,->| FP | LR |+-- Caller's stack frame > | |-|| > | | below | ___/ > | +=+ > | > | ~~~ > | Arbitrarily > | large gap, > | potentially > | including > | other data > | ~~~ > | > | +=+ ___ > | | above |\ > | |-|| > '--| FP | LR |+-- Callee's stack frame > |-|| > | below | ___/ > +=+ > > Where there's a stack transition, and the new stack is at a *higher* VA than > the old stack, that looks like: > > +=+ ___ > | above |\ > |-|| > ,--| FP | LR |+-- Callee's stack frame > | |-|| > | | below | ___/ > | +=+ > | > | ~~~ > | Arbitrarily > | large gap, > | potentially > | including > | other data > | ~~~ > | > | +=+ ___ > | | above |\ > | |-|| > '->| FP | LR |+-- Caller's stack frame > |-|| > | below | ___/ > +=+ > > In all of these cases, we *cannot* identify the boundary between the two stack > frames, we can *only* identify where something overlaps a frame record. That > might itself be a good thing, but it's not the same thing as what you describe > in the commit message. > >> It is based on the existing x86 algorithm, but implemented via stacktrace. >> This can be tested by USERCOPY_STACK_FRAME_FROM and >> USERCOPY_STACK_FRAME_TO in lkdtm. > Can you please explain *why* we'd want this? We are trying to use the hardened usercopy feature on arm64 hardware and found that the lkdtm can help validate the feature. But USERCOPY_STACK_FRAME_FROM/TO checks, which were originally added for x86, are not supported for arm64. I thought it would be good if we can enhance such hardening for arm64 and tried to add the basic frame check like on x86 in this series. And yes, with all the arm6
Re: [PATCH] misc: ocxl: fix possible double free in ocxl_file_register_afu
On 21/04/2022 00:54, Michael Ellerman wrote: Hangyu Hua writes: info_release() will be called in device_unregister() when info->dev's reference count is 0. So there is no need to call ocxl_afu_put() and kfree() again. Double frees are often exploitable. But it looks to me like this error path is not easily reachable by an attacker. ocxl_file_register_afu() is only called from ocxl_probe(), and we only go to err_unregister if the sysfs or cdev initialisation fails, which should only happen if we hit ENOMEM, or we have a duplicate device which would be a device-tree/hardware error. But maybe Fred can check more closely, I don't know the driver that well. The linux devices built here are based on what is parsed on the physical devices. Those could be FPGAs but updating the FPGA image requires root privilege. In any case, duplicate AFU names are possible, that's why the driver adds an index (the afu->config.idx part of the name) to the linux device name. So we would need to mess that up in the driver as well to have a duplicate device name. So I would agree the double free is hard to hit. mpe: I think this patch can be taken as is. The "beautification" I talked about is just that and I don't intend to work on it except if something else shows up. Fred cheers Fix this by adding free_minor() and return to err_unregister error path. Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend & frontend") Signed-off-by: Hangyu Hua --- drivers/misc/ocxl/file.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index d881f5e40ad9..6777c419a8da 100644 --- a/drivers/misc/ocxl/file.c +++ b/drivers/misc/ocxl/file.c @@ -556,7 +556,9 @@ int ocxl_file_register_afu(struct ocxl_afu *afu) err_unregister: ocxl_sysfs_unregister_afu(info); // safe to call even if register failed + free_minor(info); device_unregister(&info->dev); + return rc; err_put: ocxl_afu_put(afu); free_minor(info); -- 2.25.1
Re: Apply d799769188529abc6cbf035a10087a51f7832b6b to 5.17 and 5.15?
Nathan Chancellor writes: > Hi Greg, Sasha, and Michael, > > Commit d79976918852 ("powerpc/64: Add UADDR64 relocation support") fixes > a boot failure with CONFIG_RELOCATABLE=y kernels linked with recent > versions of ld.lld [1]. Additionally, it resolves a separate boot > failure that Paul Menzel reported [2] with ld.lld 13.0.0. Is this a > reasonable backport for 5.17 and 5.15? It applies cleanly, resolves both > problems, and does not appear to cause any other issues in my testing > for both trees but I was curious what Michael's opinion was, as I am far > from a PowerPC expert. > > This change does apply cleanly to 5.10 (I did not try earlier branches) > but there are other changes needed for ld.lld to link CONFIG_RELOCATABLE > kernels in that branch so to avoid any regressions, I think it is safe > to just focus on 5.15 and 5.17. I considered tagging it for stable, but I wanted it to get a bit of testing first, it's a reasonably big patch. I think we're reasonably confident it doesn't introduce any new bugs, but more testing time is always good. So I guess I'd be inclined to wait another week or so before requesting a stable backport? cheers
[PATCH] net: unexport csum_and_copy_{from,to}_user
csum_and_copy_from_user and csum_and_copy_to_user are exported by a few architectures, but not actually used in modular code. Drop the exports. Signed-off-by: Christoph Hellwig --- arch/alpha/lib/csum_partial_copy.c | 1 - arch/m68k/lib/checksum.c | 2 -- arch/powerpc/lib/checksum_wrappers.c | 2 -- arch/x86/lib/csum-wrappers_64.c | 2 -- 4 files changed, 7 deletions(-) diff --git a/arch/alpha/lib/csum_partial_copy.c b/arch/alpha/lib/csum_partial_copy.c index 1931a04af85a2..4d180d96f09e4 100644 --- a/arch/alpha/lib/csum_partial_copy.c +++ b/arch/alpha/lib/csum_partial_copy.c @@ -353,7 +353,6 @@ csum_and_copy_from_user(const void __user *src, void *dst, int len) return 0; return __csum_and_copy(src, dst, len); } -EXPORT_SYMBOL(csum_and_copy_from_user); __wsum csum_partial_copy_nocheck(const void *src, void *dst, int len) diff --git a/arch/m68k/lib/checksum.c b/arch/m68k/lib/checksum.c index 7e6afeae62177..5acb821849d30 100644 --- a/arch/m68k/lib/checksum.c +++ b/arch/m68k/lib/checksum.c @@ -265,8 +265,6 @@ csum_and_copy_from_user(const void __user *src, void *dst, int len) return sum; } -EXPORT_SYMBOL(csum_and_copy_from_user); - /* * copy from kernel space while checksumming, otherwise like csum_partial diff --git a/arch/powerpc/lib/checksum_wrappers.c b/arch/powerpc/lib/checksum_wrappers.c index f3999cbb2fcc4..1a14c8780278c 100644 --- a/arch/powerpc/lib/checksum_wrappers.c +++ b/arch/powerpc/lib/checksum_wrappers.c @@ -24,7 +24,6 @@ __wsum csum_and_copy_from_user(const void __user *src, void *dst, user_read_access_end(); return csum; } -EXPORT_SYMBOL(csum_and_copy_from_user); __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len) { @@ -38,4 +37,3 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len) user_write_access_end(); return csum; } -EXPORT_SYMBOL(csum_and_copy_to_user); diff --git a/arch/x86/lib/csum-wrappers_64.c b/arch/x86/lib/csum-wrappers_64.c index 189344924a2be..145f9a0bde29a 100644 --- a/arch/x86/lib/csum-wrappers_64.c +++ b/arch/x86/lib/csum-wrappers_64.c @@ -32,7 +32,6 @@ csum_and_copy_from_user(const void __user *src, void *dst, int len) user_access_end(); return sum; } -EXPORT_SYMBOL(csum_and_copy_from_user); /** * csum_and_copy_to_user - Copy and checksum to user space. @@ -57,7 +56,6 @@ csum_and_copy_to_user(const void *src, void __user *dst, int len) user_access_end(); return sum; } -EXPORT_SYMBOL(csum_and_copy_to_user); /** * csum_partial_copy_nocheck - Copy and checksum. -- 2.30.2