Re: [PATCH v2 02/14] crash: split vmcoreinfo exporting code out from crash_core.c
On 02/21/24 at 11:07pm, Sourabh Jain wrote: > Hello Baoquan, > > On 19/01/24 20:22, Baoquan He wrote: > > Now move the relevant codes into separate files: > > kernel/crash_reserve.c, include/linux/crash_reserve.h. > > > > And add config item CRASH_RESERVE to control its enabling. > > Feels like this patch is more about vmcore_info.[c|h] and CONFIG_VMCORE_INFO > then the above mentioned files and config. You are right. Above lines about crash_reserve should be removed. It's from v1, and done in patch 1 of v2 and v3. I forgot removing them. > > > > > And also update the old ifdeffery of CONFIG_CRASH_CORE, including of > > and config item dependency on CRASH_CORE > > accordingly. > > > > And also do renaming as follows: > > - arch/xxx/kernel/{crash_core.c => vmcore_info.c} > > because they are only related to vmcoreinfo exporting on x86, arm64, > > riscv. > > > > And also Remove config item CRASH_CORE, and rely on CONFIG_KEXEC_CORE to > > decide if build in crash_core.c. > > > > Signed-off-by: Baoquan He > > --- > > arch/arm64/kernel/Makefile| 2 +- > > .../kernel/{crash_core.c => vmcore_info.c}| 2 +- > > arch/powerpc/Kconfig | 2 +- > > arch/powerpc/kernel/setup-common.c| 2 +- > > arch/powerpc/platforms/powernv/opal-core.c| 2 +- > > arch/riscv/kernel/Makefile| 2 +- > > .../kernel/{crash_core.c => vmcore_info.c}| 2 +- > > arch/x86/kernel/Makefile | 2 +- > > .../{crash_core_32.c => vmcore_info_32.c} | 2 +- > > .../{crash_core_64.c => vmcore_info_64.c} | 2 +- > > drivers/firmware/qemu_fw_cfg.c| 14 +- > > fs/proc/Kconfig | 2 +- > > fs/proc/kcore.c | 2 +- > > include/linux/buildid.h | 2 +- > > include/linux/crash_core.h| 73 -- > > include/linux/kexec.h | 1 + > > include/linux/vmcore_info.h | 81 ++ > > kernel/Kconfig.kexec | 4 +- > > kernel/Makefile | 4 +- > > kernel/crash_core.c | 208 > > kernel/ksysfs.c | 6 +- > > kernel/printk/printk.c| 4 +- > > kernel/vmcore_info.c | 233 ++ > > lib/buildid.c | 2 +- > > 24 files changed, 345 insertions(+), 311 deletions(-) > > rename arch/arm64/kernel/{crash_core.c => vmcore_info.c} (97%) > > rename arch/riscv/kernel/{crash_core.c => vmcore_info.c} (96%) > > rename arch/x86/kernel/{crash_core_32.c => vmcore_info_32.c} (90%) > > rename arch/x86/kernel/{crash_core_64.c => vmcore_info_64.c} (94%) > > create mode 100644 include/linux/vmcore_info.h > > create mode 100644 kernel/vmcore_info.c > > > > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > > index d95b3d6b471a..bcf89587a549 100644 > > --- a/arch/arm64/kernel/Makefile > > +++ b/arch/arm64/kernel/Makefile > > @@ -66,7 +66,7 @@ obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o > > kexec_image.o > > obj-$(CONFIG_ARM64_RELOC_TEST)+= arm64-reloc-test.o > > arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o > > obj-$(CONFIG_CRASH_DUMP) += crash_dump.o > > -obj-$(CONFIG_CRASH_CORE) += crash_core.o > > +obj-$(CONFIG_VMCORE_INFO) += vmcore_info.o > > obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o > > obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o > > obj-$(CONFIG_ARM64_MTE) += mte.o > > diff --git a/arch/arm64/kernel/crash_core.c > > b/arch/arm64/kernel/vmcore_info.c > > similarity index 97% > > rename from arch/arm64/kernel/crash_core.c > > rename to arch/arm64/kernel/vmcore_info.c > > index 66cde752cd74..a5abf7186922 100644 > > --- a/arch/arm64/kernel/crash_core.c > > +++ b/arch/arm64/kernel/vmcore_info.c > > @@ -4,7 +4,7 @@ > >* Copyright (C) Huawei Futurewei Technologies. > >*/ > > -#include > > +#include > > #include > > #include > > #include > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > > index 6aeab95f0edd..1520146d7c2c 100644 > > --- a/arch/powerpc/Kconfig > > +++ b/arch/powerpc/Kconfig > > @@ -690,7 +690,7 @@ config ARCH_SELECTS_CRASH_DUMP > > config FA_DUMP > > bool "Firmware-assisted dump" > > depends on PPC64 && (PPC_RTAS || PPC_POWERNV) > > - select CRASH_CORE > > + select VMCORE_INFO > > select CRASH_RESERVE > > select CRASH_DUMP > > help > > diff --git a/arch/powerpc/kernel/setup-common.c > > b/arch/powerpc/kernel/setup-common.c > > index 9b142b9d5187..733f210ffda1 100644 > > --- a/arch/powerpc/kernel/setup-common.c > > +++ b/arch/powerpc/kernel/setup-common.c > > @@ -109,7 +109,7 @@ int ppc_do_canonicalize_irqs; > >
Re: [PATCH RFC net] ps3/gelic: Fix possible NULL pointer dereference
Hi Simon, On 2/22/24 01:56, Simon Horman wrote: > Fix possible NULL pointer dereference in gelic_card_release_tx_chain() > > The cited commit introduced a netdev variable to > gelic_card_release_tx_chain() which is set unconditionally on each > iteration of a for loop. > > It is set to the value of tx_chain->tail->skb->dev. However, in some > cases it is assumed that tx_chain->tail->skb may be NULL. And if that > occurs, setting netdev will cause a NULl pointer dereference. > > Given the age of this code I do wonder if this can occur in practice. > But to be on the safe side this patch assumes that it can and aims to > avoid the dereference in the case where tx_chain->tail->skb may be NULL. After 17+ years I never hit this, and never heard of anyone hitting it... > Flagged by Smatch. > Compile tested only. Thanks for 'fixing' this. Acked-by: Geoff Levand
Re: [PATCH v2 00/14] Split crash out from kexec and clean up related config items
On 02/21/24 at 12:57pm, Andrew Morton wrote: > On Wed, 21 Feb 2024 11:15:00 +0530 Hari Bathini > wrote: > > > On 04/02/24 8:56 am, Baoquan He wrote: > > >>> Hope Hari and Pingfan can help have a look, see if > > >>> it's doable. Now, I make it either have both kexec and crash enabled, or > > >>> disable both of them altogether. > > >> > > >> Sure. I will take a closer look... > > > Thanks a lot. Please feel free to post patches to make that, or I can do > > > it with your support or suggestion. > > > > Tested your changes and on top of these changes, came up with the below > > changes to get it working for powerpc: > > > > > > https://lore.kernel.org/all/20240213113150.1148276-1-hbath...@linux.ibm.com/ > > So can we take it that you're OK with Baoquan's series as-is? > > Baoquan, do you believe the patches in mm-unstable are ready for moving > into mm-stable in preparation for an upstream merge? Yeah, I think they are ready to go for merging. For Hari's patchset, the main part was planned before. And I am not familiar with fadump in powerpc, the Kconfig fix from Hari is a good guarantee with the expertise. Surely, I will await Hari's comment on that.
Re: [PATCH net-next] ps3/gelic: minor Kernel Doc corrections
Hi Simon, On 2/22/24 02:46, Simon Horman wrote: > * Update the Kernel Doc for gelic_descr_set_tx_cmdstat() > and gelic_net_setup_netdev() so that documented name > and the actual name of the function match. > > * Move define of GELIC_ALIGN() so that it is no longer > between gelic_alloc_card_net() and it's Kernel Doc. > > * Document netdev parameter of gelic_alloc_card_net() > in a way consistent to the documentation of other netdev parameters > in this file. > > Addresses the following warnings flagged by ./scripts/kernel-doc -none: > > .../ps3_gelic_net.c:711: warning: expecting prototype for > gelic_net_set_txdescr_cmdstat(). Prototype was for > gelic_descr_set_tx_cmdstat() instead > .../ps3_gelic_net.c:1474: warning: expecting prototype for > gelic_ether_setup_netdev(). Prototype was for gelic_net_setup_netdev() instead > .../ps3_gelic_net.c:1528: warning: expecting prototype for > gelic_alloc_card_net(). Prototype was for GELIC_ALIGN() instead > .../ps3_gelic_net.c:1531: warning: Function parameter or struct member > 'netdev' not described in 'gelic_alloc_card_net' > > Signed-off-by: Simon Horman > --- > drivers/net/ethernet/toshiba/ps3_gelic_net.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c > b/drivers/net/ethernet/toshiba/ps3_gelic_net.c > index d5b75af163d3..12b96ca66877 100644 > --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c > +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c > @@ -698,7 +698,7 @@ gelic_card_get_next_tx_descr(struct gelic_card *card) > } > > /** > - * gelic_net_set_txdescr_cmdstat - sets the tx descriptor command field > + * gelic_descr_set_tx_cmdstat - sets the tx descriptor command field > * @descr: descriptor structure to fill out > * @skb: packet to consider > * > @@ -1461,7 +1461,7 @@ static void gelic_ether_setup_netdev_ops(struct > net_device *netdev, > } > > /** > - * gelic_ether_setup_netdev - initialization of net_device > + * gelic_net_setup_netdev - initialization of net_device > * @netdev: net_device structure > * @card: card structure > * > @@ -1518,14 +1518,16 @@ int gelic_net_setup_netdev(struct net_device *netdev, > struct gelic_card *card) > return 0; > } > > +#define GELIC_ALIGN (32) > + > /** > * gelic_alloc_card_net - allocates net_device and card structure > + * @netdev: interface device structure > * > * returns the card structure or NULL in case of errors > * > * the card and net_device structures are linked to each other > */ > -#define GELIC_ALIGN (32) > static struct gelic_card *gelic_alloc_card_net(struct net_device **netdev) > { > struct gelic_card *card; > Looks good. Thanks for taking care of it. Acked-by: Geoff Levand
Re: [PATCH RFC net] ps3/gelic: Fix possible NULL pointer dereference
On 2/22/24 03:32, Dan Carpenter wrote: > This driver is PPC so I have never looked at the code before. I noticed > another issue that was introduced last December in commit 3ce4f9c3fbb3 > ("net/ps3_gelic_net: Add gelic_descr structures"). > > net/ethernet/toshiba/ps3_gelic_net.c ... >375 static int gelic_descr_prepare_rx(struct gelic_card *card, >376struct gelic_descr *descr) >398 descr->skb = NULL; > ^^ > NULL > >399 >400 offset = ((unsigned long)descr->skb->data) & > > Dereferenced here. There is a fix, see '[PATCH v6 net] ps3/gelic: Fix SKB allocation': https://lore.kernel.org/netdev/20240221172824.gd722...@kernel.org/T/ -Geoff
Re: [powerpc] Dump capture failure with recent linux-next
Hi Hari, On Thu, Feb 22, 2024 at 11:30:17AM +0530, Hari Bathini wrote: > Hi Sachin, > > On 22/02/24 10:55 am, Sachin Sant wrote: > > Kdump fails to save vmcore with recent linux-next builds on IBM Power server > > with following messages > > > > Starting Kdump Vmcore Save Service... > > [ 17.349599] kdump[367]: Kdump is using the default log level(3). > > [ 17.407407] kdump[391]: saving to > > /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ > > [ 17.441270] EXT4-fs (sda2): re-mounted > > 630dfb4e-74bd-45c4-a8de-232992bc8724 r/w. Quota mode: none. > > [ 17.04] kdump[395]: saving vmcore-dmesg.txt to > > /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ > > [ 17.464859] kdump[401]: saving vmcore-dmesg.txt complete > > [ 17.466636] kdump[403]: saving vmcore > > [ 17.551589] kdump.sh[404]: > > Checking for memory holes : [ 0.0 %] / > > Checking for memory holes : [100.0 %] | readpage_elf: Attempt to read > > non-existent page at 0xc. > > [ 17.551718] kdump.sh[404]: readmem: type_addr: 0, addr:c00c, > > size:16384 > > [ 17.551793] kdump.sh[404]: __exclude_unnecessary_pages: Can't read the > > buffer of struct page. > > [ 17.551864] kdump.sh[404]: create_2nd_bitmap: Can't exclude unnecessary > > pages. > > [ 17.562632] kdump.sh[404]: The kernel version is not supported. > > [ 17.562708] kdump.sh[404]: The makedumpfile operation may be incomplete. > > [ 17.562773] kdump.sh[404]: makedumpfile Failed. > > [ 17.564335] kdump[406]: saving vmcore failed, _exitcode:1 > > [ 17.566013] kdump[408]: saving the /run/initramfs/kexec-dmesg.log to > > /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ > > [ 17.583658] kdump[414]: saving vmcore failed > > > > Git bisect points to following patch > > > > commit 378eb24a0658dd922b29524e0ce35c6c43f56cba > > mm/vmalloc: remove vmap_area_list > > > > Reverting this patch allows kdump to save vmcore file correctly. > > > > Does this change require any corresponding changes to makedumpfile? > > Right. The change intends the tools to use VMALLOC_START exported via > vmcoreinfo instead of vmap_area_list. I don't see the corresponding > makedumpfile change submitted upstream yet though. > > Aditya, can you help with this.. Sure hari, I will into it. Thanks for the explanation, that will help. Thanks, Aditya Gupta > > - Hari
Re: [powerpc] Dump capture failure with recent linux-next
Hi Sachin, On 22/02/24 10:55 am, Sachin Sant wrote: Kdump fails to save vmcore with recent linux-next builds on IBM Power server with following messages Starting Kdump Vmcore Save Service... [ 17.349599] kdump[367]: Kdump is using the default log level(3). [ 17.407407] kdump[391]: saving to /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ [ 17.441270] EXT4-fs (sda2): re-mounted 630dfb4e-74bd-45c4-a8de-232992bc8724 r/w. Quota mode: none. [ 17.04] kdump[395]: saving vmcore-dmesg.txt to /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ [ 17.464859] kdump[401]: saving vmcore-dmesg.txt complete [ 17.466636] kdump[403]: saving vmcore [ 17.551589] kdump.sh[404]: Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | readpage_elf: Attempt to read non-existent page at 0xc. [ 17.551718] kdump.sh[404]: readmem: type_addr: 0, addr:c00c, size:16384 [ 17.551793] kdump.sh[404]: __exclude_unnecessary_pages: Can't read the buffer of struct page. [ 17.551864] kdump.sh[404]: create_2nd_bitmap: Can't exclude unnecessary pages. [ 17.562632] kdump.sh[404]: The kernel version is not supported. [ 17.562708] kdump.sh[404]: The makedumpfile operation may be incomplete. [ 17.562773] kdump.sh[404]: makedumpfile Failed. [ 17.564335] kdump[406]: saving vmcore failed, _exitcode:1 [ 17.566013] kdump[408]: saving the /run/initramfs/kexec-dmesg.log to /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ [ 17.583658] kdump[414]: saving vmcore failed Git bisect points to following patch commit 378eb24a0658dd922b29524e0ce35c6c43f56cba mm/vmalloc: remove vmap_area_list Reverting this patch allows kdump to save vmcore file correctly. Does this change require any corresponding changes to makedumpfile? Right. The change intends the tools to use VMALLOC_START exported via vmcoreinfo instead of vmap_area_list. I don't see the corresponding makedumpfile change submitted upstream yet though. Aditya, can you help with this.. - Hari
Re: [PATCH 1/2] powerpc: Refactor __kernel_map_pages()
Christophe Leroy writes: > __kernel_map_pages() is almost identical for PPC32 and RADIX. > > Refactor it. > > On PPC32 it is not needed for KFENCE, but to keep it simple > just make it similar to PPC64. > > Signed-off-by: Christophe Leroy > --- > arch/powerpc/include/asm/book3s/64/pgtable.h | 10 -- > arch/powerpc/include/asm/book3s/64/radix.h | 2 -- > arch/powerpc/mm/book3s64/radix_pgtable.c | 14 -- > arch/powerpc/mm/pageattr.c | 19 +++ > arch/powerpc/mm/pgtable_32.c | 15 --- > 5 files changed, 19 insertions(+), 41 deletions(-) > > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c > index 421db7c4f2a4..16b8d20d6ca8 100644 > --- a/arch/powerpc/mm/pageattr.c > +++ b/arch/powerpc/mm/pageattr.c > @@ -101,3 +101,22 @@ int change_memory_attr(unsigned long addr, int numpages, > long action) > return apply_to_existing_page_range(_mm, start, size, > change_page_attr, (void *)action); > } > + > +#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE) > +#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC > +void __kernel_map_pages(struct page *page, int numpages, int enable) > +{ > + unsigned long addr = (unsigned long)page_address(page); > + > + if (PageHighMem(page)) > + return; > + > + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) > + hash__kernel_map_pages(page, numpages, enable); > + else if (enable) > + set_memory_p(addr, numpages); > + else > + set_memory_np(addr, numpages); > +} This doesn't build on 32-bit, eg. ppc32_allmodconfig: ../arch/powerpc/mm/pageattr.c: In function '__kernel_map_pages': ../arch/powerpc/mm/pageattr.c:116:23: error: implicit declaration of function 'hash__kernel_map_pages' [-Werror=implicit-function-declaration] 116 | err = hash__kernel_map_pages(page, numpages, enable); | ^~ I couldn't see a nice way to get around it, so ended up with: void __kernel_map_pages(struct page *page, int numpages, int enable) { int err; unsigned long addr = (unsigned long)page_address(page); if (PageHighMem(page)) return; #ifdef CONFIG_PPC_BOOK3S_64 if (!radix_enabled()) err = hash__kernel_map_pages(page, numpages, enable); else #endif if (enable) err = set_memory_p(addr, numpages); else err = set_memory_np(addr, numpages); cheers
[powerpc] Dump capture failure with recent linux-next
Kdump fails to save vmcore with recent linux-next builds on IBM Power server with following messages Starting Kdump Vmcore Save Service... [ 17.349599] kdump[367]: Kdump is using the default log level(3). [ 17.407407] kdump[391]: saving to /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ [ 17.441270] EXT4-fs (sda2): re-mounted 630dfb4e-74bd-45c4-a8de-232992bc8724 r/w. Quota mode: none. [ 17.04] kdump[395]: saving vmcore-dmesg.txt to /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ [ 17.464859] kdump[401]: saving vmcore-dmesg.txt complete [ 17.466636] kdump[403]: saving vmcore [ 17.551589] kdump.sh[404]: Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | readpage_elf: Attempt to read non-existent page at 0xc. [ 17.551718] kdump.sh[404]: readmem: type_addr: 0, addr:c00c, size:16384 [ 17.551793] kdump.sh[404]: __exclude_unnecessary_pages: Can't read the buffer of struct page. [ 17.551864] kdump.sh[404]: create_2nd_bitmap: Can't exclude unnecessary pages. [ 17.562632] kdump.sh[404]: The kernel version is not supported. [ 17.562708] kdump.sh[404]: The makedumpfile operation may be incomplete. [ 17.562773] kdump.sh[404]: makedumpfile Failed. [ 17.564335] kdump[406]: saving vmcore failed, _exitcode:1 [ 17.566013] kdump[408]: saving the /run/initramfs/kexec-dmesg.log to /sysroot//var/crash//127.0.0.1-2024-02-21-15:03:55/ [ 17.583658] kdump[414]: saving vmcore failed Git bisect points to following patch commit 378eb24a0658dd922b29524e0ce35c6c43f56cba mm/vmalloc: remove vmap_area_list Reverting this patch allows kdump to save vmcore file correctly. Does this change require any corresponding changes to makedumpfile? - Sachin
Re: [PATCH v2 00/14] Split crash out from kexec and clean up related config items
On 22/02/24 2:27 am, Andrew Morton wrote: On Wed, 21 Feb 2024 11:15:00 +0530 Hari Bathini wrote: On 04/02/24 8:56 am, Baoquan He wrote: Hope Hari and Pingfan can help have a look, see if it's doable. Now, I make it either have both kexec and crash enabled, or disable both of them altogether. Sure. I will take a closer look... Thanks a lot. Please feel free to post patches to make that, or I can do it with your support or suggestion. Tested your changes and on top of these changes, came up with the below changes to get it working for powerpc: https://lore.kernel.org/all/20240213113150.1148276-1-hbath...@linux.ibm.com/ So can we take it that you're OK with Baoquan's series as-is? Hi Andrew, If you mean v3 (https://lore.kernel.org/all/20240124051254.67105-1-...@redhat.com/) + follow-up from Baoquan (https://lore.kernel.org/all/Zb8D1ASrgX0qVm9z@MiWiFi-R3L-srv/) Yes. My changes are based on top of the above patches.. Thanks Hari
Re: [PATCH v16 2/5] crash: add a new kexec flag for hotplug support
On 22/02/24 09:28, Baoquan He wrote: On 02/22/24 at 09:01am, Sourabh Jain wrote: Hello Baoquan, There are a lot of code movements introduced by your patch series, 'Split crash out from kexec and clean up related config items.' https://lore.kernel.org/all/20240221125752.36fbfe9c307496313198b...@linux-foundation.org/ Do you want me to rebase this patch series on top of the above patch series? Yes, appreciate that, that would be very helpful. Rebasing this to latest next/master. And I saw Hari's patch sereis, basically it's fine to me. It will be great if this patch can sit on that patchset. Sure, let me rebase it and send v17. Thanks, Sourabh On 17/02/24 13:44, Sourabh Jain wrote: Commit a72bbec70da2 ("crash: hotplug support for kexec_load()") introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses this flag to indicate to the kernel that it is safe to modify the elfcorehdr of the kdump image loaded using the kexec_load system call. However, it is possible that architectures may need to update kexec segments other then elfcorehdr. For example, FDT (Flatten Device Tree) on PowerPC. Introducing a new kexec flag for every new kexec segment may not be a good solution. Hence, a generic kexec flag bit, `KEXEC_CRASH_HOTPLUG_SUPPORT`, is introduced to share the CPU/Memory hotplug support intent between the kexec tool and the kernel for the kexec_load system call. Now, if the kexec tool sends KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag to the kernel, it indicates to the kernel that all the required kexec segment is skipped from SHA calculation and it is safe to update kdump image loaded using the kexec_load syscall. While loading the kdump image using the kexec_load syscall, the @update_elfcorehdr member of struct kimage is set if the kexec tool sends the KEXEC_UPDATE_ELFCOREHDR kexec flag. This member is later used to determine whether it is safe to update elfcorehdr on hotplug events. However, with the introduction of the KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag, the kexec tool could mark all the required kexec segments on an architecture as safe to update. So rename the @update_elfcorehdr to @hotplug_support. If @hotplug_support is set, the kernel can safely update all the required kexec segments of the kdump image during CPU/Memory hotplug events. Introduce an architecture-specific function to process kexec flags for determining hotplug support. Set the @hotplug_support member of struct kimage for both kexec_load and kexec_file_load system calls. This simplifies kernel checks to identify hotplug support for the currently loaded kdump image by just examining the value of @hotplug_support. Signed-off-by: Sourabh Jain Cc: Akhil Raj Cc: Andrew Morton Cc: Aneesh Kumar K.V Cc: Baoquan He Cc: Borislav Petkov (AMD) Cc: Boris Ostrovsky Cc: Christophe Leroy Cc: Dave Hansen Cc: Dave Young Cc: David Hildenbrand Cc: Eric DeVolder Cc: Greg Kroah-Hartman Cc: Hari Bathini Cc: Laurent Dufour Cc: Mahesh Salgaonkar Cc: Michael Ellerman Cc: Mimi Zohar Cc: Naveen N Rao Cc: Oscar Salvador Cc: Thomas Gleixner Cc: Valentin Schneider Cc: Vivek Goyal Cc: ke...@lists.infradead.org Cc: x...@kernel.org --- arch/x86/include/asm/kexec.h | 11 ++- arch/x86/kernel/crash.c | 28 +--- drivers/base/cpu.c | 2 +- drivers/base/memory.c| 2 +- include/linux/kexec.h| 27 +-- include/uapi/linux/kexec.h | 1 + kernel/crash_core.c | 11 --- kernel/kexec.c | 4 ++-- kernel/kexec_file.c | 5 + 9 files changed, 50 insertions(+), 41 deletions(-) diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h index 9bb6607e864e..8be622e82ba8 100644 --- a/arch/x86/include/asm/kexec.h +++ b/arch/x86/include/asm/kexec.h @@ -211,15 +211,8 @@ extern void kdump_nmi_shootdown_cpus(void); void arch_crash_handle_hotplug_event(struct kimage *image, void *arg); #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event -#ifdef CONFIG_HOTPLUG_CPU -int arch_crash_hotplug_cpu_support(void); -#define crash_hotplug_cpu_support arch_crash_hotplug_cpu_support -#endif - -#ifdef CONFIG_MEMORY_HOTPLUG -int arch_crash_hotplug_memory_support(void); -#define crash_hotplug_memory_support arch_crash_hotplug_memory_support -#endif +int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags); +#define arch_crash_hotplug_support arch_crash_hotplug_support unsigned int arch_crash_get_elfcorehdr_size(void); #define crash_get_elfcorehdr_size arch_crash_get_elfcorehdr_size diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index 44744e9c68ec..7072aaee2ea0 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -398,20 +398,26 @@ int crash_load_segments(struct kimage *image) #undef pr_fmt #define pr_fmt(fmt) "crash hp: " fmt -/* These functions provide the value for the sysfs crash_hotplug nodes */
Re: [PATCH v16 2/5] crash: add a new kexec flag for hotplug support
On 02/22/24 at 09:01am, Sourabh Jain wrote: > Hello Baoquan, > > There are a lot of code movements introduced by your patch series, 'Split > crash out from kexec and clean up related config items.' > > https://lore.kernel.org/all/20240221125752.36fbfe9c307496313198b...@linux-foundation.org/ > > Do you want me to rebase this patch series on top of the above patch series? Yes, appreciate that, that would be very helpful. Rebasing this to latest next/master. And I saw Hari's patch sereis, basically it's fine to me. It will be great if this patch can sit on that patchset. Thanks Baoquan > > On 17/02/24 13:44, Sourabh Jain wrote: > > Commit a72bbec70da2 ("crash: hotplug support for kexec_load()") > > introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses > > this flag to indicate to the kernel that it is safe to modify the > > elfcorehdr of the kdump image loaded using the kexec_load system call. > > > > However, it is possible that architectures may need to update kexec > > segments other then elfcorehdr. For example, FDT (Flatten Device Tree) > > on PowerPC. Introducing a new kexec flag for every new kexec segment > > may not be a good solution. Hence, a generic kexec flag bit, > > `KEXEC_CRASH_HOTPLUG_SUPPORT`, is introduced to share the CPU/Memory > > hotplug support intent between the kexec tool and the kernel for the > > kexec_load system call. > > > > Now, if the kexec tool sends KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag to > > the kernel, it indicates to the kernel that all the required kexec > > segment is skipped from SHA calculation and it is safe to update kdump > > image loaded using the kexec_load syscall. > > > > While loading the kdump image using the kexec_load syscall, the > > @update_elfcorehdr member of struct kimage is set if the kexec tool > > sends the KEXEC_UPDATE_ELFCOREHDR kexec flag. This member is later used > > to determine whether it is safe to update elfcorehdr on hotplug events. > > However, with the introduction of the KEXEC_CRASH_HOTPLUG_SUPPORT kexec > > flag, the kexec tool could mark all the required kexec segments on an > > architecture as safe to update. So rename the @update_elfcorehdr to > > @hotplug_support. If @hotplug_support is set, the kernel can safely > > update all the required kexec segments of the kdump image during > > CPU/Memory hotplug events. > > > > Introduce an architecture-specific function to process kexec flags for > > determining hotplug support. Set the @hotplug_support member of struct > > kimage for both kexec_load and kexec_file_load system calls. This > > simplifies kernel checks to identify hotplug support for the currently > > loaded kdump image by just examining the value of @hotplug_support. > > > > Signed-off-by: Sourabh Jain > > Cc: Akhil Raj > > Cc: Andrew Morton > > Cc: Aneesh Kumar K.V > > Cc: Baoquan He > > Cc: Borislav Petkov (AMD) > > Cc: Boris Ostrovsky > > Cc: Christophe Leroy > > Cc: Dave Hansen > > Cc: Dave Young > > Cc: David Hildenbrand > > Cc: Eric DeVolder > > Cc: Greg Kroah-Hartman > > Cc: Hari Bathini > > Cc: Laurent Dufour > > Cc: Mahesh Salgaonkar > > Cc: Michael Ellerman > > Cc: Mimi Zohar > > Cc: Naveen N Rao > > Cc: Oscar Salvador > > Cc: Thomas Gleixner > > Cc: Valentin Schneider > > Cc: Vivek Goyal > > Cc: ke...@lists.infradead.org > > Cc: x...@kernel.org > > --- > > arch/x86/include/asm/kexec.h | 11 ++- > > arch/x86/kernel/crash.c | 28 +--- > > drivers/base/cpu.c | 2 +- > > drivers/base/memory.c| 2 +- > > include/linux/kexec.h| 27 +-- > > include/uapi/linux/kexec.h | 1 + > > kernel/crash_core.c | 11 --- > > kernel/kexec.c | 4 ++-- > > kernel/kexec_file.c | 5 + > > 9 files changed, 50 insertions(+), 41 deletions(-) > > > > diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h > > index 9bb6607e864e..8be622e82ba8 100644 > > --- a/arch/x86/include/asm/kexec.h > > +++ b/arch/x86/include/asm/kexec.h > > @@ -211,15 +211,8 @@ extern void kdump_nmi_shootdown_cpus(void); > > void arch_crash_handle_hotplug_event(struct kimage *image, void *arg); > > #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event > > -#ifdef CONFIG_HOTPLUG_CPU > > -int arch_crash_hotplug_cpu_support(void); > > -#define crash_hotplug_cpu_support arch_crash_hotplug_cpu_support > > -#endif > > - > > -#ifdef CONFIG_MEMORY_HOTPLUG > > -int arch_crash_hotplug_memory_support(void); > > -#define crash_hotplug_memory_support arch_crash_hotplug_memory_support > > -#endif > > +int arch_crash_hotplug_support(struct kimage *image, unsigned long > > kexec_flags); > > +#define arch_crash_hotplug_support arch_crash_hotplug_support > > unsigned int arch_crash_get_elfcorehdr_size(void); > > #define crash_get_elfcorehdr_size arch_crash_get_elfcorehdr_size > > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c >
[powerpc:next] BUILD SUCCESS b22ea627225b53ec7ce25c19d6df9fa8217d1643
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: b22ea627225b53ec7ce25c19d6df9fa8217d1643 powerpc/perf: Power11 Performance Monitoring support elapsed time: 885m configs tested: 161 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alpha allnoconfig gcc alphaallyesconfig gcc alpha defconfig gcc arc allmodconfig gcc arc allnoconfig gcc arc allyesconfig gcc arc defconfig gcc arcnsim_700_defconfig gcc arc randconfig-001-20240222 gcc arc randconfig-002-20240222 gcc arcvdk_hs38_smp_defconfig gcc arm allmodconfig gcc arm allnoconfig clang arm allyesconfig gcc arm aspeed_g5_defconfig gcc arm defconfig clang armneponset_defconfig gcc arm omap2plus_defconfig gcc arm randconfig-001-20240222 gcc arm randconfig-002-20240222 gcc arm s5pv210_defconfig gcc armvexpress_defconfig gcc arm64allmodconfig clang arm64 allnoconfig gcc arm64 defconfig gcc arm64 randconfig-002-20240222 gcc csky allmodconfig gcc csky allnoconfig gcc csky allyesconfig gcc cskydefconfig gcc csky randconfig-001-20240222 gcc csky randconfig-002-20240222 gcc hexagon allmodconfig clang hexagon allnoconfig clang hexagon allyesconfig clang hexagon defconfig clang i386 allmodconfig gcc i386 allnoconfig gcc i386 allyesconfig gcc i386 buildonly-randconfig-001-20240222 gcc i386 buildonly-randconfig-002-20240222 clang i386 buildonly-randconfig-003-20240222 clang i386 buildonly-randconfig-004-20240222 gcc i386 buildonly-randconfig-005-20240222 clang i386 buildonly-randconfig-006-20240222 gcc i386defconfig clang i386 randconfig-001-20240222 clang i386 randconfig-002-20240222 clang i386 randconfig-003-20240222 gcc i386 randconfig-004-20240222 clang i386 randconfig-005-20240222 gcc i386 randconfig-006-20240222 clang i386 randconfig-011-20240222 gcc i386 randconfig-012-20240222 clang i386 randconfig-013-20240222 gcc i386 randconfig-014-20240222 gcc i386 randconfig-015-20240222 clang i386 randconfig-016-20240222 gcc loongarchallmodconfig gcc loongarch allnoconfig gcc loongarchallyesconfig gcc loongarch defconfig gcc loongarch randconfig-001-20240222 gcc loongarch randconfig-002-20240222 gcc m68k allmodconfig gcc m68k allnoconfig gcc m68k allyesconfig gcc m68kdefconfig gcc microblaze allmodconfig gcc microblazeallnoconfig gcc microblaze allyesconfig gcc microblaze defconfig gcc microblaze mmu_defconfig gcc mips allmodconfig gcc mips allnoconfig gcc mips allyesconfig gcc mips malta_kvm_defconfig gcc mipsmaltaup_xpa_defconfig gcc nios2 10m50_defconfig gcc nios2allmodconfig gcc nios2 allnoconfig gcc nios2allyesconfig gcc nios2 defconfig gcc nios2 randconfig-001-20240222 gcc nios2 randconfig-002-20240222 gcc openrisc
Re: [PATCH v12 08/15] media: uapi: Define audio sample format fourcc type
On Wed, Feb 21, 2024 at 7:10 PM Hans Verkuil wrote: > > On 19/02/2024 13:56, Mauro Carvalho Chehab wrote: > > Em Mon, 19 Feb 2024 12:05:02 +0800 > > Shengjiu Wang escreveu: > > > >> Hi Mauro > >> > >> On Sat, Feb 17, 2024 at 5:19 PM Mauro Carvalho Chehab > >> wrote: > >>> > >>> Em Thu, 18 Jan 2024 20:32:01 +0800 > >>> Shengjiu Wang escreveu: > >>> > The audio sample format definition is from alsa, > the header file is include/uapi/sound/asound.h, but > don't include this header file directly, because in > user space, there is another copy in alsa-lib. > There will be conflict in userspace for include > videodev2.h & asound.h and asoundlib.h > > Here still use the fourcc format. > > Signed-off-by: Shengjiu Wang > --- > .../userspace-api/media/v4l/pixfmt-audio.rst | 87 +++ > .../userspace-api/media/v4l/pixfmt.rst| 1 + > drivers/media/v4l2-core/v4l2-ioctl.c | 13 +++ > include/uapi/linux/videodev2.h| 23 + > 4 files changed, 124 insertions(+) > create mode 100644 > Documentation/userspace-api/media/v4l/pixfmt-audio.rst > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-audio.rst > b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst > new file mode 100644 > index ..04b4a7fbd8f4 > --- /dev/null > +++ b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst > @@ -0,0 +1,87 @@ > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > + > +.. _pixfmt-audio: > + > +* > +Audio Formats > +* > + > +These formats are used for :ref:`audiomem2mem` interface only. > + > +.. tabularcolumns:: |p{5.8cm}|p{1.2cm}|p{10.3cm}| > + > +.. cssclass:: longtable > + > +.. flat-table:: Audio Format > +:header-rows: 1 > +:stub-columns: 0 > +:widths: 3 1 4 > + > +* - Identifier > + - Code > + - Details > +* .. _V4L2-AUDIO-FMT-S8: > + > + - ``V4L2_AUDIO_FMT_S8`` > + - 'S8' > + - Corresponds to SNDRV_PCM_FORMAT_S8 in ALSA > +* .. _V4L2-AUDIO-FMT-S16-LE: > >>> > >>> Hmm... why can't we just use SNDRV_*_FORMAT_*? Those are already part of > >>> an uAPI header. No need to add any abstraction here and/or redefine > >>> what is there already at include/uapi/sound/asound.h. > >>> > >> Actually I try to avoid including the include/uapi/sound/asound.h. > >> Because in user space, there is another copy in alsa-lib (asoundlib.h). > >> There will be conflict in userspace when including videodev2.h and > >> asoundlib.h. > > > > Well, alsasoundlib.h seems to be using the same definitions: > > https://github.com/michaelwu/alsa-lib/blob/master/include/pcm.h > > > > So, I can't see what would be the actual issue, as both userspace library > > and ALSA internal headers use the same magic numbers. > > > > You can still do things like: > > > > #ifdef __KERNEL__ > > # include > > #else > > # include > > #endif > > > > To avoid such kind of conflicts, if you need to have it included on > > some header file. Yet, I can't see why you would need that. > > > > IMO, at uAPI headers, you just need to declare the uAPI audiofmt field > > to be either __u32 or __u64, pointing it to where this value comes from > > (on both userspace and Kernelspace. E. g.: > > > > /** > > * struct v4l2_audio_format - audio data format definition > > * @audioformat: > > *an integer number matching the fields inside > > *enum snd_pcm_format_t (e. g. `SNDRV_PCM_FORMAT_*`), as defined > > *in include/uapi/sound/asound.h and > > * > > https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#gaa14b7f26877a812acbb39811364177f8. > > * @channels: channel numbers > > * @buffersize: maximum size in bytes required for data > > */ > > struct v4l2_audio_format { > > __u32 audioformat; > > __u32 channels; > > __u32 buffersize; > > } __attribute__ ((packed)); > > > > Then, at documentation you just need to point to where the > > possible values for SNDRV_PCM_FORMAT_ are defined. No need to > > document them one by one. > > > > With such definition, you'll only need to include sound/asound.h > > within the kAPI scope. > > > >> > >> And in the V4l framework, the fourcc type is commonly used in other > >> cases (video, radio, touch, meta), to avoid changing common code > >> a lot, so I think using fourcc definition for audio may be simpler. > > > > Those are real video streams (or a video-related streams, in the case > > of metadata) where fourcc is widely used. There, it makes sense. > > However, ALSA format definitions are already being used for a long time. > >
Re: [PATCH v16 2/5] crash: add a new kexec flag for hotplug support
Hello Baoquan, There are a lot of code movements introduced by your patch series, 'Split crash out from kexec and clean up related config items.' https://lore.kernel.org/all/20240221125752.36fbfe9c307496313198b...@linux-foundation.org/ Do you want me to rebase this patch series on top of the above patch series? Thanks, Sourabh Jain On 17/02/24 13:44, Sourabh Jain wrote: Commit a72bbec70da2 ("crash: hotplug support for kexec_load()") introduced a new kexec flag, `KEXEC_UPDATE_ELFCOREHDR`. Kexec tool uses this flag to indicate to the kernel that it is safe to modify the elfcorehdr of the kdump image loaded using the kexec_load system call. However, it is possible that architectures may need to update kexec segments other then elfcorehdr. For example, FDT (Flatten Device Tree) on PowerPC. Introducing a new kexec flag for every new kexec segment may not be a good solution. Hence, a generic kexec flag bit, `KEXEC_CRASH_HOTPLUG_SUPPORT`, is introduced to share the CPU/Memory hotplug support intent between the kexec tool and the kernel for the kexec_load system call. Now, if the kexec tool sends KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag to the kernel, it indicates to the kernel that all the required kexec segment is skipped from SHA calculation and it is safe to update kdump image loaded using the kexec_load syscall. While loading the kdump image using the kexec_load syscall, the @update_elfcorehdr member of struct kimage is set if the kexec tool sends the KEXEC_UPDATE_ELFCOREHDR kexec flag. This member is later used to determine whether it is safe to update elfcorehdr on hotplug events. However, with the introduction of the KEXEC_CRASH_HOTPLUG_SUPPORT kexec flag, the kexec tool could mark all the required kexec segments on an architecture as safe to update. So rename the @update_elfcorehdr to @hotplug_support. If @hotplug_support is set, the kernel can safely update all the required kexec segments of the kdump image during CPU/Memory hotplug events. Introduce an architecture-specific function to process kexec flags for determining hotplug support. Set the @hotplug_support member of struct kimage for both kexec_load and kexec_file_load system calls. This simplifies kernel checks to identify hotplug support for the currently loaded kdump image by just examining the value of @hotplug_support. Signed-off-by: Sourabh Jain Cc: Akhil Raj Cc: Andrew Morton Cc: Aneesh Kumar K.V Cc: Baoquan He Cc: Borislav Petkov (AMD) Cc: Boris Ostrovsky Cc: Christophe Leroy Cc: Dave Hansen Cc: Dave Young Cc: David Hildenbrand Cc: Eric DeVolder Cc: Greg Kroah-Hartman Cc: Hari Bathini Cc: Laurent Dufour Cc: Mahesh Salgaonkar Cc: Michael Ellerman Cc: Mimi Zohar Cc: Naveen N Rao Cc: Oscar Salvador Cc: Thomas Gleixner Cc: Valentin Schneider Cc: Vivek Goyal Cc: ke...@lists.infradead.org Cc: x...@kernel.org --- arch/x86/include/asm/kexec.h | 11 ++- arch/x86/kernel/crash.c | 28 +--- drivers/base/cpu.c | 2 +- drivers/base/memory.c| 2 +- include/linux/kexec.h| 27 +-- include/uapi/linux/kexec.h | 1 + kernel/crash_core.c | 11 --- kernel/kexec.c | 4 ++-- kernel/kexec_file.c | 5 + 9 files changed, 50 insertions(+), 41 deletions(-) diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h index 9bb6607e864e..8be622e82ba8 100644 --- a/arch/x86/include/asm/kexec.h +++ b/arch/x86/include/asm/kexec.h @@ -211,15 +211,8 @@ extern void kdump_nmi_shootdown_cpus(void); void arch_crash_handle_hotplug_event(struct kimage *image, void *arg); #define arch_crash_handle_hotplug_event arch_crash_handle_hotplug_event -#ifdef CONFIG_HOTPLUG_CPU -int arch_crash_hotplug_cpu_support(void); -#define crash_hotplug_cpu_support arch_crash_hotplug_cpu_support -#endif - -#ifdef CONFIG_MEMORY_HOTPLUG -int arch_crash_hotplug_memory_support(void); -#define crash_hotplug_memory_support arch_crash_hotplug_memory_support -#endif +int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags); +#define arch_crash_hotplug_support arch_crash_hotplug_support unsigned int arch_crash_get_elfcorehdr_size(void); #define crash_get_elfcorehdr_size arch_crash_get_elfcorehdr_size diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index 44744e9c68ec..7072aaee2ea0 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -398,20 +398,26 @@ int crash_load_segments(struct kimage *image) #undef pr_fmt #define pr_fmt(fmt) "crash hp: " fmt -/* These functions provide the value for the sysfs crash_hotplug nodes */ -#ifdef CONFIG_HOTPLUG_CPU -int arch_crash_hotplug_cpu_support(void) +int arch_crash_hotplug_support(struct kimage *image, unsigned long kexec_flags) { - return crash_check_update_elfcorehdr(); -} -#endif -#ifdef CONFIG_MEMORY_HOTPLUG -int arch_crash_hotplug_memory_support(void) -{ - return
Re: [PATCH 02/11] cxl: Convert to platform remove callback returning void
On Wed, 2024-02-21 at 10:53 +0100, Uwe Kleine-König wrote: > The .remove() callback for a platform driver returns an int which > makes > many driver authors wrongly assume it's possible to do error handling > by > returning an error code. However the value returned is ignored (apart > from emitting a warning) and this typically results in resource > leaks. > > To improve here there is a quest to make the remove callback return > void. In the first step of this quest all drivers are converted to > .remove_new(), which already returns void. Eventually after all > drivers > are converted, .remove_new() will be renamed to .remove(). > > Trivially convert this driver from always returning zero in the > remove > callback to the void returning variant. > > Signed-off-by: Uwe Kleine-König Acked-by: Andrew Donnellan > --- > drivers/misc/cxl/of.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c > index 25ce725035e7..bcc005dff1c0 100644 > --- a/drivers/misc/cxl/of.c > +++ b/drivers/misc/cxl/of.c > @@ -431,7 +431,7 @@ int cxl_of_read_adapter_properties(struct cxl > *adapter, struct device_node *np) > return 0; > } > > -static int cxl_of_remove(struct platform_device *pdev) > +static void cxl_of_remove(struct platform_device *pdev) > { > struct cxl *adapter; > int afu; > @@ -441,7 +441,6 @@ static int cxl_of_remove(struct platform_device > *pdev) > cxl_guest_remove_afu(adapter->afu[afu]); > > cxl_guest_remove_adapter(adapter); > - return 0; > } > > static void cxl_of_shutdown(struct platform_device *pdev) > @@ -501,6 +500,6 @@ struct platform_driver cxl_of_driver = { > .owner = THIS_MODULE > }, > .probe = cxl_of_probe, > - .remove = cxl_of_remove, > + .remove_new = cxl_of_remove, > .shutdown = cxl_of_shutdown, > }; -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 1/1] PCI/portdrv: Allow DPC if the OS controls AER natively.
[+cc Mahesh, Oliver, linuxppc-dev, since I mentioned powerpc below. Probably not of interest since this is about the ACPI EDR feature, but just FYI] On Wed, Feb 21, 2024 at 05:11:04PM -0600, Bjorn Helgaas wrote: > On Tue, Jan 23, 2024 at 09:59:21AM -0600, Bjorn Helgaas wrote: > > On Mon, Jan 22, 2024 at 06:37:48PM -0800, Kuppuswamy Sathyanarayanan wrote: > > > On 1/22/24 11:32 AM, Bjorn Helgaas wrote: > > > > On Mon, Jan 08, 2024 at 05:15:08PM -0700, Matthew W Carlis wrote: > > > >> A small part is probably historical; we've been using DPC on PCIe > > > >> switches since before there was any EDR support in the kernel. It > > > >> looks like there was a PCIe DPC ECN as early as Feb 2012, but this > > > >> EDR/DPC fw ECN didn't come in till Jan 2019 & kernel support for ECN > > > >> was even later. Its not immediately clear I would want to use EDR in > > > >> my newer architecures & then there are also the older architecures > > > >> still requiring support. When I submitted this patch I came at it > > > >> with the approach of trying to keep the old behavior & still support > > > >> the newer EDR behavior. Bjorns patch from Dec 28 2023 would seem to > > > >> change the behavior for both root ports & switch ports, requiring > > > >> them to set _OSC Control Field bit 7 (DPC) and _OSC Support Field > > > >> bit 7 (EDR) or a kernel command line value. I think no matter what, > > > >> we want to ensure that PCIe Root Ports and PCIe switches arrive at > > > >> the same policy here. > > > > Is there an approved DPC ECN to the PCI Firmware spec that adds DPC > > > > control negotiation, but does *not* add the EDR requirement? > > > > > > > > I'm looking at > > > > https://members.pcisig.com/wg/PCI-SIG/document/previewpdf/12888, which > > > > seems to be the final "Downstream Port Containment Related > > > > Enhancements" ECN, which is dated 1/28/2019 and applies to the PCI > > > > Firmware spec r3.2. > > > > > > > > It adds bit 7, "PCI Express Downstream Port Containment Configuration > > > > control", to the passed-in _OSC Control field, which indicates that > > > > the OS supports both "native OS control and firmware ownership models > > > > (i.e. Error Disconnect Recover notification) of Downstream Port > > > > Containment." > > > > > > > > It also adds the dependency that "If the OS sets bit 7 of the Control > > > > field, it must set bit 7 of the Support field, indicating support for > > > > the Error Disconnect Recover event." > > > > > > > > So I'm trying to figure out if the "support DPC but not EDR" situation > > > > was ever a valid place to be. Maybe it's a mistake to have separate > > > > CONFIG_PCIE_DPC and CONFIG_PCIE_EDR options. > > > > > > My understanding is also similar. I have raised the same point in > > > https://lore.kernel.org/all/3c02a6d6-917e-486c-ad41-bdf176639...@linux.intel.com/ > > > > Ah, sorry, I missed that. > > > > > IMO, we don't need a separate config for EDR. I don't think user can > > > gain anything with disabling EDR and enabling DPC. As long as > > > firmware does not user EDR support, just compiling the code should > > > be harmless. > > > > > > So we can either remove it, or select it by default if user selects > > > DPC config. > > > > > > > CONFIG_PCIE_EDR depends on CONFIG_ACPI, so the situation is a little > > > > bit murky on non-ACPI systems that support DPC. > > > > > > If we are going to remove the EDR config, it might need #ifdef > > > CONFIG_ACPI changes in edr.c to not compile ACPI specific code. > > > Alternative choice is to compile edr.c with CONFIG_ACPI. > > > > Right. I think we should probably remove CONFIG_PCIE_EDR completely > > and make everything controlled by CONFIG_PCIE_DPC. > > In the PCI Firmware spec, r3.3, sec 4.5.1, table 4-4, the description > of "Error Disconnect Recover Supported" hints at the possibility for > an OS to support EDR but not DPC: > > In the context of PCIe, support for Error Disconnect Recover implies > that the operating system will invalidate the software state > associated with child devices of the port without attempting to > access the child device hardware. *If* the operating system supports > Downstream Port Containment (DPC), ..., the operating system shall > attempt to recover the child devices if the port implements the > Downstream Port Containment Extended Capability. > > On the other hand, sec 4.6.12 and the implementation note there with > the EDR flow seem to assume the OS *does* support DPC and can clear > error status and bring ports out of DPC even if the OS hasn't been > granted control of DPC. > > EDR is an ACPI feature, and I guess it might be theoretically possible > to use EDR on an ACPI system with some non-DPC error containment > feature like powerpc EEH. But obviously powerpc doesn't use ACPI, and > a hypothetical ACPI system with non-DPC error containment would have > to add support for that containment in edr_handle_event(). > > So while I'm not 100% sure that making
Re: [PATCH v4 1/2] powerpc: Add Power11 architected and raw mode
On Wed, 21 Feb 2024 15:46:22 +1100, Michael Ellerman wrote: > Add CPU table entries for raw and architected mode. Most fields are > copied from the Power10 table entries. > > CPU, MMU and user (ELF_HWCAP) features are unchanged vs P10. However > userspace can detect P11 because the AT_PLATFORM value changes to > "power11". > > [...] Applied to powerpc/next. [1/2] powerpc: Add Power11 architected and raw mode https://git.kernel.org/powerpc/c/c2ed087ed35ca569d8179924ba560be248c758e5 [2/2] powerpc/perf: Power11 Performance Monitoring support https://git.kernel.org/powerpc/c/b22ea627225b53ec7ce25c19d6df9fa8217d1643 cheers
Re: [PATCH] powerpc: remove unused KCSAN_SANITIZE_early_64.o in Makefile
On Fri, 16 Feb 2024 22:58:17 +0900, Masahiro Yamada wrote: > Commit 2fb857bc9f9e ("powerpc/kcsan: Add exclusions from instrumentation") > added KCSAN_SANITIZE_early_64.o to arch/powerpc/kernel/Makefile, while > it does not compile early_64.o. > > Applied to powerpc/next. [1/1] powerpc: remove unused KCSAN_SANITIZE_early_64.o in Makefile https://git.kernel.org/powerpc/c/97a5253d7c3076ba0dbba8bf30179e079c9c912b cheers
Re: [PATCH] arch/powerpc: Remove duplicate ifdefs
On Fri, 16 Feb 2024 11:00:16 +0530, Shrikanth Hegde wrote: > When a ifdef is used in the below manner, second one could be considered as > duplicate. > > ifdef DEFINE_A > ...code block... > ifdef DEFINE_A <-- This is a duplicate. > ...code block... > endif > else > ifndef DEFINE_A <-- This is also duplicate. > ...code block... > endif > endif > More details about the script and methods used to find these code > patterns are in cover letter of [1] > > [...] Applied to powerpc/next. [1/1] arch/powerpc: Remove duplicate ifdefs https://git.kernel.org/powerpc/c/8c328de8fd5046eb3ec5a7ff7b682a8175e986c3 cheers
Re: [PATCH 2/2] powerpc: Don't ignore errors from set_memory_{n}p() in __kernel_map_pages()
Christophe Leroy writes: > Le 21/02/2024 à 13:09, Michael Ellerman a écrit : >> Christophe Leroy writes: >>> set_memory_p() and set_memory_np() can fail. >>> >>> As mentioned in linux/mm.h: >>> >>> /* >>> * To support DEBUG_PAGEALLOC architecture must ensure that >>> * __kernel_map_pages() never fails >>> */ >>> >>> So panic in case set_memory_p() or set_memory_np() fail >>> in __kernel_map_pages(). >>> >>> Link: https://github.com/KSPP/linux/issues/7 >>> Signed-off-by: Christophe Leroy >>> --- >>> arch/powerpc/include/asm/book3s/64/hash.h | 2 +- >>> arch/powerpc/mm/book3s64/hash_utils.c | 3 ++- >>> arch/powerpc/mm/pageattr.c| 10 +++--- >>> 3 files changed, 10 insertions(+), 5 deletions(-) >>> >> ... >>> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c >>> index 16b8d20d6ca8..62b678585878 100644 >>> --- a/arch/powerpc/mm/pageattr.c >>> +++ b/arch/powerpc/mm/pageattr.c >>> @@ -106,17 +106,21 @@ int change_memory_attr(unsigned long addr, int >>> numpages, long action) >>> #ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC >>> void __kernel_map_pages(struct page *page, int numpages, int enable) >>> { >>> + int err; >>> unsigned long addr = (unsigned long)page_address(page); >>> >>> if (PageHighMem(page)) >>> return; >>> >>> if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) >>> - hash__kernel_map_pages(page, numpages, enable); >>> + err = hash__kernel_map_pages(page, numpages, enable); >>> else if (enable) >>> - set_memory_p(addr, numpages); >>> + err = set_memory_p(addr, numpages); >>> else >>> - set_memory_np(addr, numpages); >>> + err = set_memory_np(addr, numpages); >>> + >>> + if (err) >>> + panic("%s: set_memory_%sp() failed\n", enable ? "" : "n"); >> >> This doesn't compile, it's missing __func__ I guess. > > Damn, I was too quick when I took into account checkpatch's feedback, > sorry for that. > >> >> Seems like we could keep it simpler though, it should hopefully never >> happen anyway, eg: >> >>panic("%s: changing memory protections failed\n", __func__); > > Sure, let's do that. Do you want a v2 or you do the change directly ? No need for a v2, I'll just fix it up when applying. cheers
Re: [PATCH 0/4] arm64: mm: support dynamic vmalloc/pmd configuration
> On Wednesday, February 21, 2024 at 1:32 AM, Christophe Leroy wrote: > > On powerpc (book3s/32) we have more or less the same although it is not > directly linked to PMDs: the virtual 4G address space is split in > segments of 256M. On each segment there's a bit called NX to forbit > execution. Vmalloc space is allocated in a segment with NX bit set while > Module spare is allocated in a segment with NX bit unset. We never have > to override vmalloc wrappers. All consumers of exec memory allocate it > using module_alloc() while vmalloc() provides non-exec memory. > > For modules, all you have to do is select > ARCH_WANTS_MODULES_DATA_IN_VMALLOC and module data will be allocated > using vmalloc() hence non-exec memory in our case. This critique has led me to some valuable ideas, and I can definitely find a simpler approach without overrides. I do want to mention changes to how VMALLOC_* and MODULE_* constants are used on arm64 may introduce other issues. See discussion/code on the patch that motivated this patch at: https://lore.kernel.org/all/CAP5Mv+ydhk=Ob4b40ZahGMgT-5+-VEHxtmA=-lkjieoou+k...@mail.gmail.com/ In short, maybe the issue of code/data intermixing requires a rework of arm64 memory infrastructure, but I see a potentially elegant solution here based on the comments given on this patch. Thanks, Maxwell
Re: [PATCH 1/4] mm/vmalloc: allow arch-specific vmalloc_node overrides
> On Wednesday, February 21, 2024 12:59 AM, Christophe Leroy wrote: > > In the code you add __weak for that. But you also add the flags to the > parameters and I can't understand why when reading the above description. This change was made to allow most kernel interfaces use vmalloc_node and enable the overrides to work. It also reduces the number of kernel locations which would need to be change if there was ever a change to the vmalloc_node_range interface. However, there is a pushback to overriding the vmalloc interface, so this change will likely not show up in my final patch. Regards, Maxwell
RE: [External] Re: [PATCH 2/4] mm: pgalloc: support address-conditional pmd allocation
> On February 21, 2024 3:27 AM David Hildenbrand wrote > On 21.02.24 08:13, Christophe Leroy wrote: > > Le 20/02/2024 à 21:32, Maxwell Bland a écrit : > >> > >> While other descriptors (e.g. pud) allow allocations conditional on > >> which virtual address is allocated, pmd descriptor allocations do not. > >> However, adding support for this is straightforward and is beneficial to > >> future kernel development targeting the PMD memory granularity. > >> > >> As many architectures already implement pmd_populate_kernel in an > >> address-generic manner, it is necessary to roll out support > >> incrementally. For this purpose a preprocessor flag, > > > > Is it really worth it ? It is only 48 call sites that need to be > > updated. It would avoid that processor flag and avoid introducing that > > pmd_populate_kernel_at() in kernel core. > > +1, let's avoid that if possible. Will fix, thank you! Maxwell
RE: [External] Re: [PATCH 0/4] arm64: mm: support dynamic vmalloc/pmd configuration
> From: Conor Dooley > FYI: > > > mm/vmalloc: allow arch-specific vmalloc_node overrides > > mm: pgalloc: support address-conditional pmd allocation > > With these two arch/riscv/configs/* are broken with calls to undeclared > functions. Will fix, thanks! I will also figure out how to make sure this doesn't happen again for some other architecture. > > arm64: separate code and data virtual memory allocation > > arm64: dynamic enforcement of pmd-level PXNTable > > And with these two the 32-bit and nommu builds are broken. Was not aware there was a dependency here. I will see what I can do. Thank you, Maxwell
Re: [PATCH 0/4] arm64: mm: support dynamic vmalloc/pmd configuration
Hey Maxwell, FYI: > mm/vmalloc: allow arch-specific vmalloc_node overrides > mm: pgalloc: support address-conditional pmd allocation With these two arch/riscv/configs/* are broken with calls to undeclared functions. > arm64: separate code and data virtual memory allocation > arm64: dynamic enforcement of pmd-level PXNTable And with these two the 32-bit and nommu builds are broken. Cheers, Conor. signature.asc Description: PGP signature
[PATCH] ASoC: soc-card: Fix missing locking in snd_soc_card_get_kcontrol()
snd_soc_card_get_kcontrol() must be holding a read lock on card->controls_rwsem while walking the controls list. Compare with snd_ctl_find_numid(). The existing function is renamed snd_soc_card_get_kcontrol_locked() so that it can be called from contexts that are already holding card->controls_rwsem (for example, control get/put functions). There are few direct or indirect callers of snd_soc_card_get_kcontrol(), and most are safe. Three require changes, which have been included in this patch: codecs/cs35l45.c: cs35l45_activate_ctl() is called from a control put() function so is changed to call snd_soc_card_get_kcontrol_locked(). codecs/cs35l56.c: cs35l56_sync_asp1_mixer_widgets_with_firmware() is called from control get()/put() functions so is changed to call snd_soc_card_get_kcontrol_locked(). fsl/fsl_xcvr.c: fsl_xcvr_activate_ctl() is called from three places, one of which already holds card->controls_rwsem: 1. fsl_xcvr_mode_put(), a control put function, which will already be holding card->controls_rwsem. 2. fsl_xcvr_startup(), a DAI startup function. 3. fsl_xcvr_shutdown(), a DAI shutdown function. To fix this, fsl_xcvr_activate_ctl() has been changed to call snd_soc_card_get_kcontrol_locked() so that it is safe to call directly from fsl_xcvr_mode_put(). The fsl_xcvr_startup() and fsl_xcvr_shutdown() functions have been changed to take a read lock on card->controls_rsem() around calls to fsl_xcvr_activate_ctl(). While this is not very elegant, it keeps the change small, to avoid this patch creating a large collateral churn in fsl/fsl_xcvr.c. Analysis of other callers of snd_soc_card_get_kcontrol() is that they do not need any changes, they are not holding card->controls_rwsem when they call snd_soc_card_get_kcontrol(). Direct callers of snd_soc_card_get_kcontrol(): fsl/fsl_spdif.c: fsl_spdif_dai_probe() - DAI probe function fsl/fsl_micfil.c: voice_detected_fn() - IRQ handler Indirect callers via soc_component_notify_control(): codecs/cs42l43: cs42l43_mic_shutter() - IRQ handler codecs/cs42l43: cs42l43_spk_shutter() - IRQ handler codecs/ak4118.c: ak4118_irq_handler() - IRQ handler codecs/wm_adsp.c: wm_adsp_write_ctl() - not currently used Indirect callers via snd_soc_limit_volume(): qcom/sc8280xp.c: sc8280xp_snd_init() - DAIlink init function ti/rx51.c: rx51_aic34_init() - DAI init function I don't have hardware to test the fsl/*, qcom/sc828xp.c, ti/rx51.c and ak4118.c changes. Backport note: The fsl/, qcom/, cs35l45, cs35l56 and cs42l43 callers were added since the Fixes commit so won't all be present on older kernels. Signed-off-by: Richard Fitzgerald Fixes: 209c6cdfd283 ("ASoC: soc-card: move snd_soc_card_get_kcontrol() to soc-card") --- It would be great if people could test the fsl/, qcom/, ti/rx51 and ak4418 drivers. --- include/sound/soc-card.h | 2 ++ sound/soc/codecs/cs35l45.c | 2 +- sound/soc/codecs/cs35l56.c | 2 +- sound/soc/fsl/fsl_xcvr.c | 12 +++- sound/soc/soc-card.c | 24 ++-- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/include/sound/soc-card.h b/include/sound/soc-card.h index ecc02e955279..1f4c39922d82 100644 --- a/include/sound/soc-card.h +++ b/include/sound/soc-card.h @@ -30,6 +30,8 @@ static inline void snd_soc_card_mutex_unlock(struct snd_soc_card *card) struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card, const char *name); +struct snd_kcontrol *snd_soc_card_get_kcontrol_locked(struct snd_soc_card *soc_card, + const char *name); int snd_soc_card_jack_new(struct snd_soc_card *card, const char *id, int type, struct snd_soc_jack *jack); int snd_soc_card_jack_new_pins(struct snd_soc_card *card, const char *id, diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c index 44c221745c3b..2392c6effed8 100644 --- a/sound/soc/codecs/cs35l45.c +++ b/sound/soc/codecs/cs35l45.c @@ -184,7 +184,7 @@ static int cs35l45_activate_ctl(struct snd_soc_component *component, else snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "%s", ctl_name); - kcontrol = snd_soc_card_get_kcontrol(component->card, name); + kcontrol = snd_soc_card_get_kcontrol_locked(component->card, name); if (!kcontrol) { dev_err(component->dev, "Can't find kcontrol %s\n", name); return -EINVAL; diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c index 2c1313e34cce..6dd0319bc843 100644 --- a/sound/soc/codecs/cs35l56.c +++ b/sound/soc/codecs/cs35l56.c @@ -114,7 +114,7 @@ static int cs35l56_sync_asp1_mixer_widgets_with_firmware(struct cs35l56_private name = full_name; } - kcontrol = snd_soc_card_get_kcontrol(dapm->card, name); + kcontrol =
Re: [PATCH v2 00/14] Split crash out from kexec and clean up related config items
On Wed, 21 Feb 2024 11:15:00 +0530 Hari Bathini wrote: > On 04/02/24 8:56 am, Baoquan He wrote: > >>> Hope Hari and Pingfan can help have a look, see if > >>> it's doable. Now, I make it either have both kexec and crash enabled, or > >>> disable both of them altogether. > >> > >> Sure. I will take a closer look... > > Thanks a lot. Please feel free to post patches to make that, or I can do > > it with your support or suggestion. > > Tested your changes and on top of these changes, came up with the below > changes to get it working for powerpc: > > > https://lore.kernel.org/all/20240213113150.1148276-1-hbath...@linux.ibm.com/ So can we take it that you're OK with Baoquan's series as-is? Baoquan, do you believe the patches in mm-unstable are ready for moving into mm-stable in preparation for an upstream merge?
Re: [PATCH v2 01/14] kexec: split crashkernel reservation code out from crash_core.c
On Wed, 21 Feb 2024 22:59:47 +0530 Sourabh Jain wrote: > > config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION > > - def_bool CRASH_CORE > > + def_bool CRASH_RESEERVE > > %s/CRASH_RESEERVE/CRASH_RESERVE? Yes, thanks, this has been addressed in a followon fixup patch in the mm.git tree.
Re: [PATCH] tty: hvc: Don't enable the RISC-V SBI console by default
Hello: This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt : On Wed, 14 Feb 2024 07:34:30 -0800 you wrote: > From: Palmer Dabbelt > > The new SBI console has the same problem as the old one: there's only > one shared backing hardware and no synchronization, so the two drivers > end up stepping on each other. This was the same issue the old SBI-0.1 > console drivers had, but that was disabled by default when SBI-0.1 was. > > [...] Here is the summary with links: - tty: hvc: Don't enable the RISC-V SBI console by default https://git.kernel.org/riscv/c/481860974faa You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH RFC net] ps3/gelic: Fix possible NULL pointer dereference
This driver is PPC so I have never looked at the code before. I noticed another issue that was introduced last December in commit 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures"). net/ethernet/toshiba/ps3_gelic_net.c 375 static int gelic_descr_prepare_rx(struct gelic_card *card, 376struct gelic_descr *descr) 377 { 378 static const unsigned int rx_skb_size = 379 ALIGN(GELIC_NET_MAX_FRAME, GELIC_NET_RXBUF_ALIGN) + 380 GELIC_NET_RXBUF_ALIGN - 1; 381 dma_addr_t cpu_addr; 382 int offset; 383 384 if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) 385 dev_info(ctodev(card), "%s: ERROR status\n", __func__); 386 387 descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size); 388 if (!descr->skb) { 389 descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */ 390 return -ENOMEM; 391 } 392 descr->hw_regs.dmac_cmd_status = 0; 393 descr->hw_regs.result_size = 0; 394 descr->hw_regs.valid_size = 0; 395 descr->hw_regs.data_error = 0; 396 descr->hw_regs.payload.dev_addr = 0; 397 descr->hw_regs.payload.size = 0; 398 descr->skb = NULL; ^^ NULL 399 400 offset = ((unsigned long)descr->skb->data) & Dereferenced here. 401 (GELIC_NET_RXBUF_ALIGN - 1); 402 if (offset) 403 skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset); 404 /* io-mmu-map the skb */ 405 cpu_addr = dma_map_single(ctodev(card), descr->skb->data, 406GELIC_NET_MAX_FRAME, DMA_FROM_DEVICE); regards, dan carpenter
[PATCH net-next] ps3/gelic: minor Kernel Doc corrections
* Update the Kernel Doc for gelic_descr_set_tx_cmdstat() and gelic_net_setup_netdev() so that documented name and the actual name of the function match. * Move define of GELIC_ALIGN() so that it is no longer between gelic_alloc_card_net() and it's Kernel Doc. * Document netdev parameter of gelic_alloc_card_net() in a way consistent to the documentation of other netdev parameters in this file. Addresses the following warnings flagged by ./scripts/kernel-doc -none: .../ps3_gelic_net.c:711: warning: expecting prototype for gelic_net_set_txdescr_cmdstat(). Prototype was for gelic_descr_set_tx_cmdstat() instead .../ps3_gelic_net.c:1474: warning: expecting prototype for gelic_ether_setup_netdev(). Prototype was for gelic_net_setup_netdev() instead .../ps3_gelic_net.c:1528: warning: expecting prototype for gelic_alloc_card_net(). Prototype was for GELIC_ALIGN() instead .../ps3_gelic_net.c:1531: warning: Function parameter or struct member 'netdev' not described in 'gelic_alloc_card_net' Signed-off-by: Simon Horman --- drivers/net/ethernet/toshiba/ps3_gelic_net.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c index d5b75af163d3..12b96ca66877 100644 --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c @@ -698,7 +698,7 @@ gelic_card_get_next_tx_descr(struct gelic_card *card) } /** - * gelic_net_set_txdescr_cmdstat - sets the tx descriptor command field + * gelic_descr_set_tx_cmdstat - sets the tx descriptor command field * @descr: descriptor structure to fill out * @skb: packet to consider * @@ -1461,7 +1461,7 @@ static void gelic_ether_setup_netdev_ops(struct net_device *netdev, } /** - * gelic_ether_setup_netdev - initialization of net_device + * gelic_net_setup_netdev - initialization of net_device * @netdev: net_device structure * @card: card structure * @@ -1518,14 +1518,16 @@ int gelic_net_setup_netdev(struct net_device *netdev, struct gelic_card *card) return 0; } +#define GELIC_ALIGN (32) + /** * gelic_alloc_card_net - allocates net_device and card structure + * @netdev: interface device structure * * returns the card structure or NULL in case of errors * * the card and net_device structures are linked to each other */ -#define GELIC_ALIGN (32) static struct gelic_card *gelic_alloc_card_net(struct net_device **netdev) { struct gelic_card *card;
Re: [PATCH v2 02/14] crash: split vmcoreinfo exporting code out from crash_core.c
Hello Baoquan, On 19/01/24 20:22, Baoquan He wrote: Now move the relevant codes into separate files: kernel/crash_reserve.c, include/linux/crash_reserve.h. And add config item CRASH_RESERVE to control its enabling. Feels like this patch is more about vmcore_info.[c|h] and CONFIG_VMCORE_INFO then the above mentioned files and config. - Sourabh And also update the old ifdeffery of CONFIG_CRASH_CORE, including of and config item dependency on CRASH_CORE accordingly. And also do renaming as follows: - arch/xxx/kernel/{crash_core.c => vmcore_info.c} because they are only related to vmcoreinfo exporting on x86, arm64, riscv. And also Remove config item CRASH_CORE, and rely on CONFIG_KEXEC_CORE to decide if build in crash_core.c. Signed-off-by: Baoquan He --- arch/arm64/kernel/Makefile| 2 +- .../kernel/{crash_core.c => vmcore_info.c}| 2 +- arch/powerpc/Kconfig | 2 +- arch/powerpc/kernel/setup-common.c| 2 +- arch/powerpc/platforms/powernv/opal-core.c| 2 +- arch/riscv/kernel/Makefile| 2 +- .../kernel/{crash_core.c => vmcore_info.c}| 2 +- arch/x86/kernel/Makefile | 2 +- .../{crash_core_32.c => vmcore_info_32.c} | 2 +- .../{crash_core_64.c => vmcore_info_64.c} | 2 +- drivers/firmware/qemu_fw_cfg.c| 14 +- fs/proc/Kconfig | 2 +- fs/proc/kcore.c | 2 +- include/linux/buildid.h | 2 +- include/linux/crash_core.h| 73 -- include/linux/kexec.h | 1 + include/linux/vmcore_info.h | 81 ++ kernel/Kconfig.kexec | 4 +- kernel/Makefile | 4 +- kernel/crash_core.c | 208 kernel/ksysfs.c | 6 +- kernel/printk/printk.c| 4 +- kernel/vmcore_info.c | 233 ++ lib/buildid.c | 2 +- 24 files changed, 345 insertions(+), 311 deletions(-) rename arch/arm64/kernel/{crash_core.c => vmcore_info.c} (97%) rename arch/riscv/kernel/{crash_core.c => vmcore_info.c} (96%) rename arch/x86/kernel/{crash_core_32.c => vmcore_info_32.c} (90%) rename arch/x86/kernel/{crash_core_64.c => vmcore_info_64.c} (94%) create mode 100644 include/linux/vmcore_info.h create mode 100644 kernel/vmcore_info.c diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index d95b3d6b471a..bcf89587a549 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -66,7 +66,7 @@ obj-$(CONFIG_KEXEC_FILE) += machine_kexec_file.o kexec_image.o obj-$(CONFIG_ARM64_RELOC_TEST)+= arm64-reloc-test.o arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o obj-$(CONFIG_CRASH_DUMP) += crash_dump.o -obj-$(CONFIG_CRASH_CORE) += crash_core.o +obj-$(CONFIG_VMCORE_INFO) += vmcore_info.o obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o obj-$(CONFIG_ARM64_MTE) += mte.o diff --git a/arch/arm64/kernel/crash_core.c b/arch/arm64/kernel/vmcore_info.c similarity index 97% rename from arch/arm64/kernel/crash_core.c rename to arch/arm64/kernel/vmcore_info.c index 66cde752cd74..a5abf7186922 100644 --- a/arch/arm64/kernel/crash_core.c +++ b/arch/arm64/kernel/vmcore_info.c @@ -4,7 +4,7 @@ * Copyright (C) Huawei Futurewei Technologies. */ -#include +#include #include #include #include diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6aeab95f0edd..1520146d7c2c 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -690,7 +690,7 @@ config ARCH_SELECTS_CRASH_DUMP config FA_DUMP bool "Firmware-assisted dump" depends on PPC64 && (PPC_RTAS || PPC_POWERNV) - select CRASH_CORE + select VMCORE_INFO select CRASH_RESERVE select CRASH_DUMP help diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 9b142b9d5187..733f210ffda1 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -109,7 +109,7 @@ int ppc_do_canonicalize_irqs; EXPORT_SYMBOL(ppc_do_canonicalize_irqs); #endif -#ifdef CONFIG_CRASH_CORE +#ifdef CONFIG_VMCORE_INFO /* This keeps a track of which one is the crashing cpu. */ int crashing_cpu = -1; #endif diff --git a/arch/powerpc/platforms/powernv/opal-core.c b/arch/powerpc/platforms/powernv/opal-core.c index bb7657115f1d..c9a9b759cc92 100644 --- a/arch/powerpc/platforms/powernv/opal-core.c +++ b/arch/powerpc/platforms/powernv/opal-core.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include
Re: [PATCH v2 01/14] kexec: split crashkernel reservation code out from crash_core.c
Hello Baoquan, Thank you for reorganizing the kexec and kdump code with a well-defined configuration structure. While reviewing the patch series, I noticed a few typos. On 19/01/24 20:22, Baoquan He wrote: Both kdump and fa_dump of ppc rely on crashkernel reservation. Move the relevant codes into separate files: crash_reserve.c, include/linux/crash_reserve.h. And also add config item CRASH_RESERVE to control its enabling of the codes. And update config items which has relationship with crashkernel reservation. And also change ifdeffery from CONFIG_CRASH_CORE to CONFIG_CRASH_RESERVE when those scopes are only crashkernel reservation related. And also rename arch/XXX/include/asm/{crash_core.h => crash_reserve.h} on arm64, x86 and risc-v because those architectures' crash_core.h is only related to crashkernel reservation. Signed-off-by: Baoquan He --- arch/arm64/Kconfig| 2 +- .../asm/{crash_core.h => crash_reserve.h} | 4 +- arch/powerpc/Kconfig | 1 + arch/powerpc/mm/nohash/kaslr_booke.c | 4 +- arch/riscv/Kconfig| 2 +- .../asm/{crash_core.h => crash_reserve.h} | 4 +- arch/x86/Kconfig | 2 +- .../asm/{crash_core.h => crash_reserve.h} | 6 +- include/linux/crash_core.h| 40 -- include/linux/crash_reserve.h | 48 ++ include/linux/kexec.h | 1 + kernel/Kconfig.kexec | 5 +- kernel/Makefile | 1 + kernel/crash_core.c | 438 - kernel/crash_reserve.c| 464 ++ 15 files changed, 531 insertions(+), 491 deletions(-) rename arch/arm64/include/asm/{crash_core.h => crash_reserve.h} (81%) rename arch/riscv/include/asm/{crash_core.h => crash_reserve.h} (78%) rename arch/x86/include/asm/{crash_core.h => crash_reserve.h} (92%) create mode 100644 include/linux/crash_reserve.h create mode 100644 kernel/crash_reserve.c diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index ea01a2c43efa..d96bc3c67ec6 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1501,7 +1501,7 @@ config ARCH_SUPPORTS_CRASH_DUMP def_bool y config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION - def_bool CRASH_CORE + def_bool CRASH_RESERVE config TRANS_TABLE def_bool y diff --git a/arch/arm64/include/asm/crash_core.h b/arch/arm64/include/asm/crash_reserve.h similarity index 81% rename from arch/arm64/include/asm/crash_core.h rename to arch/arm64/include/asm/crash_reserve.h index 9f5c8d339f44..4afe027a4e7b 100644 --- a/arch/arm64/include/asm/crash_core.h +++ b/arch/arm64/include/asm/crash_reserve.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef _ARM64_CRASH_CORE_H -#define _ARM64_CRASH_CORE_H +#ifndef _ARM64_CRASH_RESERVE_H +#define _ARM64_CRASH_RESERVE_H /* Current arm64 boot protocol requires 2MB alignment */ #define CRASH_ALIGN SZ_2M diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 414b978b8010..6aeab95f0edd 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -691,6 +691,7 @@ config FA_DUMP bool "Firmware-assisted dump" depends on PPC64 && (PPC_RTAS || PPC_POWERNV) select CRASH_CORE + select CRASH_RESERVE select CRASH_DUMP help A robust mechanism to get reliable kernel crash dump with diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c b/arch/powerpc/mm/nohash/kaslr_booke.c index b4f2786a7d2b..cdff129abb14 100644 --- a/arch/powerpc/mm/nohash/kaslr_booke.c +++ b/arch/powerpc/mm/nohash/kaslr_booke.c @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include @@ -173,7 +173,7 @@ static __init bool overlaps_region(const void *fdt, u32 start, static void __init get_crash_kernel(void *fdt, unsigned long size) { -#ifdef CONFIG_CRASH_CORE +#ifdef CONFIG_CRASH_RESERVE unsigned long long crash_size, crash_base; int ret; diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index b549499eb363..37a438c23deb 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -712,7 +712,7 @@ config ARCH_SUPPORTS_CRASH_DUMP def_bool y config ARCH_HAS_GENERIC_CRASHKERNEL_RESERVATION - def_bool CRASH_CORE + def_bool CRASH_RESERVE config COMPAT bool "Kernel support for 32-bit U-mode" diff --git a/arch/riscv/include/asm/crash_core.h b/arch/riscv/include/asm/crash_reserve.h similarity index 78% rename from arch/riscv/include/asm/crash_core.h rename to arch/riscv/include/asm/crash_reserve.h index e1874b23feaf..013962e63587 100644 --- a/arch/riscv/include/asm/crash_core.h +++ b/arch/riscv/include/asm/crash_reserve.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#ifndef
[PATCH RFC net] ps3/gelic: Fix possible NULL pointer dereference
Fix possible NULL pointer dereference in gelic_card_release_tx_chain() The cited commit introduced a netdev variable to gelic_card_release_tx_chain() which is set unconditionally on each iteration of a for loop. It is set to the value of tx_chain->tail->skb->dev. However, in some cases it is assumed that tx_chain->tail->skb may be NULL. And if that occurs, setting netdev will cause a NULl pointer dereference. Given the age of this code I do wonder if this can occur in practice. But to be on the safe side this patch assumes that it can and aims to avoid the dereference in the case where tx_chain->tail->skb may be NULL. Flagged by Smatch. Compile tested only. Fixes: 589866f9f1cb ("PS3: gelic: Add support for dual network interface") Signed-off-by: Simon Horman --- drivers/net/ethernet/toshiba/ps3_gelic_net.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c index d5b75af163d3..f03489799f5d 100644 --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c @@ -549,14 +549,13 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop) { struct gelic_descr_chain *tx_chain; enum gelic_descr_dma_status status; - struct net_device *netdev; int release = 0; for (tx_chain = >tx_chain; tx_chain->head != tx_chain->tail && tx_chain->tail; tx_chain->tail = tx_chain->tail->next) { status = gelic_descr_get_status(tx_chain->tail); - netdev = tx_chain->tail->skb->dev; + switch (status) { case GELIC_DESCR_DMA_RESPONSE_ERROR: case GELIC_DESCR_DMA_PROTECTION_ERROR: @@ -566,11 +565,14 @@ static void gelic_card_release_tx_chain(struct gelic_card *card, int stop) "%s: forcing end of tx descriptor " \ "with status %x\n", __func__, status); - netdev->stats.tx_dropped++; + tx_chain->tail->skb->dev->stats.tx_dropped++; break; case GELIC_DESCR_DMA_COMPLETE: if (tx_chain->tail->skb) { + struct net_device *netdev; + + netdev = tx_chain->tail->skb->dev; netdev->stats.tx_packets++; netdev->stats.tx_bytes += tx_chain->tail->skb->len;
[PATCH 3/3] arch: Rename fbdev header and source files
The per-architecture fbdev code has no dependencies on fbdev and can be used for any video-related subsystem. Rename the files to 'video'. Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE. Further update all includes statements, includ guards, and Makefiles. Also update a few strings and comments to refer to video instead of fbdev. Signed-off-by: Thomas Zimmermann Cc: Vineet Gupta Cc: Catalin Marinas Cc: Will Deacon Cc: Huacai Chen Cc: WANG Xuerui Cc: Geert Uytterhoeven Cc: Thomas Bogendoerfer Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Yoshinori Sato Cc: Rich Felker Cc: John Paul Adrian Glaubitz Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/arc/include/asm/fb.h| 8 arch/arc/include/asm/video.h | 8 arch/arm/include/asm/fb.h| 6 -- arch/arm/include/asm/video.h | 6 ++ arch/arm64/include/asm/fb.h | 10 -- arch/arm64/include/asm/video.h | 10 ++ arch/loongarch/include/asm/{fb.h => video.h} | 8 arch/m68k/include/asm/{fb.h => video.h} | 8 arch/mips/include/asm/{fb.h => video.h} | 12 ++-- arch/parisc/include/asm/{fb.h => video.h}| 8 arch/parisc/video/Makefile | 2 +- arch/parisc/video/{fbdev.c => video-sti.c} | 2 +- arch/powerpc/include/asm/{fb.h => video.h} | 8 arch/powerpc/kernel/pci-common.c | 2 +- arch/sh/include/asm/fb.h | 7 --- arch/sh/include/asm/video.h | 7 +++ arch/sparc/include/asm/{fb.h => video.h} | 8 arch/sparc/video/Makefile| 2 +- arch/sparc/video/{fbdev.c => video.c}| 4 ++-- arch/x86/include/asm/{fb.h => video.h} | 8 arch/x86/video/Makefile | 2 +- arch/x86/video/{fbdev.c => video.c} | 3 ++- include/asm-generic/Kbuild | 2 +- include/asm-generic/{fb.h => video.h}| 6 +++--- include/linux/fb.h | 2 +- 25 files changed, 75 insertions(+), 74 deletions(-) delete mode 100644 arch/arc/include/asm/fb.h create mode 100644 arch/arc/include/asm/video.h delete mode 100644 arch/arm/include/asm/fb.h create mode 100644 arch/arm/include/asm/video.h delete mode 100644 arch/arm64/include/asm/fb.h create mode 100644 arch/arm64/include/asm/video.h rename arch/loongarch/include/asm/{fb.h => video.h} (86%) rename arch/m68k/include/asm/{fb.h => video.h} (86%) rename arch/mips/include/asm/{fb.h => video.h} (76%) rename arch/parisc/include/asm/{fb.h => video.h} (68%) rename arch/parisc/video/{fbdev.c => video-sti.c} (96%) rename arch/powerpc/include/asm/{fb.h => video.h} (76%) delete mode 100644 arch/sh/include/asm/fb.h create mode 100644 arch/sh/include/asm/video.h rename arch/sparc/include/asm/{fb.h => video.h} (89%) rename arch/sparc/video/{fbdev.c => video.c} (86%) rename arch/x86/include/asm/{fb.h => video.h} (77%) rename arch/x86/video/{fbdev.c => video.c} (97%) rename include/asm-generic/{fb.h => video.h} (96%) diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h deleted file mode 100644 index 9c2383d29cbb9..0 --- a/arch/arc/include/asm/fb.h +++ /dev/null @@ -1,8 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ - -#ifndef _ASM_FB_H_ -#define _ASM_FB_H_ - -#include - -#endif /* _ASM_FB_H_ */ diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h new file mode 100644 index 0..8ff7263727fe7 --- /dev/null +++ b/arch/arc/include/asm/video.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef _ASM_VIDEO_H_ +#define _ASM_VIDEO_H_ + +#include + +#endif /* _ASM_VIDEO_H_ */ diff --git a/arch/arm/include/asm/fb.h b/arch/arm/include/asm/fb.h deleted file mode 100644 index ce20a43c30339..0 --- a/arch/arm/include/asm/fb.h +++ /dev/null @@ -1,6 +0,0 @@ -#ifndef _ASM_FB_H_ -#define _ASM_FB_H_ - -#include - -#endif /* _ASM_FB_H_ */ diff --git a/arch/arm/include/asm/video.h b/arch/arm/include/asm/video.h new file mode 100644 index 0..f570565366e67 --- /dev/null +++ b/arch/arm/include/asm/video.h @@ -0,0 +1,6 @@ +#ifndef _ASM_VIDEO_H_ +#define _ASM_VIDEO_H_ + +#include + +#endif /* _ASM_VIDEO_H_ */ diff --git a/arch/arm64/include/asm/fb.h b/arch/arm64/include/asm/fb.h deleted file mode 100644 index 1a495d8fb2ce0..0 --- a/arch/arm64/include/asm/fb.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Copyright (C) 2012 ARM Ltd. - */ -#ifndef __ASM_FB_H_ -#define __ASM_FB_H_ - -#include - -#endif /* __ASM_FB_H_ */ diff --git a/arch/arm64/include/asm/video.h b/arch/arm64/include/asm/video.h new file mode 100644 index
[PATCH 1/3] arch: Select fbdev helpers with CONFIG_VIDEO
Various Kconfig options selected the per-architecture helpers for fbdev. But none of the contained code depends on fbdev. Standardize on CONFIG_VIDEO, which will allow to add more general helpers for video functionality. CONFIG_VIDEO protects each architecture's video/ directory. This allows for the use of more fine-grained control for each directory's files, such as the use of CONFIG_STI_CORE on parisc. Signed-off-by: Thomas Zimmermann Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/parisc/Makefile | 2 +- arch/sparc/Makefile | 4 ++-- arch/sparc/video/Makefile | 2 +- arch/x86/Makefile | 2 +- arch/x86/video/Makefile | 3 ++- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile index 316f84f1d15c8..21b8166a68839 100644 --- a/arch/parisc/Makefile +++ b/arch/parisc/Makefile @@ -119,7 +119,7 @@ export LIBGCC libs-y += arch/parisc/lib/ $(LIBGCC) -drivers-y += arch/parisc/video/ +drivers-$(CONFIG_VIDEO) += arch/parisc/video/ boot := arch/parisc/boot diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile index 5f60359361312..757451c3ea1df 100644 --- a/arch/sparc/Makefile +++ b/arch/sparc/Makefile @@ -59,8 +59,8 @@ endif libs-y += arch/sparc/prom/ libs-y += arch/sparc/lib/ -drivers-$(CONFIG_PM) += arch/sparc/power/ -drivers-$(CONFIG_FB) += arch/sparc/video/ +drivers-$(CONFIG_PM)+= arch/sparc/power/ +drivers-$(CONFIG_VIDEO) += arch/sparc/video/ boot := arch/sparc/boot diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile index 6baddbd58e4db..9dd82880a027a 100644 --- a/arch/sparc/video/Makefile +++ b/arch/sparc/video/Makefile @@ -1,3 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB) += fbdev.o +obj-y += fbdev.o diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 15a5f4f2ff0aa..c0ea612c62ebe 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -265,7 +265,7 @@ drivers-$(CONFIG_PCI)+= arch/x86/pci/ # suspend and hibernation support drivers-$(CONFIG_PM) += arch/x86/power/ -drivers-$(CONFIG_FB_CORE) += arch/x86/video/ +drivers-$(CONFIG_VIDEO) += arch/x86/video/ # boot loader support. Several targets are kept for legacy purposes diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile index 5ebe48752ffc4..9dd82880a027a 100644 --- a/arch/x86/video/Makefile +++ b/arch/x86/video/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_FB_CORE) += fbdev.o + +obj-y += fbdev.o -- 2.43.0
[PATCH 2/3] arch: Remove struct fb_info from video helpers
The per-architecture video helpers do not depend on struct fb_info or anything else from fbdev. Remove it from the interface and replace fb_is_primary_device() with video_is_primary_device(). The new helper is similar in functionality, but can operate on non-fbdev devices. Signed-off-by: Thomas Zimmermann Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: "David S. Miller" Cc: Andreas Larsson Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: x...@kernel.org Cc: "H. Peter Anvin" --- arch/parisc/include/asm/fb.h | 8 +--- arch/parisc/video/fbdev.c| 9 + arch/sparc/include/asm/fb.h | 7 --- arch/sparc/video/fbdev.c | 17 - arch/x86/include/asm/fb.h| 8 +--- arch/x86/video/fbdev.c | 18 +++--- drivers/video/fbdev/core/fbcon.c | 2 +- include/asm-generic/fb.h | 11 ++- 8 files changed, 41 insertions(+), 39 deletions(-) diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h index 658a8a7dc5312..ed2a195a3e762 100644 --- a/arch/parisc/include/asm/fb.h +++ b/arch/parisc/include/asm/fb.h @@ -2,11 +2,13 @@ #ifndef _ASM_FB_H_ #define _ASM_FB_H_ -struct fb_info; +#include + +struct device; #if defined(CONFIG_STI_CORE) -int fb_is_primary_device(struct fb_info *info); -#define fb_is_primary_device fb_is_primary_device +bool video_is_primary_device(struct device *dev); +#define video_is_primary_device video_is_primary_device #endif #include diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c index e4f8ac99fc9e0..540fa0c919d59 100644 --- a/arch/parisc/video/fbdev.c +++ b/arch/parisc/video/fbdev.c @@ -5,12 +5,13 @@ * Copyright (C) 2001-2002 Thomas Bogendoerfer */ -#include #include #include -int fb_is_primary_device(struct fb_info *info) +#include + +bool video_is_primary_device(struct device *dev) { struct sti_struct *sti; @@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info) return true; /* return true if it's the default built-in framebuffer driver */ - return (sti->dev == info->device); + return (sti->dev == dev); } -EXPORT_SYMBOL(fb_is_primary_device); +EXPORT_SYMBOL(video_is_primary_device); diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h index 24440c0fda490..07f0325d6921c 100644 --- a/arch/sparc/include/asm/fb.h +++ b/arch/sparc/include/asm/fb.h @@ -3,10 +3,11 @@ #define _SPARC_FB_H_ #include +#include #include -struct fb_info; +struct device; #ifdef CONFIG_SPARC32 static inline pgprot_t pgprot_framebuffer(pgprot_t prot, @@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot, #define pgprot_framebuffer pgprot_framebuffer #endif -int fb_is_primary_device(struct fb_info *info); -#define fb_is_primary_device fb_is_primary_device +bool video_is_primary_device(struct device *dev); +#define video_is_primary_device video_is_primary_device static inline void fb_memcpy_fromio(void *to, const volatile void __iomem *from, size_t n) { diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c index bff66dd1909a4..e46f0499c2774 100644 --- a/arch/sparc/video/fbdev.c +++ b/arch/sparc/video/fbdev.c @@ -1,26 +1,25 @@ // SPDX-License-Identifier: GPL-2.0 #include -#include +#include #include +#include #include -int fb_is_primary_device(struct fb_info *info) +bool video_is_primary_device(struct device *dev) { - struct device *dev = info->device; - struct device_node *node; + struct device_node *node = dev->of_node; if (console_set_on_cmdline) - return 0; + return false; - node = dev->of_node; if (node && node == of_console_device) - return 1; + return true; - return 0; + return false; } -EXPORT_SYMBOL(fb_is_primary_device); +EXPORT_SYMBOL(video_is_primary_device); MODULE_DESCRIPTION("Sparc fbdev helpers"); MODULE_LICENSE("GPL"); diff --git a/arch/x86/include/asm/fb.h b/arch/x86/include/asm/fb.h index c3b9582de7efd..999db33792869 100644 --- a/arch/x86/include/asm/fb.h +++ b/arch/x86/include/asm/fb.h @@ -2,17 +2,19 @@ #ifndef _ASM_X86_FB_H #define _ASM_X86_FB_H +#include + #include -struct fb_info; +struct device; pgprot_t pgprot_framebuffer(pgprot_t prot, unsigned long vm_start, unsigned long vm_end, unsigned long offset); #define pgprot_framebuffer pgprot_framebuffer -int fb_is_primary_device(struct fb_info *info); -#define fb_is_primary_device fb_is_primary_device +bool video_is_primary_device(struct device *dev); +#define video_is_primary_device video_is_primary_device #include diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c index 1dd6528cc947c..4d87ce8e257fe 100644 --- a/arch/x86/video/fbdev.c +++ b/arch/x86/video/fbdev.c @@ -7,7 +7,6 @@ * */ -#include #include
[PATCH 0/3] arch: Remove fbdev dependency from video helpers
Make architecture helpers for display functionality depend on general video functionality instead of fbdev. This avoid the dependency on fbdev and makes the functionality available for non-fbdev code. Patch 1 replaces the variety of Kconfig options that control the Makefiles with CONFIG_VIDEO. More fine-grained control of the build can then be done within each video/ directory; see sparc for an example. Patch 2 replaces fb_is_primary_device() with video_is_primary_device(), which has no dependencies on fbdev. The implementation remains identical on all affected platforms. There's one minor change in fbcon, which is the only caller of fb_is_primary_device(). Patch 3 renames the source and files from fbdev to video. Thomas Zimmermann (3): arch: Select fbdev helpers with CONFIG_VIDEO arch: Remove struct fb_info from video helpers arch: Rename fbdev header and source files arch/arc/include/asm/fb.h| 8 -- arch/arc/include/asm/video.h | 8 ++ arch/arm/include/asm/fb.h| 6 - arch/arm/include/asm/video.h | 6 + arch/arm64/include/asm/fb.h | 10 arch/arm64/include/asm/video.h | 10 arch/loongarch/include/asm/{fb.h => video.h} | 8 +++--- arch/m68k/include/asm/{fb.h => video.h} | 8 +++--- arch/mips/include/asm/{fb.h => video.h} | 12 - arch/parisc/Makefile | 2 +- arch/parisc/include/asm/fb.h | 14 --- arch/parisc/include/asm/video.h | 16 arch/parisc/video/Makefile | 2 +- arch/parisc/video/{fbdev.c => video-sti.c} | 9 --- arch/powerpc/include/asm/{fb.h => video.h} | 8 +++--- arch/powerpc/kernel/pci-common.c | 2 +- arch/sh/include/asm/fb.h | 7 -- arch/sh/include/asm/video.h | 7 ++ arch/sparc/Makefile | 4 +-- arch/sparc/include/asm/{fb.h => video.h} | 15 +-- arch/sparc/video/Makefile| 2 +- arch/sparc/video/fbdev.c | 26 arch/sparc/video/video.c | 25 +++ arch/x86/Makefile| 2 +- arch/x86/include/asm/fb.h| 19 -- arch/x86/include/asm/video.h | 21 arch/x86/video/Makefile | 3 ++- arch/x86/video/{fbdev.c => video.c} | 21 +++- drivers/video/fbdev/core/fbcon.c | 2 +- include/asm-generic/Kbuild | 2 +- include/asm-generic/{fb.h => video.h}| 17 +++-- include/linux/fb.h | 2 +- 32 files changed, 154 insertions(+), 150 deletions(-) delete mode 100644 arch/arc/include/asm/fb.h create mode 100644 arch/arc/include/asm/video.h delete mode 100644 arch/arm/include/asm/fb.h create mode 100644 arch/arm/include/asm/video.h delete mode 100644 arch/arm64/include/asm/fb.h create mode 100644 arch/arm64/include/asm/video.h rename arch/loongarch/include/asm/{fb.h => video.h} (86%) rename arch/m68k/include/asm/{fb.h => video.h} (86%) rename arch/mips/include/asm/{fb.h => video.h} (76%) delete mode 100644 arch/parisc/include/asm/fb.h create mode 100644 arch/parisc/include/asm/video.h rename arch/parisc/video/{fbdev.c => video-sti.c} (78%) rename arch/powerpc/include/asm/{fb.h => video.h} (76%) delete mode 100644 arch/sh/include/asm/fb.h create mode 100644 arch/sh/include/asm/video.h rename arch/sparc/include/asm/{fb.h => video.h} (75%) delete mode 100644 arch/sparc/video/fbdev.c create mode 100644 arch/sparc/video/video.c delete mode 100644 arch/x86/include/asm/fb.h create mode 100644 arch/x86/include/asm/video.h rename arch/x86/video/{fbdev.c => video.c} (66%) rename include/asm-generic/{fb.h => video.h} (89%) -- 2.43.0
[PATCH 3/6] powerpc: opal-prd: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- arch/powerpc/platforms/powernv/opal-prd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c index b66b06efcef1..24f04f20d3e8 100644 --- a/arch/powerpc/platforms/powernv/opal-prd.c +++ b/arch/powerpc/platforms/powernv/opal-prd.c @@ -425,12 +425,11 @@ static int opal_prd_probe(struct platform_device *pdev) return 0; } -static int opal_prd_remove(struct platform_device *pdev) +static void opal_prd_remove(struct platform_device *pdev) { misc_deregister(_prd_dev); opal_message_notifier_unregister(OPAL_MSG_PRD, _prd_event_nb); opal_message_notifier_unregister(OPAL_MSG_PRD2, _prd_event_nb2); - return 0; } static const struct of_device_id opal_prd_match[] = { @@ -444,7 +443,7 @@ static struct platform_driver opal_prd_driver = { .of_match_table = opal_prd_match, }, .probe = opal_prd_probe, - .remove = opal_prd_remove, + .remove_new = opal_prd_remove, }; module_platform_driver(opal_prd_driver); -- 2.43.0
[PATCH 1/6] powerpc: sgy_cts1000: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- arch/powerpc/platforms/85xx/sgy_cts1000.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c index 751395cbf022..34ce21f42623 100644 --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c @@ -114,7 +114,7 @@ static int gpio_halt_probe(struct platform_device *pdev) return ret; } -static int gpio_halt_remove(struct platform_device *pdev) +static void gpio_halt_remove(struct platform_device *pdev) { free_irq(halt_irq, pdev); cancel_work_sync(_halt_wq); @@ -124,8 +124,6 @@ static int gpio_halt_remove(struct platform_device *pdev) gpiod_put(halt_gpio); halt_gpio = NULL; - - return 0; } static const struct of_device_id gpio_halt_match[] = { @@ -145,7 +143,7 @@ static struct platform_driver gpio_halt_driver = { .of_match_table = gpio_halt_match, }, .probe = gpio_halt_probe, - .remove = gpio_halt_remove, + .remove_new = gpio_halt_remove, }; module_platform_driver(gpio_halt_driver); -- 2.43.0
[PATCH 0/6] powerpc: Convert to platform remove callback returning void
Hello, this series converts all platform drivers below drivers/powerpc to struct platform_driver::remove_new(). See commit 5c5a7680e67b ("platform: Provide a remove callback that returns no value") for an extended explanation and the eventual goal. All conversations are trivial, because their .remove() callbacks returned zero unconditionally. There are no interdependencies between these patches, so they could be picked up individually. But I'd hope that Michael picks them up all together. Best regards Uwe Uwe Kleine-König (6): powerpc: sgy_cts1000: Convert to platform remove callback returning void powerpc: gpio_mdio: Convert to platform remove callback returning void powerpc: opal-prd: Convert to platform remove callback returning void powerpc: papr_scm: Convert to platform remove callback returning void powerpc: fsl_msi: Convert to platform remove callback returning void powerpc: pmi: Convert to platform remove callback returning void arch/powerpc/platforms/85xx/sgy_cts1000.c | 6 ++ arch/powerpc/platforms/pasemi/gpio_mdio.c | 6 ++ arch/powerpc/platforms/powernv/opal-prd.c | 5 ++--- arch/powerpc/platforms/pseries/papr_scm.c | 6 ++ arch/powerpc/sysdev/fsl_msi.c | 6 ++ arch/powerpc/sysdev/pmi.c | 6 ++ 6 files changed, 12 insertions(+), 23 deletions(-) base-commit: 4893c639cc3659cefaa675bf1e59f4e7571afb5c -- 2.43.0
[PATCH 6/6] powerpc: pmi: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- arch/powerpc/sysdev/pmi.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/sysdev/pmi.c b/arch/powerpc/sysdev/pmi.c index fcf8d1516210..737f97fd67d7 100644 --- a/arch/powerpc/sysdev/pmi.c +++ b/arch/powerpc/sysdev/pmi.c @@ -173,7 +173,7 @@ static int pmi_of_probe(struct platform_device *dev) return rc; } -static int pmi_of_remove(struct platform_device *dev) +static void pmi_of_remove(struct platform_device *dev) { struct pmi_handler *handler, *tmp; @@ -189,13 +189,11 @@ static int pmi_of_remove(struct platform_device *dev) kfree(data); data = NULL; - - return 0; } static struct platform_driver pmi_of_platform_driver = { .probe = pmi_of_probe, - .remove = pmi_of_remove, + .remove_new = pmi_of_remove, .driver = { .name = "pmi", .of_match_table = pmi_match, -- 2.43.0
[PATCH 2/6] powerpc: gpio_mdio: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- arch/powerpc/platforms/pasemi/gpio_mdio.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c b/arch/powerpc/platforms/pasemi/gpio_mdio.c index fd130fe7a65a..4e983af32949 100644 --- a/arch/powerpc/platforms/pasemi/gpio_mdio.c +++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c @@ -260,7 +260,7 @@ static int gpio_mdio_probe(struct platform_device *ofdev) } -static int gpio_mdio_remove(struct platform_device *dev) +static void gpio_mdio_remove(struct platform_device *dev) { struct mii_bus *bus = dev_get_drvdata(>dev); @@ -271,8 +271,6 @@ static int gpio_mdio_remove(struct platform_device *dev) kfree(bus->priv); bus->priv = NULL; mdiobus_free(bus); - - return 0; } static const struct of_device_id gpio_mdio_match[] = @@ -287,7 +285,7 @@ MODULE_DEVICE_TABLE(of, gpio_mdio_match); static struct platform_driver gpio_mdio_driver = { .probe = gpio_mdio_probe, - .remove = gpio_mdio_remove, + .remove_new = gpio_mdio_remove, .driver = { .name = "gpio-mdio-bitbang", .of_match_table = gpio_mdio_match, -- 2.43.0
[PATCH 4/6] powerpc: papr_scm: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- arch/powerpc/platforms/pseries/papr_scm.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c index 1a53e048ceb7..c233f9db039b 100644 --- a/arch/powerpc/platforms/pseries/papr_scm.c +++ b/arch/powerpc/platforms/pseries/papr_scm.c @@ -1521,7 +1521,7 @@ err: kfree(p); return rc; } -static int papr_scm_remove(struct platform_device *pdev) +static void papr_scm_remove(struct platform_device *pdev) { struct papr_scm_priv *p = platform_get_drvdata(pdev); @@ -1538,8 +1538,6 @@ static int papr_scm_remove(struct platform_device *pdev) pdev->archdata.priv = NULL; kfree(p->bus_desc.provider_name); kfree(p); - - return 0; } static const struct of_device_id papr_scm_match[] = { @@ -1550,7 +1548,7 @@ static const struct of_device_id papr_scm_match[] = { static struct platform_driver papr_scm_driver = { .probe = papr_scm_probe, - .remove = papr_scm_remove, + .remove_new = papr_scm_remove, .driver = { .name = "papr_scm", .of_match_table = papr_scm_match, -- 2.43.0
[PATCH 5/6] powerpc: fsl_msi: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- arch/powerpc/sysdev/fsl_msi.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c index 558ec68d768e..8e6c84df4ca1 100644 --- a/arch/powerpc/sysdev/fsl_msi.c +++ b/arch/powerpc/sysdev/fsl_msi.c @@ -320,7 +320,7 @@ static irqreturn_t fsl_msi_cascade(int irq, void *data) return ret; } -static int fsl_of_msi_remove(struct platform_device *ofdev) +static void fsl_of_msi_remove(struct platform_device *ofdev) { struct fsl_msi *msi = platform_get_drvdata(ofdev); int virq, i; @@ -343,8 +343,6 @@ static int fsl_of_msi_remove(struct platform_device *ofdev) if ((msi->feature & FSL_PIC_IP_MASK) != FSL_PIC_IP_VMPIC) iounmap(msi->msi_regs); kfree(msi); - - return 0; } static struct lock_class_key fsl_msi_irq_class; @@ -603,7 +601,7 @@ static struct platform_driver fsl_of_msi_driver = { .of_match_table = fsl_of_msi_ids, }, .probe = fsl_of_msi_probe, - .remove = fsl_of_msi_remove, + .remove_new = fsl_of_msi_remove, }; static __init int fsl_of_msi_init(void) -- 2.43.0
Re: [PATCH v3 RESEND 3/6] bitmap: Make bitmap_onto() available to users
On Wed, Feb 21, 2024 at 02:44:31PM +0100, Herve Codina wrote: > On Thu, 15 Feb 2024 21:17:23 +0200 > Andy Shevchenko wrote: [...] > > > Now what's the plan ? > > > Andy, do you want to send a v2 of this patch or may I get the patch, > > > modify it > > > according to reviews already present in v1 and integrate it in my current > > > series ? > > > > I would like to do that, but under pile of different things. > > I would try my best but if you have enough time and motivation feel free > > to take over, address the comments and integrate in your series. > > > > I dunno what to do with bitmap_onto(), perhaps in a separate patch we can > > replace it with bitmap_scatter() (IIUC) with explanation that the former > > 1) uses atomic ops while being non-atomic as a whole, and b) having quite > > hard to get documentation. At least that's how I see it, I mean that I would > > like to leave bitmap_onto() alone and address it separately. > > I will take the Andy's bitmap_{scatter,gather}() patch in my next iteration. > And use bitmap_{scatter,gather}() in my code. Thank you and sorry that I have no time to finish that. I will be happy to help reviewing if you Cc me. > For bitmap_onto() replacement, nothing will be done in my next iteration as > it is out of this series scope. I agree on this. This will be a separate logical change related to NUMA with explanation and replacement of all callers at once. -- With Best Regards, Andy Shevchenko
Re: [PATCH 00/11] misc: Convert to platform remove callback returning void
On Wed, Feb 21, 2024, at 10:53, Uwe Kleine-König wrote: > Hello, > > this series converts all drivers below drivers/misc to struct > platform_driver::remove_new(). See commit 5c5a7680e67b ("platform: > Provide a remove callback that returns no value") for an extended > explanation and the eventual goal. > > All conversations are trivial, because their .remove() callbacks > returned zero unconditionally. > > There are no interdependencies between these patches, so they could be > picked up individually. But I'd hope that Greg or Arnd picks them up all > together. These all look good to me, whole series Acked-by: Arnd Bergmann
Re: [PATCH v3 RESEND 3/6] bitmap: Make bitmap_onto() available to users
Hi Andy, Yury, On Thu, 15 Feb 2024 21:17:23 +0200 Andy Shevchenko wrote: [...] > > Now what's the plan ? > > Andy, do you want to send a v2 of this patch or may I get the patch, modify > > it > > according to reviews already present in v1 and integrate it in my current > > series ? > > I would like to do that, but under pile of different things. > I would try my best but if you have enough time and motivation feel free > to take over, address the comments and integrate in your series. > > I dunno what to do with bitmap_onto(), perhaps in a separate patch we can > replace it with bitmap_scatter() (IIUC) with explanation that the former > 1) uses atomic ops while being non-atomic as a whole, and b) having quite > hard to get documentation. At least that's how I see it, I mean that I would > like to leave bitmap_onto() alone and address it separately. > I will take the Andy's bitmap_{scatter,gather}() patch in my next iteration. And use bitmap_{scatter,gather}() in my code. For bitmap_onto() replacement, nothing will be done in my next iteration as it is out of this series scope. Hervé
Re: [PATCH v2 00/14] Split crash out from kexec and clean up related config items
On 02/21/24 at 11:15am, Hari Bathini wrote: > Hi Baoquan, > > On 04/02/24 8:56 am, Baoquan He wrote: > > > > Hope Hari and Pingfan can help have a look, see if > > > > it's doable. Now, I make it either have both kexec and crash enabled, or > > > > disable both of them altogether. > > > > > > Sure. I will take a closer look... > > Thanks a lot. Please feel free to post patches to make that, or I can do > > it with your support or suggestion. > > Tested your changes and on top of these changes, came up with the below > changes to get it working for powerpc: > > > https://lore.kernel.org/all/20240213113150.1148276-1-hbath...@linux.ibm.com/ > > Please take a look. I added a comment to the patch 1 consulting if the "struct crash_mem" is appropriate to cover other cases except of kdump memory regions. I am wondering if its name need be adjusted, or other kind of memory you mentioned can use other structures or create a new one. If it's has to be done like that, it's fine.
Re: [PATCH v2 03/13] mm: Provide generic pmd_thp_or_huge()
On Wed, Feb 21, 2024 at 05:37:37PM +0800, Peter Xu wrote: > On Mon, Jan 15, 2024 at 01:55:51PM -0400, Jason Gunthorpe wrote: > > On Wed, Jan 03, 2024 at 05:14:13PM +0800, pet...@redhat.com wrote: > > > From: Peter Xu > > > > > > ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD. It > > > can be a helpful helper if we want to merge more THP and hugetlb code > > > paths. Make it a generic default implementation, only exist when > > > CONFIG_MMU. Arch can overwrite it by defining its own version. > > > > > > For example, ARM's pgtable-2level.h defines it to always return false. > > > > > > Keep the macro declared with all config, it should be optimized to a false > > > anyway if !THP && !HUGETLB. > > > > > > Signed-off-by: Peter Xu > > > --- > > > include/linux/pgtable.h | 4 > > > mm/gup.c| 3 +-- > > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > > index 466cf477551a..2b42e95a4e3a 100644 > > > --- a/include/linux/pgtable.h > > > +++ b/include/linux/pgtable.h > > > @@ -1362,6 +1362,10 @@ static inline int pmd_write(pmd_t pmd) > > > #endif /* pmd_write */ > > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > > > +#ifndef pmd_thp_or_huge > > > +#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) > > > +#endif > > > > Why not just use pmd_leaf() ? > > > > This GUP case seems to me exactly like what pmd_leaf() should really > > do and be used for.. > > I think I mostly agree with you, and these APIs are indeed confusing. IMHO > the challenge is about the risk of breaking others on small changes in the > details where evil resides. These APIs are super confusing, which is why I brought it up.. Adding even more subtly different variations is not helping. I think pmd_leaf means the entry is present and refers to a physical page not another radix level. > > eg x86 does: > > > > #define pmd_leafpmd_large > > static inline int pmd_large(pmd_t pte) > > return pmd_flags(pte) & _PAGE_PSE; > > > > static inline int pmd_trans_huge(pmd_t pmd) > > return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE; > > > > int pmd_huge(pmd_t pmd) > > return !pmd_none(pmd) && > > (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_PSE)) != _PAGE_PRESENT; > > For example, here I don't think it's strictly pmd_leaf()? As pmd_huge() > will return true if PRESENT=0 && PSE=0 (as long as none pte ruled out > first), while pmd_leaf() will return false; I think that came from > cbef8478bee5. Yikes, but do you even want to handle non-present entries in GUP world? Isn't everything gated by !present in the first place? > Besides that, there're also other cases where it's not clear of such direct > replacement, not until further investigated. E.g., arm-3level has: > > #define pmd_leaf(pmd) pmd_sect(pmd) > #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ >PMD_TYPE_SECT) > #define PMD_TYPE_SECT (_AT(pmdval_t, 1) << 0) > > While pmd_huge() there relies on PMD_TABLE_BIT () I looked at tht, it looked OK.. #define PMD_TYPE_MASK (_AT(pmdval_t, 3) << 0) #define PMD_TABLE_BIT (_AT(pmdval_t, 1) << 1) It is the same stuff, just a little confusingly written Jason
Re: [PATCH 2/2] powerpc: Don't ignore errors from set_memory_{n}p() in __kernel_map_pages()
Le 21/02/2024 à 13:09, Michael Ellerman a écrit : > Christophe Leroy writes: >> set_memory_p() and set_memory_np() can fail. >> >> As mentioned in linux/mm.h: >> >> /* >> * To support DEBUG_PAGEALLOC architecture must ensure that >> * __kernel_map_pages() never fails >> */ >> >> So panic in case set_memory_p() or set_memory_np() fail >> in __kernel_map_pages(). >> >> Link: https://github.com/KSPP/linux/issues/7 >> Signed-off-by: Christophe Leroy >> --- >> arch/powerpc/include/asm/book3s/64/hash.h | 2 +- >> arch/powerpc/mm/book3s64/hash_utils.c | 3 ++- >> arch/powerpc/mm/pageattr.c| 10 +++--- >> 3 files changed, 10 insertions(+), 5 deletions(-) >> > ... >> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c >> index 16b8d20d6ca8..62b678585878 100644 >> --- a/arch/powerpc/mm/pageattr.c >> +++ b/arch/powerpc/mm/pageattr.c >> @@ -106,17 +106,21 @@ int change_memory_attr(unsigned long addr, int >> numpages, long action) >> #ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC >> void __kernel_map_pages(struct page *page, int numpages, int enable) >> { >> +int err; >> unsigned long addr = (unsigned long)page_address(page); >> >> if (PageHighMem(page)) >> return; >> >> if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) >> -hash__kernel_map_pages(page, numpages, enable); >> +err = hash__kernel_map_pages(page, numpages, enable); >> else if (enable) >> -set_memory_p(addr, numpages); >> +err = set_memory_p(addr, numpages); >> else >> -set_memory_np(addr, numpages); >> +err = set_memory_np(addr, numpages); >> + >> +if (err) >> +panic("%s: set_memory_%sp() failed\n", enable ? "" : "n"); > > This doesn't compile, it's missing __func__ I guess. Damn, I was too quick when I took into account checkpatch's feedback, sorry for that. > > Seems like we could keep it simpler though, it should hopefully never > happen anyway, eg: > >panic("%s: changing memory protections failed\n", __func__); Sure, let's do that. Do you want a v2 or you do the change directly ? Thanks Christophe
Re: [PATCH 2/2] powerpc: Don't ignore errors from set_memory_{n}p() in __kernel_map_pages()
Christophe Leroy writes: > set_memory_p() and set_memory_np() can fail. > > As mentioned in linux/mm.h: > > /* > * To support DEBUG_PAGEALLOC architecture must ensure that > * __kernel_map_pages() never fails > */ > > So panic in case set_memory_p() or set_memory_np() fail > in __kernel_map_pages(). > > Link: https://github.com/KSPP/linux/issues/7 > Signed-off-by: Christophe Leroy > --- > arch/powerpc/include/asm/book3s/64/hash.h | 2 +- > arch/powerpc/mm/book3s64/hash_utils.c | 3 ++- > arch/powerpc/mm/pageattr.c| 10 +++--- > 3 files changed, 10 insertions(+), 5 deletions(-) > ... > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c > index 16b8d20d6ca8..62b678585878 100644 > --- a/arch/powerpc/mm/pageattr.c > +++ b/arch/powerpc/mm/pageattr.c > @@ -106,17 +106,21 @@ int change_memory_attr(unsigned long addr, int > numpages, long action) > #ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC > void __kernel_map_pages(struct page *page, int numpages, int enable) > { > + int err; > unsigned long addr = (unsigned long)page_address(page); > > if (PageHighMem(page)) > return; > > if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) > - hash__kernel_map_pages(page, numpages, enable); > + err = hash__kernel_map_pages(page, numpages, enable); > else if (enable) > - set_memory_p(addr, numpages); > + err = set_memory_p(addr, numpages); > else > - set_memory_np(addr, numpages); > + err = set_memory_np(addr, numpages); > + > + if (err) > + panic("%s: set_memory_%sp() failed\n", enable ? "" : "n"); This doesn't compile, it's missing __func__ I guess. Seems like we could keep it simpler though, it should hopefully never happen anyway, eg: panic("%s: changing memory protections failed\n", __func__); cheers
Re: [PATCH v2 06/13] mm/gup: Drop folio_fast_pin_allowed() in hugepd processing
On Mon, Jan 15, 2024 at 02:37:48PM -0400, Jason Gunthorpe wrote: > > Drop that check, not only because it'll never be true for hugepd per any > > known plan, but also it paves way for reusing the function outside > > fast-gup. > > I didn't see any other caller of this function in this series? When > does this re-use happen?? It's reused in patch 12 ("mm/gup: Handle hugepd for follow_page()"). Thanks, -- Peter Xu
Re: [PATCH v2 10/13] mm/gup: Handle huge pud for follow_pud_mask()
On Mon, Jan 15, 2024 at 02:49:00PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 03, 2024 at 05:14:20PM +0800, pet...@redhat.com wrote: > > diff --git a/mm/gup.c b/mm/gup.c > > index 63845b3ec44f..760406180222 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -525,6 +525,70 @@ static struct page *no_page_table(struct > > vm_area_struct *vma, > > return NULL; > > } > > > > +#ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES > > +static struct page *follow_huge_pud(struct vm_area_struct *vma, > > + unsigned long addr, pud_t *pudp, > > + int flags, struct follow_page_context *ctx) > > +{ > > + struct mm_struct *mm = vma->vm_mm; > > + struct page *page; > > + pud_t pud = *pudp; > > + unsigned long pfn = pud_pfn(pud); > > + int ret; > > + > > + assert_spin_locked(pud_lockptr(mm, pudp)); > > + > > + if ((flags & FOLL_WRITE) && !pud_write(pud)) > > + return NULL; > > + > > + if (!pud_present(pud)) > > + return NULL; > > + > > + pfn += (addr & ~PUD_MASK) >> PAGE_SHIFT; > > + > > +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD > > + if (pud_devmap(pud)) { > > Can this use IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD) ? Sure. > > > + /* > > +* device mapped pages can only be returned if the caller > > +* will manage the page reference count. > > +* > > +* At least one of FOLL_GET | FOLL_PIN must be set, so > > +* assert that here: > > +*/ > > + if (!(flags & (FOLL_GET | FOLL_PIN))) > > + return ERR_PTR(-EEXIST); > > + > > + if (flags & FOLL_TOUCH) > > + touch_pud(vma, addr, pudp, flags & FOLL_WRITE); > > + > > + ctx->pgmap = get_dev_pagemap(pfn, ctx->pgmap); > > + if (!ctx->pgmap) > > + return ERR_PTR(-EFAULT); > > + } > > +#endif /* CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */ > > + page = pfn_to_page(pfn); > > + > > + if (!pud_devmap(pud) && !pud_write(pud) && > > + gup_must_unshare(vma, flags, page)) > > + return ERR_PTR(-EMLINK); > > + > > + ret = try_grab_page(page, flags); > > + if (ret) > > + page = ERR_PTR(ret); > > + else > > + ctx->page_mask = HPAGE_PUD_NR - 1; > > + > > + return page; > > +} > > +#else /* CONFIG_PGTABLE_HAS_HUGE_LEAVES */ > > +static struct page *follow_huge_pud(struct vm_area_struct *vma, > > + unsigned long addr, pud_t *pudp, > > + int flags, struct follow_page_context *ctx) > > +{ > > + return NULL; > > +} > > +#endif /* CONFIG_PGTABLE_HAS_HUGE_LEAVES */ > > + > > static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long > > address, > > pte_t *pte, unsigned int flags) > > { > > @@ -760,11 +824,11 @@ static struct page *follow_pud_mask(struct > > vm_area_struct *vma, > > > > pudp = pud_offset(p4dp, address); > > pud = READ_ONCE(*pudp); > > - if (pud_none(pud)) > > + if (pud_none(pud) || !pud_present(pud)) > > return no_page_table(vma, flags, address); > > Isn't 'pud_none() || !pud_present()' redundent? A none pud is > non-present, by definition? Hmm yes, seems redundant. Let me drop it. > > > - if (pud_devmap(pud)) { > > + if (pud_huge(pud)) { > > ptl = pud_lock(mm, pudp); > > - page = follow_devmap_pud(vma, address, pudp, flags, > > >pgmap); > > + page = follow_huge_pud(vma, address, pudp, flags, ctx); > > spin_unlock(ptl); > > if (page) > > return page; > > Otherwise it looks OK to me > > Reviewed-by: Jason Gunthorpe Thanks! -- Peter Xu
Re: [PATCH v12 07/15] media: v4l2: Add audio capture and output support
On 21/02/2024 11:11, Shengjiu Wang wrote: > On Wed, Feb 21, 2024 at 12:30 PM Tomasz Figa wrote: >> >> On Sat, Feb 17, 2024 at 6:42 PM Mauro Carvalho Chehab >> wrote: >>> >>> Em Thu, 18 Jan 2024 20:32:00 +0800 >>> Shengjiu Wang escreveu: >>> Audio signal processing has the requirement for memory to memory similar as Video. This patch is to add this support in v4l2 framework, defined new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format for audio case usage. The created audio device is named "/dev/v4l-audioX". Signed-off-by: Shengjiu Wang --- .../userspace-api/media/v4l/buffer.rst| 6 ++ .../media/v4l/dev-audio-mem2mem.rst | 71 +++ .../userspace-api/media/v4l/devices.rst | 1 + .../media/v4l/vidioc-enum-fmt.rst | 2 + .../userspace-api/media/v4l/vidioc-g-fmt.rst | 4 ++ .../media/videodev2.h.rst.exceptions | 2 + .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 +++ drivers/media/v4l2-core/v4l2-dev.c| 17 + drivers/media/v4l2-core/v4l2-ioctl.c | 53 ++ include/media/v4l2-dev.h | 2 + include/media/v4l2-ioctl.h| 34 + include/uapi/linux/videodev2.h| 17 + 13 files changed, 222 insertions(+) create mode 100644 Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst index 52bbee81c080..a3754ca6f0d6 100644 --- a/Documentation/userspace-api/media/v4l/buffer.rst +++ b/Documentation/userspace-api/media/v4l/buffer.rst @@ -438,6 +438,12 @@ enum v4l2_buf_type * - ``V4L2_BUF_TYPE_META_OUTPUT`` - 14 >>> - Buffer for metadata output, see :ref:`metadata`. +* - ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` + - 15 + - Buffer for audio capture, see :ref:`audio`. +* - ``V4L2_BUF_TYPE_AUDIO_OUTPUT`` + - 16 >>> >>> Hmm... alsa APi define input/output as: >>> enum { >>> SNDRV_PCM_STREAM_PLAYBACK = 0, >>> SNDRV_PCM_STREAM_CAPTURE, >>> SNDRV_PCM_STREAM_LAST = SNDRV_PCM_STREAM_CAPTURE, >>> }; >>> >>> >>> I would use a namespace as close as possible to the >>> ALSA API. Also, we're not talking about V4L2, but, instead >>> audio. so, not sure if I like the prefix to start with >>> V4L2_. Maybe ALSA_? >>> >>> So, a better namespace would be: >>> >>> ${prefix}_BUF_TYPE_PCM_STREAM_PLAYBACK >>> and >>> ${prefix}_BUF_TYPE_PCM_STREAM_CAPTURE >>> >> >> The API is still V4L2, and all the other non-video buf types also use >> the V4L2_ prefix, so perhaps that's good here as well? >> >> Whether AUDIO or PCM_STREAM makes more sense goes outside of my >> expertise. Subjectively, a PCM stream sounds more specific than an >> audio stream. Do those buf types also support non-PCM audio streams? > > Currently I use it for PCM, but I think it can also be used for non-PCM. > So use the below name? > V4L2_BUF_TYPE_AUDIO_CAPTURE > V4L2_BUF_TYPE_AUDIO_PLAYBACK I really prefer keeping the names as they are in this patch. CAPTURE/OUTPUT is consistent with V4L2 nomenclature, and since this is a M2M device 'PLAYBACK' isn't really a good name either. It's not an audio playback device, it's a rate converter. Regards, Hans > >> + - Buffer for audio output, see :ref:`audio`. .. _buffer-flags: diff --git a/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst new file mode 100644 index ..68faecfe3a02 --- /dev/null +++ b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst @@ -0,0 +1,71 @@ +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later + +.. _audiomem2mem: + + +Audio Memory-To-Memory Interface + + +An audio memory-to-memory device can compress, decompress, transform, or +otherwise convert audio data from one format into another format, in memory. +Such memory-to-memory devices set the ``V4L2_CAP_AUDIO_M2M`` capability. +Examples of memory-to-memory devices are audio codecs, audio preprocessing, +audio postprocessing. + +A memory-to-memory audio node supports both output (sending audio frames from +memory to the hardware) and capture (receiving the processed audio frames +from the hardware into memory) stream I/O. An application will have to +setup the stream I/O for both sides and finally call
Re: [PATCH v12 07/15] media: v4l2: Add audio capture and output support
On 17/02/2024 10:42, Mauro Carvalho Chehab wrote: > Em Thu, 18 Jan 2024 20:32:00 +0800 > Shengjiu Wang escreveu: > >> Audio signal processing has the requirement for memory to >> memory similar as Video. >> >> This patch is to add this support in v4l2 framework, defined >> new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and >> V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format >> for audio case usage. >> >> The created audio device is named "/dev/v4l-audioX". >> >> Signed-off-by: Shengjiu Wang >> --- >> .../userspace-api/media/v4l/buffer.rst| 6 ++ >> .../media/v4l/dev-audio-mem2mem.rst | 71 +++ >> .../userspace-api/media/v4l/devices.rst | 1 + >> .../media/v4l/vidioc-enum-fmt.rst | 2 + >> .../userspace-api/media/v4l/vidioc-g-fmt.rst | 4 ++ >> .../media/videodev2.h.rst.exceptions | 2 + >> .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ >> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 +++ >> drivers/media/v4l2-core/v4l2-dev.c| 17 + >> drivers/media/v4l2-core/v4l2-ioctl.c | 53 ++ >> include/media/v4l2-dev.h | 2 + >> include/media/v4l2-ioctl.h| 34 + >> include/uapi/linux/videodev2.h| 17 + >> 13 files changed, 222 insertions(+) >> create mode 100644 >> Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst >> >> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst >> b/Documentation/userspace-api/media/v4l/buffer.rst >> index 52bbee81c080..a3754ca6f0d6 100644 >> --- a/Documentation/userspace-api/media/v4l/buffer.rst >> +++ b/Documentation/userspace-api/media/v4l/buffer.rst >> @@ -438,6 +438,12 @@ enum v4l2_buf_type >> * - ``V4L2_BUF_TYPE_META_OUTPUT`` >>- 14 > >>- Buffer for metadata output, see :ref:`metadata`. >> +* - ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` >> + - 15 >> + - Buffer for audio capture, see :ref:`audio`. >> +* - ``V4L2_BUF_TYPE_AUDIO_OUTPUT`` >> + - 16 > > Hmm... alsa APi define input/output as: > enum { > SNDRV_PCM_STREAM_PLAYBACK = 0, > SNDRV_PCM_STREAM_CAPTURE, > SNDRV_PCM_STREAM_LAST = SNDRV_PCM_STREAM_CAPTURE, > }; > > > I would use a namespace as close as possible to the > ALSA API. Also, we're not talking about V4L2, but, instead > audio. so, not sure if I like the prefix to start with > V4L2_. Maybe ALSA_? > > So, a better namespace would be: > > ${prefix}_BUF_TYPE_PCM_STREAM_PLAYBACK > and > ${prefix}_BUF_TYPE_PCM_STREAM_CAPTURE > >> + - Buffer for audio output, see :ref:`audio`. >> >> >> .. _buffer-flags: >> diff --git a/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst >> b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst >> new file mode 100644 >> index ..68faecfe3a02 >> --- /dev/null >> +++ b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst >> @@ -0,0 +1,71 @@ >> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later >> + >> +.. _audiomem2mem: >> + >> + >> +Audio Memory-To-Memory Interface >> + >> + >> +An audio memory-to-memory device can compress, decompress, transform, or >> +otherwise convert audio data from one format into another format, in memory. >> +Such memory-to-memory devices set the ``V4L2_CAP_AUDIO_M2M`` capability. >> +Examples of memory-to-memory devices are audio codecs, audio preprocessing, >> +audio postprocessing. >> + >> +A memory-to-memory audio node supports both output (sending audio frames >> from >> +memory to the hardware) and capture (receiving the processed audio frames >> +from the hardware into memory) stream I/O. An application will have to >> +setup the stream I/O for both sides and finally call >> +:ref:`VIDIOC_STREAMON ` for both capture and output to >> +start the hardware. >> + >> +Memory-to-memory devices function as a shared resource: you can >> +open the audio node multiple times, each application setting up their >> +own properties that are local to the file handle, and each can use >> +it independently from the others. The driver will arbitrate access to >> +the hardware and reprogram it whenever another file handler gets access. >> + >> +Audio memory-to-memory devices are accessed through character device >> +special files named ``/dev/v4l-audio`` >> + >> +Querying Capabilities >> += >> + >> +Device nodes supporting the audio memory-to-memory interface set the >> +``V4L2_CAP_AUDIO_M2M`` flag in the ``device_caps`` field of the >> +:c:type:`v4l2_capability` structure returned by the >> :c:func:`VIDIOC_QUERYCAP` >> +ioctl. >> + >> +Data Format Negotiation >> +=== >> + >> +The audio device uses the :ref:`format` ioctls to select the capture format. >> +The audio buffer content format is bound to that selected format. In >> addition >> +to the basic
Re: [PATCH v12 08/15] media: uapi: Define audio sample format fourcc type
On 19/02/2024 13:56, Mauro Carvalho Chehab wrote: > Em Mon, 19 Feb 2024 12:05:02 +0800 > Shengjiu Wang escreveu: > >> Hi Mauro >> >> On Sat, Feb 17, 2024 at 5:19 PM Mauro Carvalho Chehab >> wrote: >>> >>> Em Thu, 18 Jan 2024 20:32:01 +0800 >>> Shengjiu Wang escreveu: >>> The audio sample format definition is from alsa, the header file is include/uapi/sound/asound.h, but don't include this header file directly, because in user space, there is another copy in alsa-lib. There will be conflict in userspace for include videodev2.h & asound.h and asoundlib.h Here still use the fourcc format. Signed-off-by: Shengjiu Wang --- .../userspace-api/media/v4l/pixfmt-audio.rst | 87 +++ .../userspace-api/media/v4l/pixfmt.rst| 1 + drivers/media/v4l2-core/v4l2-ioctl.c | 13 +++ include/uapi/linux/videodev2.h| 23 + 4 files changed, 124 insertions(+) create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-audio.rst diff --git a/Documentation/userspace-api/media/v4l/pixfmt-audio.rst b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst new file mode 100644 index ..04b4a7fbd8f4 --- /dev/null +++ b/Documentation/userspace-api/media/v4l/pixfmt-audio.rst @@ -0,0 +1,87 @@ +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later + +.. _pixfmt-audio: + +* +Audio Formats +* + +These formats are used for :ref:`audiomem2mem` interface only. + +.. tabularcolumns:: |p{5.8cm}|p{1.2cm}|p{10.3cm}| + +.. cssclass:: longtable + +.. flat-table:: Audio Format +:header-rows: 1 +:stub-columns: 0 +:widths: 3 1 4 + +* - Identifier + - Code + - Details +* .. _V4L2-AUDIO-FMT-S8: + + - ``V4L2_AUDIO_FMT_S8`` + - 'S8' + - Corresponds to SNDRV_PCM_FORMAT_S8 in ALSA +* .. _V4L2-AUDIO-FMT-S16-LE: >>> >>> Hmm... why can't we just use SNDRV_*_FORMAT_*? Those are already part of >>> an uAPI header. No need to add any abstraction here and/or redefine >>> what is there already at include/uapi/sound/asound.h. >>> >> Actually I try to avoid including the include/uapi/sound/asound.h. >> Because in user space, there is another copy in alsa-lib (asoundlib.h). >> There will be conflict in userspace when including videodev2.h and >> asoundlib.h. > > Well, alsasoundlib.h seems to be using the same definitions: > https://github.com/michaelwu/alsa-lib/blob/master/include/pcm.h > > So, I can't see what would be the actual issue, as both userspace library > and ALSA internal headers use the same magic numbers. > > You can still do things like: > > #ifdef __KERNEL__ > # include > #else > # include > #endif > > To avoid such kind of conflicts, if you need to have it included on > some header file. Yet, I can't see why you would need that. > > IMO, at uAPI headers, you just need to declare the uAPI audiofmt field > to be either __u32 or __u64, pointing it to where this value comes from > (on both userspace and Kernelspace. E. g.: > > /** > * struct v4l2_audio_format - audio data format definition > * @audioformat: > *an integer number matching the fields inside > *enum snd_pcm_format_t (e. g. `SNDRV_PCM_FORMAT_*`), as defined > *in include/uapi/sound/asound.h and > * > https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#gaa14b7f26877a812acbb39811364177f8. > * @channels: channel numbers > * @buffersize: maximum size in bytes required for data > */ > struct v4l2_audio_format { > __u32 audioformat; > __u32 channels; > __u32 buffersize; > } __attribute__ ((packed)); > > Then, at documentation you just need to point to where the > possible values for SNDRV_PCM_FORMAT_ are defined. No need to > document them one by one. > > With such definition, you'll only need to include sound/asound.h > within the kAPI scope. > >> >> And in the V4l framework, the fourcc type is commonly used in other >> cases (video, radio, touch, meta), to avoid changing common code >> a lot, so I think using fourcc definition for audio may be simpler. > > Those are real video streams (or a video-related streams, in the case > of metadata) where fourcc is widely used. There, it makes sense. > However, ALSA format definitions are already being used for a long time. > There's no sense on trying to reinvent it - or having an abstract layer > to convert from/to fourcc <==> enum snd_pcm_format_t. Just use what is > there already. The problem is that within V4L2 we use fourcc consistently to describe a format, including in VIDIOC_ENUM_FMT. And the expectation is that the
Re: [PATCH 2/4] mm: pgalloc: support address-conditional pmd allocation
On 21.02.24 08:13, Christophe Leroy wrote: Le 20/02/2024 à 21:32, Maxwell Bland a écrit : [Vous ne recevez pas souvent de courriers de mbl...@motorola.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] While other descriptors (e.g. pud) allow allocations conditional on which virtual address is allocated, pmd descriptor allocations do not. However, adding support for this is straightforward and is beneficial to future kernel development targeting the PMD memory granularity. As many architectures already implement pmd_populate_kernel in an address-generic manner, it is necessary to roll out support incrementally. For this purpose a preprocessor flag, Is it really worth it ? It is only 48 call sites that need to be updated. It would avoid that processor flag and avoid introducing that pmd_populate_kernel_at() in kernel core. +1, let's avoid that if possible. -- Cheers, David / dhildenb
Re: [PATCH 1/4] mm/vmalloc: allow arch-specific vmalloc_node overrides
Le 21/02/2024 à 06:43, Christoph Hellwig a écrit : > On Tue, Feb 20, 2024 at 02:32:53PM -0600, Maxwell Bland wrote: >> Present non-uniform use of __vmalloc_node and __vmalloc_node_range makes >> enforcing appropriate code and data seperation untenable on certain >> microarchitectures, as VMALLOC_START and VMALLOC_END are monolithic >> while the use of the vmalloc interface is non-monolithic: in particular, >> appropriate randomness in ASLR makes it such that code regions must fall >> in some region between VMALLOC_START and VMALLOC_end, but this >> necessitates that code pages are intermingled with data pages, meaning >> code-specific protections, such as arm64's PXNTable, cannot be >> performantly runtime enforced. > > That's not actually true. We have MODULE_START/END to separate them, > which is used by mips only for now. We have MODULES_VADDR and MODULES_END that are used by arm, arm64, loongarcg, powerpc, riscv, s390, sparc, x86_64 is_vmalloc_or_module_addr() is using MODULES_VADDR so I guess this function fails on mips ? > >> >> The solution to this problem allows architectures to override the >> vmalloc wrapper functions by enforcing that the rest of the kernel does >> not reimplement __vmalloc_node by using __vmalloc_node_range with the >> same parameters as __vmalloc_node or provides a __weak tag to those >> functions using __vmalloc_node_range with parameters repeating those of >> __vmalloc_node. > > I'm really not too happy about overriding the functions. Especially > as the separation is a generally good idea and it would be good to > move everyone (or at least all modern architectures) over to a scheme > like this.
Re: [PATCH 0/4] arm64: mm: support dynamic vmalloc/pmd configuration
Le 20/02/2024 à 21:32, Maxwell Bland a écrit : > [Vous ne recevez pas souvent de courriers de mbl...@motorola.com. Découvrez > pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > Reworks ARM's virtual memory allocation infrastructure to support > dynamic enforcement of page middle directory PXNTable restrictions > rather than only during the initial memory mapping. Runtime enforcement > of this bit prevents write-then-execute attacks, where malicious code is > staged in vmalloc'd data regions, and later the page table is changed to > make this code executable. > > Previously the entire region from VMALLOC_START to VMALLOC_END was > vulnerable, but now the vulnerable region is restricted to the 2GB > reserved by module_alloc, a region which is generally read-only and more > difficult to inject staging code into, e.g., data must pass the BPF > verifier. These changes also set the stage for other systems, such as > KVM-level (EL2) changes to mark page tables immutable and code page > verification changes, forging a path toward complete mitigation of > kernel exploits on ARM. > > Implementing this required minimal changes to the generic vmalloc > interface in the kernel to allow architecture overrides of some vmalloc > wrapper functions, refactoring vmalloc calls to use a standard interface > in the generic kernel, and passing the address parameter already passed > into PTE allocation to the pte_allocate child function call. > > The new arm64 vmalloc wrapper functions ensure vmalloc data is not > allocated into the region reserved for module_alloc. arm64 BPF and > kprobe code also see a two-line-change ensuring their allocations abide > by the segmentation of code from data. Finally, arm64's pmd_populate > function is modified to set the PXNTable bit appropriately. On powerpc (book3s/32) we have more or less the same although it is not directly linked to PMDs: the virtual 4G address space is split in segments of 256M. On each segment there's a bit called NX to forbit execution. Vmalloc space is allocated in a segment with NX bit set while Module spare is allocated in a segment with NX bit unset. We never have to override vmalloc wrappers. All consumers of exec memory allocate it using module_alloc() while vmalloc() provides non-exec memory. For modules, all you have to do is select ARCH_WANTS_MODULES_DATA_IN_VMALLOC and module data will be allocated using vmalloc() hence non-exec memory in our case. Christophe
Re: [PATCH 3/4] arm64: separate code and data virtual memory allocation
Le 20/02/2024 à 21:32, Maxwell Bland a écrit : > [Vous ne recevez pas souvent de courriers de mbl...@motorola.com. Découvrez > pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > Current BPF and kprobe instruction allocation interfaces do not match > the base kernel and intermingle code and data pages within the same > sections. In the case of BPF, this appears to be a result of code > duplication between the kernel's JIT compiler and arm64's JIT. However, > This is no longer necessary given the possibility of overriding vmalloc > wrapper functions. Why do you need to override vmalloc wrapper functions for that ? See powerpc, for kprobes, alloc_insn_page() uses module_alloc(). On powerpc, the approach is that vmalloc() provides non-exec memory while module_alloc() provides executable memory. Christophe
Re: [PATCH 2/4] mm: pgalloc: support address-conditional pmd allocation
Le 20/02/2024 à 21:32, Maxwell Bland a écrit : > [Vous ne recevez pas souvent de courriers de mbl...@motorola.com. Découvrez > pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > While other descriptors (e.g. pud) allow allocations conditional on > which virtual address is allocated, pmd descriptor allocations do not. > However, adding support for this is straightforward and is beneficial to > future kernel development targeting the PMD memory granularity. > > As many architectures already implement pmd_populate_kernel in an > address-generic manner, it is necessary to roll out support > incrementally. For this purpose a preprocessor flag, Is it really worth it ? It is only 48 call sites that need to be updated. It would avoid that processor flag and avoid introducing that pmd_populate_kernel_at() in kernel core. $ git grep -l pmd_populate_kernel -- arch/ | wc -l 48 > __HAVE_ARCH_ADDR_COND_PMD is introduced to capture whether the > architecture supports some feature requiring PMD allocation conditional > on virtual address. Some microarchitectures (e.g. arm64) support > configurations for table descriptors, for example to enforce Privilege > eXecute Never, which benefit from knowing the virtual memory addresses > referenced by PMDs. > > Thus two major arguments in favor of this change are (1) unformity of > allocation between PMD and other table descriptor types and (2) the > capability of address-specific PMD allocation. Can you give more details on that uniformity ? I can't find any function called pud_populate_kernel(). Previously, pmd_populate_kernel() had same arguments as pmd_populate(). Why not also updating pmd_populate() to keep consistancy ? (can be done in a follow-up patch, not in this patch). > > Signed-off-by: Maxwell Bland > --- > include/asm-generic/pgalloc.h | 18 ++ > include/linux/mm.h| 4 ++-- > mm/hugetlb_vmemmap.c | 4 ++-- > mm/kasan/init.c | 22 +- > mm/memory.c | 4 ++-- > mm/percpu.c | 2 +- > mm/pgalloc-track.h| 3 ++- > mm/sparse-vmemmap.c | 2 +- > 8 files changed, 41 insertions(+), 18 deletions(-) > > diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h > index 879e5f8aa5e9..e5cdce77c6e4 100644 > --- a/include/asm-generic/pgalloc.h > +++ b/include/asm-generic/pgalloc.h > @@ -142,6 +142,24 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, > unsigned long addr) > } > #endif > > +#ifdef __HAVE_ARCH_ADDR_COND_PMD > +static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp, > + pte_t *ptep, unsigned long address); > +#else > +static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp, > + pte_t *ptep); > +#endif > + > +static inline void pmd_populate_kernel_at(struct mm_struct *mm, pmd_t *pmdp, > + pte_t *ptep, unsigned long address) > +{ > +#ifdef __HAVE_ARCH_ADDR_COND_PMD > + pmd_populate_kernel(mm, pmdp, ptep, address); > +#else > + pmd_populate_kernel(mm, pmdp, ptep); > +#endif > +} > + > #ifndef __HAVE_ARCH_PMD_FREE > static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd) > { > diff --git a/include/linux/mm.h b/include/linux/mm.h > index f5a97dec5169..6a9d5ded428d 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -2782,7 +2782,7 @@ static inline void mm_dec_nr_ptes(struct mm_struct *mm) > {} > #endif > > int __pte_alloc(struct mm_struct *mm, pmd_t *pmd); > -int __pte_alloc_kernel(pmd_t *pmd); > +int __pte_alloc_kernel(pmd_t *pmd, unsigned long address); > > #if defined(CONFIG_MMU) > > @@ -2977,7 +2977,7 @@ pte_t *pte_offset_map_nolock(struct mm_struct *mm, > pmd_t *pmd, > NULL : pte_offset_map_lock(mm, pmd, address, ptlp)) > > #define pte_alloc_kernel(pmd, address) \ > - ((unlikely(pmd_none(*(pmd))) && __pte_alloc_kernel(pmd))? \ > + ((unlikely(pmd_none(*(pmd))) && __pte_alloc_kernel(pmd, address)) ? \ > NULL: pte_offset_kernel(pmd, address)) > > #if USE_SPLIT_PMD_PTLOCKS > diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c > index da177e49d956..1f5664b656f1 100644 > --- a/mm/hugetlb_vmemmap.c > +++ b/mm/hugetlb_vmemmap.c > @@ -58,7 +58,7 @@ static int vmemmap_split_pmd(pmd_t *pmd, struct page *head, > unsigned long start, > if (!pgtable) > return -ENOMEM; > > - pmd_populate_kernel(_mm, &__pmd, pgtable); > + pmd_populate_kernel_at(_mm, &__pmd, pgtable, addr); > > for (i = 0; i < PTRS_PER_PTE; i++, addr += PAGE_SIZE) { > pte_t entry, *pte; > @@ -81,7 +81,7 @@ static int vmemmap_split_pmd(pmd_t *pmd, struct page *head, > unsigned long start, > > /* Make pte visible before pmd. See comment in > pmd_install(). */ > smp_wmb(); > -
Re: [PATCH 1/4] mm/vmalloc: allow arch-specific vmalloc_node overrides
Le 20/02/2024 à 21:32, Maxwell Bland a écrit : > [Vous ne recevez pas souvent de courriers de mbl...@motorola.com. Découvrez > pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > Present non-uniform use of __vmalloc_node and __vmalloc_node_range makes > enforcing appropriate code and data seperation untenable on certain > microarchitectures, as VMALLOC_START and VMALLOC_END are monolithic > while the use of the vmalloc interface is non-monolithic: in particular, > appropriate randomness in ASLR makes it such that code regions must fall > in some region between VMALLOC_START and VMALLOC_end, but this > necessitates that code pages are intermingled with data pages, meaning > code-specific protections, such as arm64's PXNTable, cannot be > performantly runtime enforced. > > The solution to this problem allows architectures to override the > vmalloc wrapper functions by enforcing that the rest of the kernel does > not reimplement __vmalloc_node by using __vmalloc_node_range with the > same parameters as __vmalloc_node or provides a __weak tag to those > functions using __vmalloc_node_range with parameters repeating those of > __vmalloc_node. > > Two benefits of this approach are (1) greater flexibility to each > architecture for handling of virtual memory while not compromising the > kernel's vmalloc logic and (2) more uniform use of the __vmalloc_node > interface, reserving the more specialized __vmalloc_node_range for more > specialized cases, such as kasan's shadow memory. I'm not sure I understand the message. What I understand is that you allow architectures to override vmalloc_node(). In the code you add __weak for that. But you also add the flags to the parameters and I can't understand why when reading the above description. Christophe
Re: [PATCH v12 07/15] media: v4l2: Add audio capture and output support
On Wed, Feb 21, 2024 at 12:30 PM Tomasz Figa wrote: > > On Sat, Feb 17, 2024 at 6:42 PM Mauro Carvalho Chehab > wrote: > > > > Em Thu, 18 Jan 2024 20:32:00 +0800 > > Shengjiu Wang escreveu: > > > > > Audio signal processing has the requirement for memory to > > > memory similar as Video. > > > > > > This patch is to add this support in v4l2 framework, defined > > > new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and > > > V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format > > > for audio case usage. > > > > > > The created audio device is named "/dev/v4l-audioX". > > > > > > Signed-off-by: Shengjiu Wang > > > --- > > > .../userspace-api/media/v4l/buffer.rst| 6 ++ > > > .../media/v4l/dev-audio-mem2mem.rst | 71 +++ > > > .../userspace-api/media/v4l/devices.rst | 1 + > > > .../media/v4l/vidioc-enum-fmt.rst | 2 + > > > .../userspace-api/media/v4l/vidioc-g-fmt.rst | 4 ++ > > > .../media/videodev2.h.rst.exceptions | 2 + > > > .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ > > > drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 +++ > > > drivers/media/v4l2-core/v4l2-dev.c| 17 + > > > drivers/media/v4l2-core/v4l2-ioctl.c | 53 ++ > > > include/media/v4l2-dev.h | 2 + > > > include/media/v4l2-ioctl.h| 34 + > > > include/uapi/linux/videodev2.h| 17 + > > > 13 files changed, 222 insertions(+) > > > create mode 100644 > > > Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst > > > > > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst > > > b/Documentation/userspace-api/media/v4l/buffer.rst > > > index 52bbee81c080..a3754ca6f0d6 100644 > > > --- a/Documentation/userspace-api/media/v4l/buffer.rst > > > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > > > @@ -438,6 +438,12 @@ enum v4l2_buf_type > > > * - ``V4L2_BUF_TYPE_META_OUTPUT`` > > >- 14 > > > > >- Buffer for metadata output, see :ref:`metadata`. > > > +* - ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` > > > + - 15 > > > + - Buffer for audio capture, see :ref:`audio`. > > > +* - ``V4L2_BUF_TYPE_AUDIO_OUTPUT`` > > > + - 16 > > > > Hmm... alsa APi define input/output as: > > enum { > > SNDRV_PCM_STREAM_PLAYBACK = 0, > > SNDRV_PCM_STREAM_CAPTURE, > > SNDRV_PCM_STREAM_LAST = SNDRV_PCM_STREAM_CAPTURE, > > }; > > > > > > I would use a namespace as close as possible to the > > ALSA API. Also, we're not talking about V4L2, but, instead > > audio. so, not sure if I like the prefix to start with > > V4L2_. Maybe ALSA_? > > > > So, a better namespace would be: > > > > ${prefix}_BUF_TYPE_PCM_STREAM_PLAYBACK > > and > > ${prefix}_BUF_TYPE_PCM_STREAM_CAPTURE > > > > The API is still V4L2, and all the other non-video buf types also use > the V4L2_ prefix, so perhaps that's good here as well? > > Whether AUDIO or PCM_STREAM makes more sense goes outside of my > expertise. Subjectively, a PCM stream sounds more specific than an > audio stream. Do those buf types also support non-PCM audio streams? Currently I use it for PCM, but I think it can also be used for non-PCM. So use the below name? V4L2_BUF_TYPE_AUDIO_CAPTURE V4L2_BUF_TYPE_AUDIO_PLAYBACK > > > > + - Buffer for audio output, see :ref:`audio`. > > > > > > > > > .. _buffer-flags: > > > diff --git a/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst > > > b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst > > > new file mode 100644 > > > index ..68faecfe3a02 > > > --- /dev/null > > > +++ b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst > > > @@ -0,0 +1,71 @@ > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > > > + > > > +.. _audiomem2mem: > > > + > > > + > > > +Audio Memory-To-Memory Interface > > > + > > > + > > > +An audio memory-to-memory device can compress, decompress, transform, or > > > +otherwise convert audio data from one format into another format, in > > > memory. > > > +Such memory-to-memory devices set the ``V4L2_CAP_AUDIO_M2M`` capability. > > > +Examples of memory-to-memory devices are audio codecs, audio > > > preprocessing, > > > +audio postprocessing. > > > + > > > +A memory-to-memory audio node supports both output (sending audio frames > > > from > > > +memory to the hardware) and capture (receiving the processed audio frames > > > +from the hardware into memory) stream I/O. An application will have to > > > +setup the stream I/O for both sides and finally call > > > +:ref:`VIDIOC_STREAMON ` for both capture and output to > > > +start the hardware. > > > + > > > +Memory-to-memory devices function as a shared resource: you can > > > +open the audio node multiple times, each application setting up their > > > +own properties
[PATCH 02/11] cxl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König --- drivers/misc/cxl/of.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c index 25ce725035e7..bcc005dff1c0 100644 --- a/drivers/misc/cxl/of.c +++ b/drivers/misc/cxl/of.c @@ -431,7 +431,7 @@ int cxl_of_read_adapter_properties(struct cxl *adapter, struct device_node *np) return 0; } -static int cxl_of_remove(struct platform_device *pdev) +static void cxl_of_remove(struct platform_device *pdev) { struct cxl *adapter; int afu; @@ -441,7 +441,6 @@ static int cxl_of_remove(struct platform_device *pdev) cxl_guest_remove_afu(adapter->afu[afu]); cxl_guest_remove_adapter(adapter); - return 0; } static void cxl_of_shutdown(struct platform_device *pdev) @@ -501,6 +500,6 @@ struct platform_driver cxl_of_driver = { .owner = THIS_MODULE }, .probe = cxl_of_probe, - .remove = cxl_of_remove, + .remove_new = cxl_of_remove, .shutdown = cxl_of_shutdown, }; -- 2.43.0
[PATCH 00/11] misc: Convert to platform remove callback returning void
Hello, this series converts all drivers below drivers/misc to struct platform_driver::remove_new(). See commit 5c5a7680e67b ("platform: Provide a remove callback that returns no value") for an extended explanation and the eventual goal. All conversations are trivial, because their .remove() callbacks returned zero unconditionally. There are no interdependencies between these patches, so they could be picked up individually. But I'd hope that Greg or Arnd picks them up all together. Best regards Uwe Uwe Kleine-König (11): misc: atmel-ssc: Convert to platform remove callback returning void cxl: Convert to platform remove callback returning void misc: fastrpc: Convert to platform remove callback returning void misc: hisi_hikey_usb: Convert to platform remove callback returning void mei: vsc: Convert to platform remove callback returning void misc: open-dice: Convert to platform remove callback returning void misc: sram: Convert to platform remove callback returning void misc: ti-st: st_kim: Convert to platform remove callback returning void misc: vcpu_stall_detector: Convert to platform remove callback returning void misc: xilinx_sdfec: Convert to platform remove callback returning void misc: xilinx_tmr_inject: Convert to platform remove callback returning void drivers/misc/atmel-ssc.c | 6 ++ drivers/misc/cxl/of.c | 5 ++--- drivers/misc/fastrpc.c | 6 ++ drivers/misc/hisi_hikey_usb.c | 6 ++ drivers/misc/mei/platform-vsc.c| 6 ++ drivers/misc/open-dice.c | 5 ++--- drivers/misc/sram.c| 6 ++ drivers/misc/ti-st/st_kim.c| 5 ++--- drivers/misc/vcpu_stall_detector.c | 6 ++ drivers/misc/xilinx_sdfec.c| 5 ++--- drivers/misc/xilinx_tmr_inject.c | 5 ++--- 11 files changed, 22 insertions(+), 39 deletions(-) base-commit: 4893c639cc3659cefaa675bf1e59f4e7571afb5c -- 2.43.0
Re: [PATCH v2 03/13] mm: Provide generic pmd_thp_or_huge()
On Mon, Jan 15, 2024 at 01:55:51PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 03, 2024 at 05:14:13PM +0800, pet...@redhat.com wrote: > > From: Peter Xu > > > > ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD. It > > can be a helpful helper if we want to merge more THP and hugetlb code > > paths. Make it a generic default implementation, only exist when > > CONFIG_MMU. Arch can overwrite it by defining its own version. > > > > For example, ARM's pgtable-2level.h defines it to always return false. > > > > Keep the macro declared with all config, it should be optimized to a false > > anyway if !THP && !HUGETLB. > > > > Signed-off-by: Peter Xu > > --- > > include/linux/pgtable.h | 4 > > mm/gup.c| 3 +-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > > index 466cf477551a..2b42e95a4e3a 100644 > > --- a/include/linux/pgtable.h > > +++ b/include/linux/pgtable.h > > @@ -1362,6 +1362,10 @@ static inline int pmd_write(pmd_t pmd) > > #endif /* pmd_write */ > > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > > > +#ifndef pmd_thp_or_huge > > +#define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) > > +#endif > > Why not just use pmd_leaf() ? > > This GUP case seems to me exactly like what pmd_leaf() should really > do and be used for.. I think I mostly agree with you, and these APIs are indeed confusing. IMHO the challenge is about the risk of breaking others on small changes in the details where evil resides. > > eg x86 does: > > #define pmd_leaf pmd_large > static inline int pmd_large(pmd_t pte) > return pmd_flags(pte) & _PAGE_PSE; > > static inline int pmd_trans_huge(pmd_t pmd) > return (pmd_val(pmd) & (_PAGE_PSE|_PAGE_DEVMAP)) == _PAGE_PSE; > > int pmd_huge(pmd_t pmd) > return !pmd_none(pmd) && > (pmd_val(pmd) & (_PAGE_PRESENT|_PAGE_PSE)) != _PAGE_PRESENT; For example, here I don't think it's strictly pmd_leaf()? As pmd_huge() will return true if PRESENT=0 && PSE=0 (as long as none pte ruled out first), while pmd_leaf() will return false; I think that came from cbef8478bee5. I'm not sure whether that is the best solution, e.g., from a 1st glance it seems better to me to process swap entries separately (including both migration and poisoned entries).. Sparc has similar things there, which in that case I'm not sure whether a direct replace is always safe. Besides that, there're also other cases where it's not clear of such direct replacement, not until further investigated. E.g., arm-3level has: #define pmd_leaf(pmd) pmd_sect(pmd) #define pmd_sect(pmd) ((pmd_val(pmd) & PMD_TYPE_MASK) == \ PMD_TYPE_SECT) #define PMD_TYPE_SECT (_AT(pmdval_t, 1) << 0) While pmd_huge() there relies on PMD_TABLE_BIT () int pmd_huge(pmd_t pmd) { return pmd_val(pmd) && !(pmd_val(pmd) & PMD_TABLE_BIT); } #define PMD_TABLE_BIT (_AT(pmdval_t, 1) << 1) These are just the trivial details that I wanted to avoid to touch in this series, so as to resolve the hugetlb issue separately from others. The new pmd_huge_or_thp() is not ideal, but that easily isolates all these trivial details / evils out of the picture, so that we can tackle them one by one. It is strictly an OR or huge||thp, so it's hopefully safe to not break anything yet from that regard. > > I spot checked a couple arches and it looks like it holds up. > > Further, it looks to me like this site in GUP is the only core code > caller.. > > So, I'd suggest a small series to go arch by arch and convert the arch > to use pmd_huge() == pmd_leaf(). Then retire pmd_huge() as a public > API. > > > diff --git a/mm/gup.c b/mm/gup.c > > index df83182ec72d..eebae70d2465 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -3004,8 +3004,7 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, > > unsigned long addr, unsigned lo > > if (!pmd_present(pmd)) > > return 0; > > > > - if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) || > > -pmd_devmap(pmd))) { > > + if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) { > > /* See gup_pte_range() */ > > if (pmd_protnone(pmd)) > > return 0; > > And the devmap thing here doesn't make any sense either. The arch > should ensure that pmd_devmap() implies pmd_leaf(). Since devmap is a > purely SW construct it almost certainly does already anyhow. Yep, but only if pmd_leaf() is safe to be put here. A pmd devmap should always imply as a pmd_leaf() indeed. Thanks, -- Peter Xu