Re: Bug: Write fault blocked by KUAP! (kernel 6.2-rc6, Talos II)
Le 03/02/2023 à 03:45, Nicholas Piggin a écrit :
> On Fri Feb 3, 2023 at 12:02 PM AEST, Benjamin Gray wrote:
>> On Fri, 2023-02-03 at 00:46 +0100, Erhard F. wrote:
>>> Happened during boot:
>>>
>>> [...]
>>> Creating 6 MTD partitions on "flash@0":
>>> 0x-0x0400 : "PNOR"
>>> 0x01b21000-0x03921000 : "BOOTKERNEL"
>>> 0x03a44000-0x03a68000 : "CAPP"
>>> 0x03a88000-0x03a89000 : "VERSION"
>>> 0x03a89000-0x03ac9000 : "IMA_CATALOG"
>>> 0x03e1-0x0400 : "BOOTKERNFW"
>>> BTRFS info: devid 1 device path /dev/root changed to /dev/nvme0n1p3
>>> scanned by systemd-udevd (387)
>>> Kernel attempted to write user page (aa55c28) - exploit attempt?
>>> (uid: 0)
>>> [ cut here ]
>>> Bug: Write fault blocked by KUAP!
>
> KUAP is a red herring of course, the KUAP test just checks if the
> faulting address is below TASK_SIZE.
>
> [snip]
>
>>> --- interrupt: 300 at __patch_instruction+0x50/0x70
>>> NIP: c0064670 LR: c0064c2c CTR: c0048ee0
>>> REGS: c00023b57630 TRAP: 0300 Tainted: G T
>>> (6.2.0-rc6-P9)
>>> MSR: 9280b032 CR:
>>> 2444 XER:
>>> CFAR: c006462c DAR: 0aa55c28 DSISR: 4200 IRQMASK:
> ^^
> First byte of page, store, no PTE.
>
>>> 1
>>> GPR00: c00023b578d0 c0e7cc00
>>> c0080ce33ffc
>>> GPR04: 041ae130 0aa55c27fffc
>
> Last word of previous page.
>
> Probably from create_stub function descriptor patching, which is not
> actually patching in an instruction so it probably gets unlucky and
> gets some data that matches prefix opcode and so it tries to store
> 8 bytes.
An easy fix would probably be to also check the suffix as a prefixed
instruction with 0 as suffix is not valid :
diff --git a/arch/powerpc/include/asm/inst.h
b/arch/powerpc/include/asm/inst.h
index 684d3f453282..87084a52598b 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -86,7 +86,7 @@ static inline ppc_inst_t ppc_inst_read(const u32 *ptr)
static inline bool ppc_inst_prefixed(ppc_inst_t x)
{
- return IS_ENABLED(CONFIG_PPC64) && ppc_inst_primary_opcode(x) ==
OP_PREFIX;
+ return IS_ENABLED(CONFIG_PPC64) && ppc_inst_primary_opcode(x) ==
OP_PREFIX && ppc_inst_suffix(x);
}
static inline ppc_inst_t ppc_inst_swab(ppc_inst_t x)
>
> So not your bug, your temp mm code just exposed it. Data shouldn't
> be patched using patch_instruction. We should have a patch_data_u32
> or similar that doesn't use instructions.
There is work in progress on this but if the above check works, it is
probably a better approach as a fix that can be backported.
Re: [PATCH 1/3] powerpc/code-patching: Add generic memory patching
Le 07/02/2023 à 02:56, Benjamin Gray a écrit :
> patch_instruction() is designed for patching instructions in otherwise
> readonly memory. Other consumers also sometimes need to patch readonly
> memory, so have abused patch_instruction() for arbitrary data patches.
>
> This is a problem on ppc64 as patch_instruction() decides on the patch
> width using the 'instruction' opcode to see if it's a prefixed
> instruction. Data that triggers this can lead to larger writes, possibly
> crossing a page boundary and failing the write altogether.
>
> Introduce patch_uint(), and patch_ulong(), with aliases patch_u32(), and
> patch_u64() (on ppc64) designed for aligned data patches. The patch
> size is now determined by the called function, and is passed as an
> additional parameter to generic internals.
>
> While the instruction flushing is not required for data patches, the
> use cases for data patching (mainly module loading and static calls)
> are less performance sensitive than for instruction patching
> (ftrace activation). So the instruction flushing remains unconditional
> in this patch.
>
> ppc32 does not support prefixed instructions, so is unaffected by the
> original issue. Care is taken in not exposing the size parameter in the
> public (non-static) interface, so the compiler can const-propagate it
> away.
>
> Signed-off-by: Benjamin Gray
>
> ---
>
> GCC 12.2.1 compiled kernels show the following structure:
>
> - ppc64le_defconfig (ppc64, strict RWX): patch_{uint,ulong,instruction}()
>are small wrappers that tail call into patch_memory()
>
> - ppc_defconfig (ppc32, strict RWX): patch_uint() is the only full RWX
>aware implementation. All use of patch size is eliminated.
>
>Note: patch_instruction() is marked noinline to prevent making
>patch_instruction() a wrapper of patch_memory(); GCC likes to tail
>call patch_branch() -> patch_memory(), skipping patch_instruction()
>and preventing patch_memory() from inlining inside patch_instruction().
>
> - pmac32_defconfig (ppc32, no strict RWX): patch_uint() and
>raw_patch_instruction() are both implemented but have the same
>~40 byte function body (as is the case already mind).
> ---
> arch/powerpc/include/asm/code-patching.h | 33
> arch/powerpc/lib/code-patching.c | 69 +---
> 2 files changed, 84 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h
> b/arch/powerpc/include/asm/code-patching.h
> index 3f881548fb61..89fc4c3f6ecf 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -75,6 +75,39 @@ int patch_branch(u32 *addr, unsigned long target, int
> flags);
> int patch_instruction(u32 *addr, ppc_inst_t instr);
> int raw_patch_instruction(u32 *addr, ppc_inst_t instr);
>
> +/*
> + * patch_uint() and patch_ulong() should only be called on addresses where
> the
> + * patch does not cross a cacheline, otherwise it may not be flushed properly
> + * and mixes of new and stale data may be observed. It cannot cross a page
> + * boundary, as only the target page is mapped as writable.
> + *
> + * patch_instruction() and other instruction patchers automatically satisfy
> this
> + * requirement due to instruction alignment requirements.
> + */
> +
> +#ifdef CONFIG_PPC64
> +
> +int patch_uint(void *addr, unsigned int val);
> +#define patch_u32 patch_uint
> +
> +int patch_ulong(void *addr, unsigned long val);
> +#define patch_u64 patch_ulong
> +
> +#else
> +
> +static inline int patch_uint(u32 *addr, unsigned int val)
> +{
> + return patch_instruction(addr, ppc_inst(val));
Would it make more sense that patch_instruction() calls patch_uint()
instead of the reverse ?
> +}
> +#define patch_u32 patch_uint
You can put this outside the ifdef instead of duplicating it.
> +
> +static inline int patch_ulong(void *addr, unsigned long val)
> +{
> + return patch_instruction(addr, ppc_inst(val));
> +}
> +
> +#endif
> +
> static inline unsigned long patch_site_addr(s32 *site)
> {
> return (unsigned long)site + *site;
> diff --git a/arch/powerpc/lib/code-patching.c
> b/arch/powerpc/lib/code-patching.c
> index b00112d7ad46..0f7e9949faf0 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -20,16 +20,14 @@
> #include
> #include
>
> -static int __patch_instruction(u32 *exec_addr, ppc_inst_t instr, u32
> *patch_addr)
> +static int __patch_memory(void *exec_addr, unsigned long val, void
> *patch_addr,
> + bool is_dword)
> {
> - if (!ppc_inst_prefixed(instr)) {
> - u32 val = ppc_inst_val(instr);
> -
> - __put_kernel_nofault(patch_addr, , u32, failed);
> - } else {
> - u64 val = ppc_inst_as_ulong(instr);
> -
> + if (IS_ENABLED(CONFIG_PPC64) && unlikely(is_dword)) {
> __put_kernel_nofault(patch_addr, , u64, failed);
> + } else {
> +
[PATCH 14/24] Documentation: powerpc: correct spelling
Correct spelling problems for Documentation/powerpc/ as reported by codespell. Signed-off-by: Randy Dunlap Cc: Michael Ellerman Cc: linuxppc-dev@lists.ozlabs.org Cc: Jonathan Corbet Cc: linux-...@vger.kernel.org --- Documentation/powerpc/kasan.txt |2 +- Documentation/powerpc/papr_hcalls.rst |2 +- Documentation/powerpc/qe_firmware.rst |4 ++-- Documentation/powerpc/vas-api.rst |4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff -- a/Documentation/powerpc/kasan.txt b/Documentation/powerpc/kasan.txt --- a/Documentation/powerpc/kasan.txt +++ b/Documentation/powerpc/kasan.txt @@ -40,7 +40,7 @@ checks can be delayed until after the MM instrument any code that runs with translations off after booting. This is the current approach. -To avoid this limitiation, the KASAN shadow would have to be placed inside the +To avoid this limitation, the KASAN shadow would have to be placed inside the linear mapping, using the same high-bits trick we use for the rest of the linear mapping. This is tricky: diff -- a/Documentation/powerpc/papr_hcalls.rst b/Documentation/powerpc/papr_hcalls.rst --- a/Documentation/powerpc/papr_hcalls.rst +++ b/Documentation/powerpc/papr_hcalls.rst @@ -22,7 +22,7 @@ privileged operations. Currently there a On PPC64 arch a guest kernel running on top of a PAPR hypervisor is called a *pSeries guest*. A pseries guest runs in a supervisor mode (HV=0) and must issue hypercalls to the hypervisor whenever it needs to perform an action -that is hypervisor priviledged [3]_ or for other services managed by the +that is hypervisor privileged [3]_ or for other services managed by the hypervisor. Hence a Hypercall (hcall) is essentially a request by the pseries guest diff -- a/Documentation/powerpc/qe_firmware.rst b/Documentation/powerpc/qe_firmware.rst --- a/Documentation/powerpc/qe_firmware.rst +++ b/Documentation/powerpc/qe_firmware.rst @@ -232,11 +232,11 @@ For example, to match the 8323, revision 'extended_modes' is a bitfield that defines special functionality which has an impact on the device drivers. Each bit has its own impact and has special instructions for the driver associated with it. This field is stored in -the QE library and available to any driver that calles qe_get_firmware_info(). +the QE library and available to any driver that calls qe_get_firmware_info(). 'vtraps' is an array of 8 words that contain virtual trap values for each virtual traps. As with 'extended_modes', this field is stored in the QE -library and available to any driver that calles qe_get_firmware_info(). +library and available to any driver that calls qe_get_firmware_info(). 'microcode' (type: struct qe_microcode): For each RISC processor there is one 'microcode' structure. The first diff -- a/Documentation/powerpc/vas-api.rst b/Documentation/powerpc/vas-api.rst --- a/Documentation/powerpc/vas-api.rst +++ b/Documentation/powerpc/vas-api.rst @@ -46,7 +46,7 @@ request queue into the application's vir The application can then submit one or more requests to the engine by using copy/paste instructions and pasting the CRBs to the virtual address (aka paste_address) returned by mmap(). User space can close the -established connection or send window by closing the file descriptior +established connection or send window by closing the file descriptor (close(fd)) or upon the process exit. Note that applications can send several requests with the same window or @@ -240,7 +240,7 @@ issued. This signal returns with the fol siginfo.si_signo = SIGSEGV; siginfo.si_errno = EFAULT; siginfo.si_code = SEGV_MAPERR; - siginfo.si_addr = CSB adress; + siginfo.si_addr = CSB address; In the case of multi-thread applications, NX send windows can be shared across all threads. For example, a child thread can open a send window,
[powerpc:fixes-test] BUILD SUCCESS 2ea31e2e62bbc4d11c411eeb36f1b02841dbcab1
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 2ea31e2e62bbc4d11c411eeb36f1b02841dbcab1 powerpc/64s/interrupt: Fix interrupt exit race with security mitigation switch elapsed time: 724m configs tested: 2 configs skipped: 119 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: powerpc allnoconfig powerpc allmodconfig -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
[powerpc:next-test] BUILD SUCCESS 18e3525c5e7c6634f88857242a2e1997d5f065e8
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test branch HEAD: 18e3525c5e7c6634f88857242a2e1997d5f065e8 integrity/powerpc: Support loading keys from PLPKS elapsed time: 722m configs tested: 68 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: um x86_64_defconfig um i386_defconfig x86_64allnoconfig arc defconfig powerpc allnoconfig alpha defconfig x86_64 rhel-8.3-bpf m68k allyesconfig x86_64 rhel-8.3-syz x86_64 rhel-8.3-kunit m68k allmodconfig x86_64 rhel-8.3-kvm arc allyesconfig alphaallyesconfig x86_64 defconfig arm defconfig x86_64 rhel-8.3 s390 allmodconfig i386 randconfig-a011-20230206 i386 randconfig-a014-20230206 i386 randconfig-a012-20230206 s390defconfig i386 randconfig-a013-20230206 x86_64 allyesconfig x86_64 randconfig-a014-20230206 x86_64 randconfig-a013-20230206 i386 randconfig-a016-20230206 i386 randconfig-a015-20230206 arm allyesconfig sh allmodconfig i386defconfig s390 allyesconfig ia64 allmodconfig x86_64 randconfig-a011-20230206 arm64allyesconfig x86_64 randconfig-a015-20230206 x86_64 randconfig-a012-20230206 mips allyesconfig x86_64 randconfig-a016-20230206 powerpc allmodconfig x86_64rhel-8.3-kselftests x86_64 rhel-8.3-func riscvrandconfig-r042-20230204 arc randconfig-r043-20230204 arc randconfig-r043-20230206 s390 randconfig-r044-20230204 riscvrandconfig-r042-20230206 s390 randconfig-r044-20230206 i386 allyesconfig clang tested configs: i386 randconfig-a002-20230206 i386 randconfig-a004-20230206 i386 randconfig-a003-20230206 x86_64 randconfig-a001-20230206 i386 randconfig-a001-20230206 x86_64 randconfig-a002-20230206 x86_64 randconfig-a003-20230206 x86_64 randconfig-a004-20230206 i386 randconfig-a005-20230206 i386 randconfig-a006-20230206 x86_64 rhel-8.3-rust hexagon randconfig-r045-20230206 hexagon randconfig-r041-20230206 hexagon randconfig-r041-20230204 arm randconfig-r046-20230204 arm randconfig-r046-20230206 hexagon randconfig-r045-20230204 x86_64 randconfig-a005-20230206 x86_64 randconfig-a006-20230206 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
[powerpc:next] BUILD SUCCESS b505063910c134778202dfad9332dfcecb76bab3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: b505063910c134778202dfad9332dfcecb76bab3 powerpc/iommu: fix memory leak with using debugfs_lookup() elapsed time: 723m configs tested: 2 configs skipped: 119 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: powerpc allnoconfig powerpc allmodconfig -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
Re: [PATCH v2 0/8] powerpc/85xx: p2020: Create one unified machine description
On Monday 23 January 2023 21:09:22 Pali Rohár wrote:
> On Monday 23 January 2023 14:32:36 Christophe Leroy wrote:
> > Le 22/01/2023 à 12:16, Pali Rohár a écrit :
> > > Hello! Do you have any comments for this patch series?
> >
> >
> > I think patches 1 and 2 could be a single patch.
>
> Well, if you want to have them in single patch, it could be easily
> squashed during applying. I thought that it is better to have them
> separated because of different boards, files, etc...:
> https://lore.kernel.org/linuxppc-dev/5bf1f2fc-a1de-d873-7d1b-0058ff8b9...@csgroup.eu/
>
> > I'm having hard time understanding how things are built. Patch 3
> > introduces 273 lines of new code in a file named p2020.c while only
> > removing 23 lines and 44 lines from mpc85xx_{ds/rdb}.c.
>
> In v1 I generated that patch with git -M, -C and other options which
> detects copy and renames. But I had an impression that it is less readable:
> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-4-p...@kernel.org/
>
> So I tried to describe all changes in commit message and generated that
> patch without copy options (so it is plain patch with add lines).
>
> This patch moves all p2020 boards from mpc85xx_rdb.c and mpc85xx_ds.c
> files into new p2020.c file, and plus it copies all helper functions
> which p2020 boards requires. This patch does not introduce any new code
> or functional change. It should be really plain copy/move.
I sent same patch but generated by git -M and -C options. See if it is
better.
> > Then patches 4,
> > 5 and 6 exclusively modify p2020.c which was a completely new file added
> > by patch 3.
>
> In later patches is then that moved/copied code improved and cleaned.
>
> > Why not making it correct from the beginning, that is merge
> > patches 4, 5 and 6 in patch 3 ?
>
> I wanted to separate logical changes into separate commits. So first
> just moves/copy code (which should be noop) and then do functional
> changes in followup patches. I like this progress because for me it is
> easier for reviewing. Important parts are functional changes, which are
> in separated commits and it is visually separated from boring move/copy
> code changes.
>
> > Or maybe p2020.c is not really new but is a copy of some previously
> > existing code ? In that case it would be better to make it explicit, for
> > history.
>
> Yes. Do you have any suggestion how to make it _more_ explicit? I tried
> to explain it in commit message (but I'm not sure if it is enough). And
> when viewing via git show, it is needed to call it with additional -M
> and -C options to see this. git does not do it automatically.
Do you have any other suggestions what should I do?
> >
> > Christophe
> >
> >
> > >
> > > On Saturday 24 December 2022 22:14:17 Pali Rohár wrote:
> > >> This patch series unifies all P2020 boards and machine descriptions into
> > >> one generic unified P2020 machine description. With this generic machine
> > >> description, kernel can boot on any P2020-based board with correct DTS
> > >> file.
> > >>
> > >> Tested on CZ.NIC Turris 1.1 board with has Freescale P2020 processor.
> > >> Kernel during booting correctly detects P2020 and prints:
> > >> [0.00] Using Freescale P2020 machine description
> > >>
> > >> Changes in v2:
> > >> * Added patch "p2020: Move i8259 code into own function" (separated from
> > >> the next one)
> > >> * Renamed CONFIG_P2020 to CONFIG_PPC_P2020
> > >> * Fixed descriptions
> > >>
> > >> Link to v1:
> > >> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-p...@kernel.org/
> > >>
> > >> Pali Rohár (8):
> > >>powerpc/85xx: Mark mpc85xx_rdb_pic_init() as static
> > >>powerpc/85xx: Mark mpc85xx_ds_pic_init() as static
> > >>powerpc/85xx: p2020: Move all P2020 machine descriptions to p2020.c
> > >>powerpc/85xx: p2020: Move i8259 code into own function
> > >>powerpc/85xx: p2020: Unify .setup_arch and .init_IRQ callbacks
> > >>powerpc/85xx: p2020: Define just one machine description
> > >>powerpc/85xx: p2020: Enable boards by new config option
> > >> CONFIG_PPC_P2020
> > >>powerpc: dts: turris1x.dts: Remove "fsl,P2020RDB-PC" compatible string
> > >>
> > >> arch/powerpc/boot/dts/turris1x.dts| 2 +-
> > >> arch/powerpc/platforms/85xx/Kconfig | 22 ++-
> > >> arch/powerpc/platforms/85xx/Makefile | 1 +
> > >> arch/powerpc/platforms/85xx/mpc85xx_ds.c | 25 +--
> > >> arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 46 +-
> > >> arch/powerpc/platforms/85xx/p2020.c | 193 ++
> > >> 6 files changed, 215 insertions(+), 74 deletions(-)
> > >> create mode 100644 arch/powerpc/platforms/85xx/p2020.c
> > >>
> > >> --
> > >> 2.20.1
> > >>
Re: [PATCH v4 3/3] powerpc: dts: turris1x.dts: Set lower priority for CPLD syscon-reboot
On Monday 26 December 2022 12:45:13 Pali Rohár wrote:
> Due to CPLD firmware bugs, set CPLD syscon-reboot priority level to 64
> (between rstcr and watchdog) to ensure that rstcr's global-utilities reset
> method which is preferred stay as default one, and to ensure that CPLD
> syscon-reboot is more preferred than watchdog reset method.
>
> Fixes: 0531a4abd1c6 ("powerpc: dts: turris1x.dts: Add CPLD reboot node")
> Signed-off-by: Pali Rohár
May I ask who can take this 3/3 patch? powersupply or powerpc tree?
> ---
> arch/powerpc/boot/dts/turris1x.dts | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/turris1x.dts
> b/arch/powerpc/boot/dts/turris1x.dts
> index e9cda34a140e..c9b619f6ed5c 100644
> --- a/arch/powerpc/boot/dts/turris1x.dts
> +++ b/arch/powerpc/boot/dts/turris1x.dts
> @@ -367,11 +367,34 @@
> };
>
> reboot@d {
> + /*
> + * CPLD firmware which manages system reset and
> + * watchdog registers has bugs. It does not
> + * autoclear system reset register after change
> + * and watchdog ignores reset line on immediate
> + * succeeding reset cycle triggered by watchdog.
> + * These bugs have to be workarounded in U-Boot
> + * bootloader. So use system reset via syscon as
> + * a last resort because older U-Boot versions
> + * do not have workaround for watchdog.
> + *
> + * Reset method via rstcr's global-utilities
> + * (the preferred one) has priority level 128,
> + * watchdog has priority level 0 and default
> + * syscon-reboot priority level is 192.
> + *
> + * So define syscon-reboot with custom priority
> + * level 64 (between rstcr and watchdog) because
> + * rstcr should stay as default preferred reset
> + * method and reset via watchdog is more broken
> + * than system reset via syscon.
> + */
> compatible = "syscon-reboot";
> reg = <0x0d 0x01>;
> offset = <0x0d>;
> mask = <0x01>;
> value = <0x01>;
> + priority = <64>;
> };
>
> led-controller@13 {
> --
> 2.20.1
>
Re: [PATCH v4 1/7] kcsan: Add atomic builtin stubs for 32-bit systems
> On 8 Feb 2023, at 11:23 pm, Christophe Leroy
> wrote:
>
>
>
> Le 08/02/2023 à 04:21, Rohan McLure a écrit :
>> KCSAN instruments calls to atomic builtins, and will in turn call these
>> builtins itself. As such, architectures supporting KCSAN must have
>> compiler support for these atomic primitives.
>>
>> Since 32-bit systems are unlikely to have 64-bit compiler builtins,
>> provide a stub for each missing builtin, and use BUG() to assert
>> unreachability.
>>
>> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
>> locally, but does not advertise the fact with preprocessor macros. To
>> avoid production of duplicate symbols, do not build the stubs on xtensa.
>> A future patch will remove the xtensa implementation of these stubs.
>>
>> Signed-off-by: Rohan McLure
>> ---
>> v4: New patch
>> ---
>> kernel/kcsan/Makefile | 3 ++
>> kernel/kcsan/stubs.c | 78 +++
>> 2 files changed, 81 insertions(+)
>> create mode 100644 kernel/kcsan/stubs.c
>
> I think it would be better to merge patch 1 and patch 2, that way we
> would keep the history and see that stubs.c almost comes from xtensa.
>
> The summary would then be:
>
> arch/xtensa/lib/Makefile | 1 -
> kernel/kcsan/Makefile | 2 +-
> arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 26
> +-
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
>
>>
>> diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
>> index 8cf70f068d92..5dfc5825aae9 100644
>> --- a/kernel/kcsan/Makefile
>> +++ b/kernel/kcsan/Makefile
>> @@ -12,6 +12,9 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
>> -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>>
>> obj-y := core.o debugfs.o report.o
>> +ifndef XTENSA
>> + obj-y += stubs.o
>
> obj-$(CONFIG_32BIT) += stubs.o
>
> That would avoid the #if CONFIG_32BIT in stubs.c
Thanks. Yes happy to do this.
>
>> +endif
>>
>> KCSAN_INSTRUMENT_BARRIERS_selftest.o := y
>> obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
>> diff --git a/kernel/kcsan/stubs.c b/kernel/kcsan/stubs.c
>> new file mode 100644
>> index ..ec5cd99be422
>> --- /dev/null
>> +++ b/kernel/kcsan/stubs.c
>> @@ -0,0 +1,78 @@
>> +// SPDX-License Identifier: GPL-2.0
>
> Missing - between License and Identifier ?
>
>> +
>> +#include
>> +#include
>> +
>> +#ifdef CONFIG_32BIT
>
> Should be handled in Makefile
>
>> +
>> +#if !__has_builtin(__atomic_store_8)
>
> Does any 32 bit ARCH support that ? Is that #if required ?
>
> If yes, do we really need the #if for each and every function, can't we
> just check for one and assume that if we don't have __atomic_store_8 we
> don't have any of the functions ?
Turns out that testing with gcc provides 8-byte atomic builtins on x86
and arm on 32-bit. However I believe it should just suffice to check for
__atomic_store_8 or any other such builtin i.e. if an arch implements one it
likely implements them all from what I’ve seen.
>
>
>> +void __atomic_store_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_load_8)
>> +u64 __atomic_load_8(const volatile void *p, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_exchange_8)
>> +u64 __atomic_exchange_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_compare_exchange_8)
>> +bool __atomic_compare_exchange_8(volatile void *p1, void *p2, u64 v, bool
>> b, int i1, int i2)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_fetch_add_8)
>> +u64 __atomic_fetch_add_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_fetch_sub_8)
>> +u64 __atomic_fetch_sub_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_fetch_and_8)
>> +u64 __atomic_fetch_and_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_fetch_or_8)
>> +u64 __atomic_fetch_or_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_fetch_xor_8)
>> +u64 __atomic_fetch_xor_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#if !__has_builtin(__atomic_fetch_nand_8)
>> +u64 __atomic_fetch_nand_8(volatile void *p, u64 v, int i)
>> +{
>> + BUG();
>> +}
>> +#endif
>> +
>> +#endif /* CONFIG_32BIT */
Re: [PATCH v4 02/18] ARM: s3c24xx: Use the right include
On 08/02/2023 18:33, Andy Shevchenko wrote: > From: Linus Walleij > > The file s3c64xx.c is including despite using no > symbols from the file, however it needs it to implicitly bring in > of_have_populated_dt() so include explicitly instead. > > Signed-off-by: Linus Walleij > Signed-off-by: Andy Shevchenko Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 02/18] ARM: s3c24xx: Use the right include
On Wed, Feb 08, 2023 at 06:39:12PM +0100, Krzysztof Kozlowski wrote: > On 08/02/2023 18:33, Andy Shevchenko wrote: ... > It's not s3c24xx anymore, so subject prefix: > ARM: s3c64xx: Fixed locally, thanks. -- With Best Regards, Andy Shevchenko
Re: [PATCH v4 02/18] ARM: s3c24xx: Use the right include
On 08/02/2023 18:33, Andy Shevchenko wrote: > From: Linus Walleij > > The file s3c64xx.c is including despite using no > symbols from the file, however it needs it to implicitly bring in > of_have_populated_dt() so include explicitly instead. > > Signed-off-by: Linus Walleij > Signed-off-by: Andy Shevchenko > --- > arch/arm/mach-s3c/s3c64xx.c | 2 +- It's not s3c24xx anymore, so subject prefix: ARM: s3c64xx: Best regards, Krzysztof
[PATCH v4 18/18] gpiolib: Clean up headers
There is a few things done:
- include only the headers we are direct user of
- when pointer is in use, provide a forward declaration
- add missing headers
- group generic headers and subsystem headers
- sort each group alphabetically
Signed-off-by: Andy Shevchenko
---
drivers/gpio/gpiolib-acpi.c | 10 ++
drivers/gpio/gpiolib-acpi.h | 1 -
drivers/gpio/gpiolib-of.c | 6 --
drivers/gpio/gpiolib-of.h | 1 -
drivers/gpio/gpiolib-swnode.c | 5 +++--
drivers/gpio/gpiolib-sysfs.c | 21 -
drivers/gpio/gpiolib.c| 9 ++---
include/linux/gpio.h | 10 --
include/linux/gpio/consumer.h | 14 ++
include/linux/gpio/driver.h | 30 +++---
10 files changed, 72 insertions(+), 35 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index bb583cea366c..3871dade186a 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -7,17 +7,19 @@
* Mika Westerberg
*/
+#include
#include
#include
-#include
-#include
-#include
#include
-#include
#include
+#include
#include
#include
+#include
+#include
+#include
+
#include "gpiolib.h"
#include "gpiolib-acpi.h"
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 5fa315b3c912..a6f3be0bb921 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -9,7 +9,6 @@
#define GPIOLIB_ACPI_H
#include
-#include
#include
#include
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 0f699af438b0..1436cdb5fa26 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -10,14 +10,16 @@
#include
#include
#include
-#include
#include
-#include
+#include
#include
#include
#include
#include
#include
+#include
+
+#include
#include
#include "gpiolib.h"
diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h
index e5bb065d82ef..6b3a5347c5d9 100644
--- a/drivers/gpio/gpiolib-of.h
+++ b/drivers/gpio/gpiolib-of.h
@@ -4,7 +4,6 @@
#define GPIOLIB_OF_H
#include
-#include
#include
#include
diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index dd9ccac214d1..b5a6eaf3729b 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -6,13 +6,14 @@
*/
#include
#include
-#include
-#include
#include
#include
#include
#include
+#include
+#include
+
#include "gpiolib.h"
#include "gpiolib-swnode.h"
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 6e4267944f80..c1cbf71329f0 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -1,18 +1,29 @@
// SPDX-License-Identifier: GPL-2.0
+
+#include
+#include
#include
+#include
+#include
+#include
+#include
+#include
#include
-#include
+#include
+#include
+#include
+#include
#include
+#include
+
#include
#include
-#include
-#include
-#include
-#include
#include "gpiolib.h"
#include "gpiolib-sysfs.h"
+struct kernfs_node;
+
#define GPIO_IRQF_TRIGGER_NONE 0
#define GPIO_IRQF_TRIGGER_FALLING BIT(0)
#define GPIO_IRQF_TRIGGER_RISING BIT(1)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 99a2c77c3711..900f6573c070 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -6,22 +6,25 @@
#include
#include
#include
+#include
#include
#include
-#include
-#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
#include
+#include
+#include
+#include
+
#include
#include "gpiolib-acpi.h"
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index a86953696e47..8528353e073b 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -12,9 +12,10 @@
#ifndef __LINUX_GPIO_H
#define __LINUX_GPIO_H
-#include
#include
+struct device;
+
/* see Documentation/driver-api/gpio/legacy.rst */
/* make these flag values available regardless of GPIO kconfig options */
@@ -134,19 +135,16 @@ void gpio_free_array(const struct gpio *array, size_t
num);
/* CONFIG_GPIOLIB: bindings for managed devices that want to request gpios */
-struct device;
-
int devm_gpio_request(struct device *dev, unsigned gpio, const char *label);
int devm_gpio_request_one(struct device *dev, unsigned gpio,
unsigned long flags, const char *label);
#else /* ! CONFIG_GPIOLIB */
-#include
#include
-struct device;
-struct gpio_chip;
+#include
+#include
static inline bool gpio_is_valid(int number)
{
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 5432e5d5fbfb..1c4385a00f88 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -3,15 +3,14 @@
#define __LINUX_GPIO_CONSUMER_H
#include
-#include
-#include
-#include
+#include
struct acpi_device;
struct device;
[PATCH v4 13/18] gpio: reg: Add missing header(s)
Do not imply that some of the generic headers may be always included.
Instead, include explicitly what we are direct user of.
While at it, split out the GPIO group of headers.
Signed-off-by: Andy Shevchenko
---
drivers/gpio/gpio-reg.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-reg.c b/drivers/gpio/gpio-reg.c
index d35169bde25a..73c7260d89c0 100644
--- a/drivers/gpio/gpio-reg.c
+++ b/drivers/gpio/gpio-reg.c
@@ -4,11 +4,19 @@
*
* Copyright (C) 2016 Russell King
*/
-#include
-#include
+#include
+#include
+#include
+#include
+#include
#include
+#include
#include
#include
+#include
+
+#include
+#include
struct gpio_reg {
struct gpio_chip gc;
--
2.39.1
[PATCH v4 15/18] gpiolib: Drop unused forward declaration from driver.h
There is no struct device_node pointers anywhere in the header, drop unused forward declaration. Signed-off-by: Andy Shevchenko --- include/linux/gpio/driver.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index ccd8a512d854..262a84ce9bcb 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -16,7 +16,6 @@ struct gpio_desc; struct of_phandle_args; -struct device_node; struct seq_file; struct gpio_device; struct module; -- 2.39.1
[PATCH v4 12/18] gpio: aggregator: Add missing header(s)
Do not imply that some of the generic headers may be always included. Instead, include explicitly what we are direct user of. While at it, drop unused linux/gpio.h and split out the GPIO group of headers. Signed-off-by: Andy Shevchenko Reviewed-by: Geert Uytterhoeven --- drivers/gpio/gpio-aggregator.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c index 6d17d262ad91..20a686f12df7 100644 --- a/drivers/gpio/gpio-aggregator.c +++ b/drivers/gpio/gpio-aggregator.c @@ -10,19 +10,20 @@ #include #include #include -#include -#include -#include -#include #include #include #include #include #include #include +#include #include #include +#include +#include +#include + #define AGGREGATOR_MAX_GPIOS 512 /* -- 2.39.1
[PATCH v4 10/18] gpiolib: split linux/gpio/driver.h out of linux/gpio.h
From: Arnd Bergmann Almost all gpio drivers include linux/gpio/driver.h, and other files should not rely on includes from this header. Remove the indirect include from here and include the correct headers directly from where they are used. Reviewed-by: Andy Shevchenko Signed-off-by: Arnd Bergmann Reviewed-by: Linus Walleij Signed-off-by: Andy Shevchenko Acked-by: Lee Jones --- arch/arm/mach-omap1/irq.c | 1 + arch/arm/mach-orion5x/board-rd88f5182.c| 1 + arch/arm/mach-sa1100/assabet.c | 1 + drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c | 1 + include/linux/mfd/ucb1x00.h| 1 + 5 files changed, 5 insertions(+) diff --git a/arch/arm/mach-omap1/irq.c b/arch/arm/mach-omap1/irq.c index 9ccc784fd614..bfc7ab010ae2 100644 --- a/arch/arm/mach-omap1/irq.c +++ b/arch/arm/mach-omap1/irq.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include diff --git a/arch/arm/mach-orion5x/board-rd88f5182.c b/arch/arm/mach-orion5x/board-rd88f5182.c index 596601367989..1c14e49a90a6 100644 --- a/arch/arm/mach-orion5x/board-rd88f5182.c +++ b/arch/arm/mach-orion5x/board-rd88f5182.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c index 2eba112f2ad8..d000c678b439 100644 --- a/arch/arm/mach-sa1100/assabet.c +++ b/arch/arm/mach-sa1100/assabet.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c index 9540a05247c2..89c8829528c2 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/led.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include #include +#include #include #include #include diff --git a/include/linux/mfd/ucb1x00.h b/include/linux/mfd/ucb1x00.h index 43bcf35afe27..ede237384723 100644 --- a/include/linux/mfd/ucb1x00.h +++ b/include/linux/mfd/ucb1x00.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #define UCB_IO_DATA0x00 -- 2.39.1
[PATCH v4 08/18] gpiolib: remove gpio_set_debounce()
From: Arnd Bergmann
gpio_set_debounce() only has a single user, which is trivially
converted to gpiod_set_debounce().
Signed-off-by: Arnd Bergmann
Reviewed-by: Linus Walleij
Reviewed-by: Andy Shevchenko
Signed-off-by: Andy Shevchenko
---
Documentation/driver-api/gpio/legacy.rst | 2 --
.../translations/zh_CN/driver-api/gpio/legacy.rst | 1 -
Documentation/translations/zh_TW/gpio.txt | 1 -
drivers/input/touchscreen/ads7846.c| 5 +++--
include/linux/gpio.h | 10 --
5 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/Documentation/driver-api/gpio/legacy.rst
b/Documentation/driver-api/gpio/legacy.rst
index a0559d93efd1..e0306e78e34b 100644
--- a/Documentation/driver-api/gpio/legacy.rst
+++ b/Documentation/driver-api/gpio/legacy.rst
@@ -238,8 +238,6 @@ setup or driver probe/teardown code, so this is an easy
constraint.)::
## gpio_free_array()
gpio_free()
-gpio_set_debounce()
-
Claiming and Releasing GPIOs
diff --git a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst
b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst
index 74fa473bb504..dee2a0517c1c 100644
--- a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst
+++ b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst
@@ -219,7 +219,6 @@ GPIO 值的命令需要等待其信息排到队首才发送命令，再获得其
## gpio_free_array()
gpio_free()
-gpio_set_debounce()
diff --git a/Documentation/translations/zh_TW/gpio.txt
b/Documentation/translations/zh_TW/gpio.txt
index 1b986bbb0909..dc608358d90a 100644
--- a/Documentation/translations/zh_TW/gpio.txt
+++ b/Documentation/translations/zh_TW/gpio.txt
@@ -226,7 +226,6 @@ GPIO 值的命令需要等待其信息排到隊首才發送命令，再獲得其
## gpio_free_array()
gpio_free()
- gpio_set_debounce()
diff --git a/drivers/input/touchscreen/ads7846.c
b/drivers/input/touchscreen/ads7846.c
index 17f11bce8113..bb1058b1e7fd 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1012,8 +1013,8 @@ static int ads7846_setup_pendown(struct spi_device *spi,
ts->gpio_pendown = pdata->gpio_pendown;
if (pdata->gpio_pendown_debounce)
- gpio_set_debounce(pdata->gpio_pendown,
- pdata->gpio_pendown_debounce);
+ gpiod_set_debounce(gpio_to_desc(ts->gpio_pendown),
+ pdata->gpio_pendown_debounce);
} else {
dev_err(>dev, "no get_pendown_state nor gpio_pendown?\n");
return -EINVAL;
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index d5ce78e2bdd9..fc56733e8514 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -100,11 +100,6 @@ static inline int gpio_direction_output(unsigned gpio, int
value)
return gpiod_direction_output_raw(gpio_to_desc(gpio), value);
}
-static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
-{
- return gpiod_set_debounce(gpio_to_desc(gpio), debounce);
-}
-
static inline int gpio_get_value_cansleep(unsigned gpio)
{
return gpiod_get_raw_value_cansleep(gpio_to_desc(gpio));
@@ -214,11 +209,6 @@ static inline int gpio_direction_output(unsigned gpio, int
value)
return -ENOSYS;
}
-static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
-{
- return -ENOSYS;
-}
-
static inline int gpio_get_value(unsigned gpio)
{
/* GPIO can never have been requested or set as {in,out}put */
--
2.39.1
[PATCH v4 17/18] gpiolib: Group forward declarations in consumer.h
For better maintenance group the forward declarations together.
Signed-off-by: Andy Shevchenko
---
include/linux/gpio/consumer.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index a7eb8aa1e54c..5432e5d5fbfb 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -7,6 +7,7 @@
#include
#include
+struct acpi_device;
struct device;
struct fwnode_handle;
struct gpio_desc;
@@ -602,8 +603,6 @@ struct acpi_gpio_mapping {
unsigned int quirks;
};
-struct acpi_device;
-
#if IS_ENABLED(CONFIG_GPIOLIB) && IS_ENABLED(CONFIG_ACPI)
int acpi_dev_add_driver_gpios(struct acpi_device *adev,
--
2.39.1
[PATCH v4 05/18] gpiolib: remove empty asm/gpio.h files
From: Arnd Bergmann
The arm and sh versions of this file are identical to the generic
versions and can just be removed.
The drivers that actually use the sh3 specific version also include
cpu/gpio.h directly, with the exception of magicpanelr2, which is
easily fixed. This leaves coldfire as the only gpio driver
that needs something custom for gpiolib.
Reviewed-by: Geert Uytterhoeven
Reviewed-by: Andy Shevchenko
Acked-by: Bartosz Golaszewski
Signed-off-by: Arnd Bergmann
Reviewed-by: Linus Walleij
Signed-off-by: Andy Shevchenko
Reviewed-by: Vincenzo Palazzo
---
arch/arm/Kconfig| 1 -
arch/arm/include/asm/gpio.h | 21 --
arch/sh/Kconfig | 1 -
arch/sh/boards/board-magicpanelr2.c | 1 +
arch/sh/include/asm/gpio.h | 45 -
5 files changed, 1 insertion(+), 68 deletions(-)
delete mode 100644 arch/arm/include/asm/gpio.h
delete mode 100644 arch/sh/include/asm/gpio.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e24a9820e12f..1d1a603d964d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -24,7 +24,6 @@ config ARM
select ARCH_HAS_SYNC_DMA_FOR_CPU
select ARCH_HAS_TEARDOWN_DMA_OPS if MMU
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
- select ARCH_HAVE_CUSTOM_GPIO_H
select ARCH_HAVE_NMI_SAFE_CMPXCHG if CPU_V7 || CPU_V7M || CPU_V6K
select ARCH_HAS_GCOV_PROFILE_ALL
select ARCH_KEEP_MEMBLOCK
diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
deleted file mode 100644
index 4ebbb58f06ea..
--- a/arch/arm/include/asm/gpio.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ARCH_ARM_GPIO_H
-#define _ARCH_ARM_GPIO_H
-
-#include
-
-/* The trivial gpiolib dispatchers */
-#define gpio_get_value __gpio_get_value
-#define gpio_set_value __gpio_set_value
-#define gpio_cansleep __gpio_cansleep
-
-/*
- * Provide a default gpio_to_irq() which should satisfy every case.
- * However, some platforms want to do this differently, so allow them
- * to override it.
- */
-#ifndef gpio_to_irq
-#define gpio_to_irq__gpio_to_irq
-#endif
-
-#endif /* _ARCH_ARM_GPIO_H */
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 0665ac0add0b..ccb866750a88 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -4,7 +4,6 @@ config SUPERH
select ARCH_32BIT_OFF_T
select ARCH_ENABLE_MEMORY_HOTPLUG if SPARSEMEM && MMU
select ARCH_ENABLE_MEMORY_HOTREMOVE if SPARSEMEM && MMU
- select ARCH_HAVE_CUSTOM_GPIO_H
select ARCH_HAVE_NMI_SAFE_CMPXCHG if (GUSA_RB || CPU_SH4A)
select ARCH_HAS_BINFMT_FLAT if !MMU
select ARCH_HAS_CURRENT_STACK_POINTER
diff --git a/arch/sh/boards/board-magicpanelr2.c
b/arch/sh/boards/board-magicpanelr2.c
index 56bd386ff3b0..75de893152af 100644
--- a/arch/sh/boards/board-magicpanelr2.c
+++ b/arch/sh/boards/board-magicpanelr2.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
/* Dummy supplies, where voltage doesn't matter */
diff --git a/arch/sh/include/asm/gpio.h b/arch/sh/include/asm/gpio.h
deleted file mode 100644
index 588c1380e4cb..
--- a/arch/sh/include/asm/gpio.h
+++ /dev/null
@@ -1,45 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0
- *
- * include/asm-sh/gpio.h
- *
- * Generic GPIO API and pinmux table support for SuperH.
- *
- * Copyright (c) 2008 Magnus Damm
- */
-#ifndef __ASM_SH_GPIO_H
-#define __ASM_SH_GPIO_H
-
-#include
-#include
-
-#if defined(CONFIG_CPU_SH3)
-#include
-#endif
-
-#include
-
-#ifdef CONFIG_GPIOLIB
-
-static inline int gpio_get_value(unsigned gpio)
-{
- return __gpio_get_value(gpio);
-}
-
-static inline void gpio_set_value(unsigned gpio, int value)
-{
- __gpio_set_value(gpio, value);
-}
-
-static inline int gpio_cansleep(unsigned gpio)
-{
- return __gpio_cansleep(gpio);
-}
-
-static inline int gpio_to_irq(unsigned gpio)
-{
- return __gpio_to_irq(gpio);
-}
-
-#endif /* CONFIG_GPIOLIB */
-
-#endif /* __ASM_SH_GPIO_H */
--
2.39.1
[PATCH v4 16/18] gpiolib: Deduplicate forward declarations in consumer.h
The struct fwnode_handle pointer is used in both branches of ifdeffery, no need to have a copy of the same in each of them, just make it global. Signed-off-by: Andy Shevchenko --- include/linux/gpio/consumer.h | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 59cb20cfac3d..a7eb8aa1e54c 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -8,6 +8,7 @@ #include struct device; +struct fwnode_handle; struct gpio_desc; struct gpio_array; @@ -171,9 +172,6 @@ int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name); struct gpio_desc *gpio_to_desc(unsigned gpio); int desc_to_gpio(const struct gpio_desc *desc); -/* Child properties interface */ -struct fwnode_handle; - struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode, const char *con_id, int index, enum gpiod_flags flags, @@ -546,9 +544,6 @@ static inline int desc_to_gpio(const struct gpio_desc *desc) return -EINVAL; } -/* Child properties interface */ -struct fwnode_handle; - static inline struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode, const char *con_id, int index, -- 2.39.1
[PATCH v4 03/18] hte: tegra-194: Use proper includes
From: Linus Walleij The test driver uses the gpiod consumer API so include the right header. This may cause a problem with struct of_device_id being implcitly pulled in by the legacy header so include explicitly as well. While at it, drop explicit moduleparam.h (it's included with module.h) and sort the headers. Signed-off-by: Linus Walleij Signed-off-by: Andy Shevchenko --- drivers/hte/hte-tegra194-test.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/hte/hte-tegra194-test.c b/drivers/hte/hte-tegra194-test.c index 5d776a185bd6..358d4a10c6a1 100644 --- a/drivers/hte/hte-tegra194-test.c +++ b/drivers/hte/hte-tegra194-test.c @@ -6,14 +6,14 @@ */ #include -#include -#include +#include +#include #include -#include -#include +#include +#include #include +#include #include -#include /* * This sample HTE GPIO test driver demonstrates HTE API usage by enabling -- 2.39.1
[PATCH v4 00/18] gpiolib cleanups
These are some older patches Arnd did last year, rebased to linux-next-20230208. On top there are Andy's patches regarding similar topic. The series starts with Linus Walleij's patches. The main goal is to remove some of the legacy bits of the gpiolib interfaces, where the corner cases are easily avoided or replaced with gpio descriptor based interfaces. The idea is to get an immutable branch and route the whole series via GPIO tree. Changes in v4: - incorporated Linus Walleij's patches - reworked touchscreen patch to have bare minimum changes (Dmitry) - described changes in gpio-aggregator in full (Geert) - addressed compilation errors (LKP) - added tags (Geert, Lee, Vincenzo) Changes in v3: - reworked touchscreen patch in accordance with Dmitry's comments - rebased on the latest Linux Next - added on top Andy's series Changes in v2: - dropped patch 8 after Andy's identical patch was merged - rebase on latest gpio tree - leave unused gpio_cansleep() in place for now - address feedback from Andy Shevchenko Andy Shevchenko (7): gpio: aggregator: Add missing header(s) gpio: reg: Add missing header(s) gpio: regmap: Add missing header(s) gpiolib: Drop unused forward declaration from driver.h gpiolib: Deduplicate forward declarations in consumer.h gpiolib: Group forward declarations in consumer.h gpiolib: Clean up headers Arnd Bergmann (7): gpiolib: remove empty asm/gpio.h files gpiolib: coldfire: remove custom asm/gpio.h gpiolib: remove asm-generic/gpio.h gpiolib: remove gpio_set_debounce() gpiolib: remove legacy gpio_export() gpiolib: split linux/gpio/driver.h out of linux/gpio.h gpiolib: split of_mm_gpio_chip out of linux/of_gpio.h Linus Walleij (4): ARM: orion/gpio: Use the right include ARM: s3c24xx: Use the right include hte: tegra-194: Use proper includes gpiolib: Make the legacy consumer-only Documentation/admin-guide/gpio/sysfs.rst | 2 +- Documentation/driver-api/gpio/legacy.rst | 23 --- .../zh_CN/driver-api/gpio/legacy.rst | 20 --- Documentation/translations/zh_TW/gpio.txt | 19 --- MAINTAINERS | 1 - arch/arm/Kconfig | 1 - arch/arm/include/asm/gpio.h | 21 --- arch/arm/mach-omap1/irq.c | 1 + arch/arm/mach-omap2/pdata-quirks.c| 9 +- arch/arm/mach-orion5x/board-rd88f5182.c | 1 + arch/arm/mach-s3c/s3c64xx.c | 2 +- arch/arm/mach-sa1100/assabet.c| 1 + arch/arm/plat-orion/gpio.c| 5 +- arch/m68k/Kconfig.cpu | 1 - arch/m68k/include/asm/gpio.h | 95 --- arch/m68k/include/asm/mcfgpio.h | 2 +- arch/powerpc/platforms/44x/Kconfig| 1 + arch/powerpc/platforms/4xx/gpio.c | 2 +- arch/powerpc/platforms/8xx/Kconfig| 1 + arch/powerpc/platforms/8xx/cpm1.c | 2 +- arch/powerpc/platforms/Kconfig| 2 + arch/powerpc/sysdev/cpm_common.c | 2 +- arch/sh/Kconfig | 1 - arch/sh/boards/board-magicpanelr2.c | 1 + arch/sh/boards/mach-ap325rxa/setup.c | 7 +- arch/sh/include/asm/gpio.h| 45 -- drivers/gpio/Kconfig | 19 ++- drivers/gpio/TODO | 15 +- drivers/gpio/gpio-aggregator.c| 9 +- drivers/gpio/gpio-altera.c| 2 +- drivers/gpio/gpio-davinci.c | 2 - drivers/gpio/gpio-mm-lantiq.c | 2 +- drivers/gpio/gpio-mpc5200.c | 2 +- drivers/gpio/gpio-reg.c | 12 +- drivers/gpio/gpio-regmap.c| 12 +- drivers/gpio/gpiolib-acpi.c | 10 +- drivers/gpio/gpiolib-acpi.h | 1 - drivers/gpio/gpiolib-of.c | 9 +- drivers/gpio/gpiolib-of.h | 1 - drivers/gpio/gpiolib-swnode.c | 5 +- drivers/gpio/gpiolib-sysfs.c | 25 ++- drivers/gpio/gpiolib.c| 9 +- drivers/hte/hte-tegra194-test.c | 10 +- drivers/input/touchscreen/ads7846.c | 5 +- drivers/media/pci/sta2x11/sta2x11_vip.c | 10 +- drivers/net/ieee802154/ca8210.c | 3 +- .../broadcom/brcm80211/brcmsmac/led.c | 1 + drivers/pinctrl/core.c| 1 - drivers/soc/fsl/qe/gpio.c | 2 +- include/asm-generic/gpio.h| 147 -- include/linux/gpio.h | 104 - include/linux/gpio/consumer.h | 24 +-- include/linux/gpio/driver.h | 31 +++- .../legacy-of-mm-gpiochip.h} | 33 +--- include/linux/mfd/ucb1x00.h | 1 + include
[PATCH v4 02/18] ARM: s3c24xx: Use the right include
From: Linus Walleij The file s3c64xx.c is including despite using no symbols from the file, however it needs it to implicitly bring in of_have_populated_dt() so include explicitly instead. Signed-off-by: Linus Walleij Signed-off-by: Andy Shevchenko --- arch/arm/mach-s3c/s3c64xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-s3c/s3c64xx.c b/arch/arm/mach-s3c/s3c64xx.c index e97bd59083a8..9f9717874d67 100644 --- a/arch/arm/mach-s3c/s3c64xx.c +++ b/arch/arm/mach-s3c/s3c64xx.c @@ -21,13 +21,13 @@ #include #include #include +#include #include #include #include #include #include #include -#include #include #include -- 2.39.1
[PATCH v4 11/18] gpiolib: split of_mm_gpio_chip out of linux/of_gpio.h
From: Arnd Bergmann
This is a rarely used feature that has nothing to do with the
client-side of_gpio.h.
Split it out with a separate header file and Kconfig option
so it can be removed on its own timeline aside from removing
the of_gpio consumer interfaces.
Reviewed-by: Andy Shevchenko
Signed-off-by: Arnd Bergmann
Reviewed-by: Linus Walleij
Signed-off-by: Andy Shevchenko
---
arch/powerpc/platforms/44x/Kconfig| 1 +
arch/powerpc/platforms/4xx/gpio.c | 2 +-
arch/powerpc/platforms/8xx/Kconfig| 1 +
arch/powerpc/platforms/8xx/cpm1.c | 2 +-
arch/powerpc/platforms/Kconfig| 2 ++
arch/powerpc/sysdev/cpm_common.c | 2 +-
drivers/gpio/Kconfig | 11 +++
drivers/gpio/TODO | 15 ++---
drivers/gpio/gpio-altera.c| 2 +-
drivers/gpio/gpio-mm-lantiq.c | 2 +-
drivers/gpio/gpio-mpc5200.c | 2 +-
drivers/gpio/gpiolib-of.c | 3 ++
drivers/soc/fsl/qe/gpio.c | 2 +-
.../legacy-of-mm-gpiochip.h} | 33 +++
include/linux/of_gpio.h | 21
15 files changed, 40 insertions(+), 61 deletions(-)
copy include/linux/{of_gpio.h => gpio/legacy-of-mm-gpiochip.h} (50%)
diff --git a/arch/powerpc/platforms/44x/Kconfig
b/arch/powerpc/platforms/44x/Kconfig
index 25b80cd558f8..1624ebf95497 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -230,6 +230,7 @@ config PPC4xx_GPIO
bool "PPC4xx GPIO support"
depends on 44x
select GPIOLIB
+ select OF_GPIO_MM_GPIOCHIP
help
Enable gpiolib support for ppc440 based boards
diff --git a/arch/powerpc/platforms/4xx/gpio.c
b/arch/powerpc/platforms/4xx/gpio.c
index 49ee8d365852..e5f2319e5cbe 100644
--- a/arch/powerpc/platforms/4xx/gpio.c
+++ b/arch/powerpc/platforms/4xx/gpio.c
@@ -14,7 +14,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
diff --git a/arch/powerpc/platforms/8xx/Kconfig
b/arch/powerpc/platforms/8xx/Kconfig
index 60cc5b537a98..a14d9d8997a4 100644
--- a/arch/powerpc/platforms/8xx/Kconfig
+++ b/arch/powerpc/platforms/8xx/Kconfig
@@ -101,6 +101,7 @@ comment "Generic MPC8xx Options"
config 8xx_GPIO
bool "GPIO API Support"
select GPIOLIB
+ select OF_GPIO_MM_GPIOCHIP
help
Saying Y here will cause the ports on an MPC8xx processor to be used
with the GPIO API. If you say N here, the kernel needs less memory.
diff --git a/arch/powerpc/platforms/8xx/cpm1.c
b/arch/powerpc/platforms/8xx/cpm1.c
index bb38c8d8f8de..56ca14f77543 100644
--- a/arch/powerpc/platforms/8xx/cpm1.c
+++ b/arch/powerpc/platforms/8xx/cpm1.c
@@ -44,7 +44,7 @@
#include
#ifdef CONFIG_8xx_GPIO
-#include
+#include
#endif
#define CPM_MAP_SIZE(0x4000)
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index d41dad227de8..8e4bbd19dec5 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -244,6 +244,7 @@ config QE_GPIO
bool "QE GPIO support"
depends on QUICC_ENGINE
select GPIOLIB
+ select OF_GPIO_MM_GPIOCHIP
help
Say Y here if you're going to use hardware that connects to the
QE GPIOs.
@@ -254,6 +255,7 @@ config CPM2
select CPM
select HAVE_PCI
select GPIOLIB
+ select OF_GPIO_MM_GPIOCHIP
help
The CPM2 (Communications Processor Module) is a coprocessor on
embedded CPUs made by Freescale. Selecting this option means that
diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
index 7dc1960f8bdb..8234013a8772 100644
--- a/arch/powerpc/sysdev/cpm_common.c
+++ b/arch/powerpc/sysdev/cpm_common.c
@@ -31,7 +31,7 @@
#include
#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO)
-#include
+#include
#endif
static int __init cpm_init(void)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 06a268d56800..178025ca3b34 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -39,6 +39,14 @@ config GPIOLIB_IRQCHIP
select IRQ_DOMAIN
bool
+config OF_GPIO_MM_GPIOCHIP
+ bool
+ help
+ This adds support for the legacy 'struct of_mm_gpio_chip' interface
+ from PowerPC. Existing drivers using this interface need to select
+ this symbol, but new drivers should use the generic gpio-regmap
+ infrastructure instead.
+
config DEBUG_GPIO
bool "Debug GPIO calls"
depends on DEBUG_KERNEL
@@ -131,6 +139,7 @@ config GPIO_ALTERA
tristate "Altera GPIO"
depends on OF_GPIO
select GPIOLIB_IRQCHIP
+ select OF_GPIO_MM_GPIOCHIP
help
Say Y or M here to build support for the Altera PIO device.
@@ -403,6 +412,7 @@ config
[PATCH v4 14/18] gpio: regmap: Add missing header(s)
Do not imply that some of the generic headers may be always included.
Instead, include explicitly what we are direct user of.
While at it, split out the GPIO group of headers.
Signed-off-by: Andy Shevchenko
---
drivers/gpio/gpio-regmap.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index fca17d478984..c08c8e528867 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -5,11 +5,17 @@
* Copyright 2020 Michael Walle
*/
-#include
-#include
-#include
+#include
+#include
+#include
+#include
#include
#include
+#include
+#include
+
+#include
+#include
struct gpio_regmap {
struct device *parent;
--
2.39.1
[PATCH v4 09/18] gpiolib: remove legacy gpio_export()
From: Arnd Bergmann There are only a handful of users of gpio_export() and related functions. As these are just wrappers around the modern gpiod_export() helper, remove the wrappers and open-code the gpio_to_desc in all callers to shrink the legacy API. Reviewed-by: Andy Shevchenko Signed-off-by: Arnd Bergmann Reviewed-by: Linus Walleij Signed-off-by: Andy Shevchenko --- Documentation/admin-guide/gpio/sysfs.rst | 2 +- Documentation/driver-api/gpio/legacy.rst | 21 --- .../zh_CN/driver-api/gpio/legacy.rst | 19 - Documentation/translations/zh_TW/gpio.txt | 18 - arch/arm/mach-omap2/pdata-quirks.c| 9 --- arch/sh/boards/mach-ap325rxa/setup.c | 7 ++--- drivers/gpio/gpiolib-sysfs.c | 4 +-- drivers/media/pci/sta2x11/sta2x11_vip.c | 10 --- drivers/net/ieee802154/ca8210.c | 3 ++- include/linux/gpio.h | 27 --- 10 files changed, 21 insertions(+), 99 deletions(-) diff --git a/Documentation/admin-guide/gpio/sysfs.rst b/Documentation/admin-guide/gpio/sysfs.rst index ec09ffd983e7..35171d15f78d 100644 --- a/Documentation/admin-guide/gpio/sysfs.rst +++ b/Documentation/admin-guide/gpio/sysfs.rst @@ -145,7 +145,7 @@ requested using gpio_request():: /* export the GPIO to userspace */ int gpiod_export(struct gpio_desc *desc, bool direction_may_change); - /* reverse gpio_export() */ + /* reverse gpiod_export() */ void gpiod_unexport(struct gpio_desc *desc); /* create a sysfs link to an exported GPIO node */ diff --git a/Documentation/driver-api/gpio/legacy.rst b/Documentation/driver-api/gpio/legacy.rst index e0306e78e34b..78372853c6d4 100644 --- a/Documentation/driver-api/gpio/legacy.rst +++ b/Documentation/driver-api/gpio/legacy.rst @@ -714,27 +714,6 @@ gpiochip nodes (possibly in conjunction with schematics) to determine the correct GPIO number to use for a given signal. -Exporting from Kernel code --- -Kernel code can explicitly manage exports of GPIOs which have already been -requested using gpio_request():: - - /* export the GPIO to userspace */ - int gpio_export(unsigned gpio, bool direction_may_change); - - /* reverse gpio_export() */ - void gpio_unexport(); - -After a kernel driver requests a GPIO, it may only be made available in -the sysfs interface by gpio_export(). The driver can control whether the -signal direction may change. This helps drivers prevent userspace code -from accidentally clobbering important system state. - -This explicit exporting can help with debugging (by making some kinds -of experiments easier), or can provide an always-there interface that's -suitable for documenting as part of a board support package. - - API Reference = diff --git a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst index dee2a0517c1c..84ce2322fdba 100644 --- a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst +++ b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst @@ -653,25 +653,6 @@ GPIO 控制器的路径类似 /sys/class/gpio/gpiochip42/ (对于从#42 GPIO 确定给定信号所用的 GPIO 编号。 -从内核代码中导出 - - -内核代码可以明确地管理那些已通过 gpio_request()申请的 GPIO 的导出:: - - /* 导出 GPIO 到用户空间 */ - int gpio_export(unsigned gpio, bool direction_may_change); - - /* gpio_export()的逆操作 */ - void gpio_unexport(); - -在一个内核驱动申请一个 GPIO 之后，它可以通过 gpio_export()使其在 sysfs -接口中可见。该驱动可以控制信号方向是否可修改。这有助于防止用户空间代码无意间 -破坏重要的系统状态。 - -这个明确的导出有助于(通过使某些实验更容易来)调试，也可以提供一个始终存在的接口， -与文档配合作为板级支持包的一部分。 - - API参考 === diff --git a/Documentation/translations/zh_TW/gpio.txt b/Documentation/translations/zh_TW/gpio.txt index dc608358d90a..62e560ffe628 100644 --- a/Documentation/translations/zh_TW/gpio.txt +++ b/Documentation/translations/zh_TW/gpio.txt @@ -614,21 +614,3 @@ GPIO 控制器的路徑類似 /sys/class/gpio/gpiochip42/ (對於從#42 GPIO 固定的,例如在擴展卡上的 GPIO會根據所使用的主板或所在堆疊架構中其他的板子而 有所不同。在這種情況下,你可能需要使用 gpiochip 節點(儘可能地結合電路圖)來 確定給定信號所用的 GPIO 編號。 - - -從內核代碼中導出 -- -內核代碼可以明確地管理那些已通過 gpio_request()申請的 GPIO 的導出: - - /* 導出 GPIO 到用戶空間 */ - int gpio_export(unsigned gpio, bool direction_may_change); - - /* gpio_export()的逆操作 */ - void gpio_unexport(); - -在一個內核驅動申請一個 GPIO 之後，它可以通過 gpio_export()使其在 sysfs -接口中可見。該驅動可以控制信號方向是否可修改。這有助於防止用戶空間代碼無意間 -破壞重要的系統狀態。 - -這個明確的導出有助於(通過使某些實驗更容易來)調試，也可以提供一個始終存在的接口， -與文檔配合作爲板級支持包的一部分。 diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c index baba73fd6f11..04208cc52784 100644 --- a/arch/arm/mach-omap2/pdata-quirks.c +++ b/arch/arm/mach-omap2/pdata-quirks.c @@ -6,6 +6,7 @@ */ #include #include +#include #include #include #include @@ -108,7 +109,7 @@ static int omap3_sbc_t3730_twl_callback(struct device *dev, if (res) return res; -
[PATCH v4 06/18] gpiolib: coldfire: remove custom asm/gpio.h
From: Arnd Bergmann
Now that coldfire is the only user of a custom asm/gpio.h, it seems
better to remove this as well, and have the same interface everywhere.
For the gpio_get_value()/gpio_set_value()/gpio_to_irq(), gpio_cansleep()
functions, the custom version is only a micro-optimization to inline the
function for constant GPIO numbers. However, in the coldfire defconfigs,
I was unable to find a single instance where this micro-optimization
was even used, and according to Geert the only user appears to be the
QSPI chip that is disabled everywhere.
The custom gpio_request_one() function is even less useful, as it is
guarded by an #ifdef that is never true.
Reviewed-by: Geert Uytterhoeven
Reviewed-by: Andy Shevchenko
Acked-by: Bartosz Golaszewski
Signed-off-by: Arnd Bergmann
Reviewed-by: Linus Walleij
Signed-off-by: Andy Shevchenko
---
arch/m68k/Kconfig.cpu| 1 -
arch/m68k/include/asm/gpio.h | 95
drivers/gpio/Kconfig | 8 ---
include/linux/gpio.h | 7 ---
4 files changed, 111 deletions(-)
delete mode 100644 arch/m68k/include/asm/gpio.h
diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu
index 9380f6e3bb66..96a0fb4f1af5 100644
--- a/arch/m68k/Kconfig.cpu
+++ b/arch/m68k/Kconfig.cpu
@@ -24,7 +24,6 @@ config M68KCLASSIC
config COLDFIRE
bool "Coldfire CPU family support"
- select ARCH_HAVE_CUSTOM_GPIO_H
select CPU_HAS_NO_BITFIELDS
select CPU_HAS_NO_CAS
select CPU_HAS_NO_MULDIV64
diff --git a/arch/m68k/include/asm/gpio.h b/arch/m68k/include/asm/gpio.h
deleted file mode 100644
index 5cfc0996ba94..
--- a/arch/m68k/include/asm/gpio.h
+++ /dev/null
@@ -1,95 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Coldfire generic GPIO support
- *
- * (C) Copyright 2009, Steven King
-*/
-
-#ifndef coldfire_gpio_h
-#define coldfire_gpio_h
-
-#include
-#include
-#include
-#include
-/*
- * The Generic GPIO functions
- *
- * If the gpio is a compile time constant and is one of the Coldfire gpios,
- * use the inline version, otherwise dispatch thru gpiolib.
- */
-
-static inline int gpio_get_value(unsigned gpio)
-{
- if (__builtin_constant_p(gpio) && gpio < MCFGPIO_PIN_MAX)
- return mcfgpio_read(__mcfgpio_ppdr(gpio)) & mcfgpio_bit(gpio);
- else
- return __gpio_get_value(gpio);
-}
-
-static inline void gpio_set_value(unsigned gpio, int value)
-{
- if (__builtin_constant_p(gpio) && gpio < MCFGPIO_PIN_MAX) {
- if (gpio < MCFGPIO_SCR_START) {
- unsigned long flags;
- MCFGPIO_PORTTYPE data;
-
- local_irq_save(flags);
- data = mcfgpio_read(__mcfgpio_podr(gpio));
- if (value)
- data |= mcfgpio_bit(gpio);
- else
- data &= ~mcfgpio_bit(gpio);
- mcfgpio_write(data, __mcfgpio_podr(gpio));
- local_irq_restore(flags);
- } else {
- if (value)
- mcfgpio_write(mcfgpio_bit(gpio),
- MCFGPIO_SETR_PORT(gpio));
- else
- mcfgpio_write(~mcfgpio_bit(gpio),
- MCFGPIO_CLRR_PORT(gpio));
- }
- } else
- __gpio_set_value(gpio, value);
-}
-
-static inline int gpio_to_irq(unsigned gpio)
-{
-#if defined(MCFGPIO_IRQ_MIN)
- if ((gpio >= MCFGPIO_IRQ_MIN) && (gpio < MCFGPIO_IRQ_MAX))
-#else
- if (gpio < MCFGPIO_IRQ_MAX)
-#endif
- return gpio + MCFGPIO_IRQ_VECBASE;
- else
- return __gpio_to_irq(gpio);
-}
-
-static inline int gpio_cansleep(unsigned gpio)
-{
- return gpio < MCFGPIO_PIN_MAX ? 0 : __gpio_cansleep(gpio);
-}
-
-#ifndef CONFIG_GPIOLIB
-static inline int gpio_request_one(unsigned gpio, unsigned long flags, const
char *label)
-{
- int err;
-
- err = gpio_request(gpio, label);
- if (err)
- return err;
-
- if (flags & GPIOF_DIR_IN)
- err = gpio_direction_input(gpio);
- else
- err = gpio_direction_output(gpio,
- (flags & GPIOF_INIT_HIGH) ? 1 : 0);
-
- if (err)
- gpio_free(gpio);
-
- return err;
-}
-#endif /* !CONFIG_GPIOLIB */
-#endif
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 218d7e4c27ff..06a268d56800 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -3,14 +3,6 @@
# GPIO infrastructure and drivers
#
-config ARCH_HAVE_CUSTOM_GPIO_H
- bool
- help
- Selecting this config option from the architecture Kconfig allows
- the architecture to provide a custom asm/gpio.h implementation
- overriding the default implementations. New uses of this
[PATCH v4 07/18] gpiolib: remove asm-generic/gpio.h
From: Arnd Bergmann
The asm-generic/gpio.h file is now always included when
using gpiolib, so just move its contents into linux/gpio.h
with a few minor simplifications.
Signed-off-by: Arnd Bergmann
Reviewed-by: Linus Walleij
Reviewed-by: Andy Shevchenko
Signed-off-by: Andy Shevchenko
---
MAINTAINERS | 1 -
arch/m68k/include/asm/mcfgpio.h | 2 +-
drivers/gpio/gpio-davinci.c | 2 -
drivers/pinctrl/core.c | 1 -
include/asm-generic/gpio.h | 146
include/linux/gpio.h| 94 +---
6 files changed, 85 insertions(+), 161 deletions(-)
delete mode 100644 include/asm-generic/gpio.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 94971603568b..955a513ac504 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8730,7 +8730,6 @@ F:Documentation/admin-guide/gpio/
F: Documentation/devicetree/bindings/gpio/
F: Documentation/driver-api/gpio/
F: drivers/gpio/
-F: include/asm-generic/gpio.h
F: include/dt-bindings/gpio/
F: include/linux/gpio.h
F: include/linux/gpio/
diff --git a/arch/m68k/include/asm/mcfgpio.h b/arch/m68k/include/asm/mcfgpio.h
index 27f32cc81da6..2cefe8445980 100644
--- a/arch/m68k/include/asm/mcfgpio.h
+++ b/arch/m68k/include/asm/mcfgpio.h
@@ -9,7 +9,7 @@
#define mcfgpio_h
#ifdef CONFIG_GPIOLIB
-#include
+#include
#else
int __mcfgpio_get_value(unsigned gpio);
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 26b1f7465e09..7fc83057990a 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -24,8 +24,6 @@
#include
#include
-#include
-
#define MAX_REGS_BANKS 5
#define MAX_INT_PER_BANK 32
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index d6e6c751255f..401886c81344 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -30,7 +30,6 @@
#ifdef CONFIG_GPIOLIB
#include "../gpio/gpiolib.h"
-#include
#endif
#include "core.h"
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
deleted file mode 100644
index 1c910d124423..
--- a/include/asm-generic/gpio.h
+++ /dev/null
@@ -1,146 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_GENERIC_GPIO_H
-#define _ASM_GENERIC_GPIO_H
-
-#include
-#include
-
-#ifdef CONFIG_GPIOLIB
-
-#include
-#include
-
-/*
- * Platforms may implement their GPIO interface with library code,
- * at a small performance cost for non-inlined operations and some
- * extra memory (for code and for per-GPIO table entries).
- */
-
-/*
- * At the end we want all GPIOs to be dynamically allocated from 0.
- * However, some legacy drivers still perform fixed allocation.
- * Until they are all fixed, leave 0-512 space for them.
- */
-#define GPIO_DYNAMIC_BASE 512
-
-struct device;
-struct gpio;
-struct seq_file;
-struct module;
-struct device_node;
-struct gpio_desc;
-
-/* Always use the library code for GPIO management calls,
- * or when sleeping may be involved.
- */
-extern int gpio_request(unsigned gpio, const char *label);
-extern void gpio_free(unsigned gpio);
-
-static inline int gpio_direction_input(unsigned gpio)
-{
- return gpiod_direction_input(gpio_to_desc(gpio));
-}
-static inline int gpio_direction_output(unsigned gpio, int value)
-{
- return gpiod_direction_output_raw(gpio_to_desc(gpio), value);
-}
-
-static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
-{
- return gpiod_set_debounce(gpio_to_desc(gpio), debounce);
-}
-
-static inline int gpio_get_value_cansleep(unsigned gpio)
-{
- return gpiod_get_raw_value_cansleep(gpio_to_desc(gpio));
-}
-static inline void gpio_set_value_cansleep(unsigned gpio, int value)
-{
- return gpiod_set_raw_value_cansleep(gpio_to_desc(gpio), value);
-}
-
-
-/* A platform's code may want to inline the I/O calls when
- * the GPIO is constant and refers to some always-present controller,
- * giving direct access to chip registers and tight bitbanging loops.
- */
-static inline int __gpio_get_value(unsigned gpio)
-{
- return gpiod_get_raw_value(gpio_to_desc(gpio));
-}
-static inline void __gpio_set_value(unsigned gpio, int value)
-{
- return gpiod_set_raw_value(gpio_to_desc(gpio), value);
-}
-
-static inline int __gpio_cansleep(unsigned gpio)
-{
- return gpiod_cansleep(gpio_to_desc(gpio));
-}
-
-static inline int __gpio_to_irq(unsigned gpio)
-{
- return gpiod_to_irq(gpio_to_desc(gpio));
-}
-
-extern int gpio_request_one(unsigned gpio, unsigned long flags, const char
*label);
-extern int gpio_request_array(const struct gpio *array, size_t num);
-extern void gpio_free_array(const struct gpio *array, size_t num);
-
-/*
- * A sysfs interface can be exported by individual drivers if they want,
- * but more typically is configured entirely from userspace.
- */
-static inline int gpio_export(unsigned gpio, bool direction_may_change)
-{
- return gpiod_export(gpio_to_desc(gpio), direction_may_change);
-}
-
[PATCH v4 04/18] gpiolib: Make the legacy consumer-only
From: Linus Walleij The legacy header was an all-inclusive header used by drivers and consumers alike. After eliminating the last users of the driver defines, we can drop the inclusion of the header. Signed-off-by: Linus Walleij Signed-off-by: Andy Shevchenko --- include/asm-generic/gpio.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 22cb8c9efc1d..1c910d124423 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -8,7 +8,6 @@ #ifdef CONFIG_GPIOLIB #include -#include #include /* -- 2.39.1
[PATCH v4 01/18] ARM: orion/gpio: Use the right include
From: Linus Walleij This is a GPIO driver so include and not the legacy header. Switch a single call to the legacy API and use as well. Signed-off-by: Linus Walleij Signed-off-by: Andy Shevchenko --- arch/arm/plat-orion/gpio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c index 3ef9ecdd6343..595e9cb33c1d 100644 --- a/arch/arm/plat-orion/gpio.c +++ b/arch/arm/plat-orion/gpio.c @@ -18,7 +18,8 @@ #include #include #include -#include +#include +#include #include #include #include @@ -312,7 +313,7 @@ int orion_gpio_led_blink_set(struct gpio_desc *desc, int state, case GPIO_LED_NO_BLINK_LOW: case GPIO_LED_NO_BLINK_HIGH: orion_gpio_set_blink(gpio, 0); - gpio_set_value(gpio, state); + gpiod_set_raw_value(desc, state); break; case GPIO_LED_BLINK: orion_gpio_set_blink(gpio, 1); -- 2.39.1
Re: [External] : RE: [EXT] [PATCH v2 1/1] PCI: layerscape: Add EP mode support for ls1028a
On Tue, Feb 07, 2023 at 04:20:21PM +, Frank Li wrote:
> > Subject: Re: [External] : RE: [EXT] [PATCH v2 1/1] PCI: layerscape: Add EP
> > mode support for ls1028a
> >
> > { .compatible = "fsl,ls1046a-pcie-ep", .data = _ep_drvdata },
> > + { .compatible = "fsl,ls1028a-pcie-ep", .data = _ep_drvdata },
> > { .compatible = "fsl,ls1088a-pcie-ep", .data = _ep_drvdata },
> >
> > can it be like this for better readability. ?
>
> It is just chip name and follow name conversion, which already
> upstreamed and documented.
>
> Why do you think it not is good readability?
I thought maybe ALOK's point was to sort the list, which does make a
lot of sense. But if you want to sort by the .data member, I would
think you would make .compatible a secondary sort key, which means
ls1028a would come before ls1046a, so you would end up with this
instead:
static const struct of_device_id ls_pcie_ep_of_match[] = {
+ { .compatible = "fsl,ls1028a-pcie-ep", .data = _ep_drvdata },
{ .compatible = "fsl,ls1046a-pcie-ep", .data = _ep_drvdata },
{ .compatible = "fsl,ls1088a-pcie-ep", .data = _ep_drvdata },
{ .compatible = "fsl,ls2088a-pcie-ep", .data = _ep_drvdata },
{ .compatible = "fsl,lx2160ar2-pcie-ep", .data = _ep_drvdata },
{ },
};
Re: [PATCH 7/7] KVM: Change return type of kvm_arch_vm_ioctl() to "int"
On Fri, 3 Feb 2023 10:42:30 +0100
Thomas Huth wrote:
> All kvm_arch_vm_ioctl() implementations now only deal with "int"
> types as return values, so we can change the return type of these
> functions to use "int" instead of "long".
>
> Signed-off-by: Thomas Huth
Acked-by: Claudio Imbrenda
> ---
> arch/arm64/kvm/arm.c | 3 +--
> arch/mips/kvm/mips.c | 4 ++--
> arch/powerpc/kvm/powerpc.c | 5 ++---
> arch/riscv/kvm/vm.c| 3 +--
> arch/s390/kvm/kvm-s390.c | 3 +--
> arch/x86/kvm/x86.c | 3 +--
> include/linux/kvm_host.h | 3 +--
> 7 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9c5573bc4614..e791ad6137b8 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1449,8 +1449,7 @@ static int kvm_vm_ioctl_set_device_addr(struct kvm *kvm,
> }
> }
>
> -long kvm_arch_vm_ioctl(struct file *filp,
> -unsigned int ioctl, unsigned long arg)
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> struct kvm *kvm = filp->private_data;
> void __user *argp = (void __user *)arg;
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index a25e0b73ee70..84cadaa2c2d3 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -1003,9 +1003,9 @@ void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm,
> kvm_flush_remote_tlbs(kvm);
> }
>
> -long kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> - long r;
> + int r;
>
> switch (ioctl) {
> default:
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 04494a4fb37a..6f6ba55c224f 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -2386,12 +2386,11 @@ static int kvmppc_get_cpu_char(struct
> kvm_ppc_cpu_char *cp)
> }
> #endif
>
> -long kvm_arch_vm_ioctl(struct file *filp,
> - unsigned int ioctl, unsigned long arg)
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> struct kvm *kvm __maybe_unused = filp->private_data;
> void __user *argp = (void __user *)arg;
> - long r;
> + int r;
>
> switch (ioctl) {
> case KVM_PPC_GET_PVINFO: {
> diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c
> index 65a964d7e70d..c13130ab459a 100644
> --- a/arch/riscv/kvm/vm.c
> +++ b/arch/riscv/kvm/vm.c
> @@ -87,8 +87,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> return r;
> }
>
> -long kvm_arch_vm_ioctl(struct file *filp,
> -unsigned int ioctl, unsigned long arg)
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> return -EINVAL;
> }
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 8ad1972b8a73..86ca49814983 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2850,8 +2850,7 @@ static int kvm_s390_vm_mem_op(struct kvm *kvm, struct
> kvm_s390_mem_op *mop)
> return r;
> }
>
> -long kvm_arch_vm_ioctl(struct file *filp,
> -unsigned int ioctl, unsigned long arg)
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> struct kvm *kvm = filp->private_data;
> void __user *argp = (void __user *)arg;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index caa2541833dd..c03363efc774 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6653,8 +6653,7 @@ static int kvm_vm_ioctl_set_clock(struct kvm *kvm, void
> __user *argp)
> return 0;
> }
>
> -long kvm_arch_vm_ioctl(struct file *filp,
> -unsigned int ioctl, unsigned long arg)
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg)
> {
> struct kvm *kvm = filp->private_data;
> void __user *argp = (void __user *)arg;
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 4f26b244f6d0..ed2f1f02976b 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1398,8 +1398,7 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct
> kvm_irq_level *irq_level,
> bool line_status);
> int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
> struct kvm_enable_cap *cap);
> -long kvm_arch_vm_ioctl(struct file *filp,
> -unsigned int ioctl, unsigned long arg);
> +int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long
> arg);
> long kvm_arch_vm_compat_ioctl(struct file *filp, unsigned int ioctl,
> unsigned long arg);
>
Re: [PATCH mm-unstable v1 04/26] arm/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On Wed, Feb 08, 2023 at 03:12:06PM +0100, David Hildenbrand wrote: > On 07.02.23 01:32, Mark Brown wrote: > > Today's -next (and at least back to Friday, older logs are unclear - I > > only noticed -next issues today) fails to NFS boot on an AT91SAM9G20-EK > > (an old ARMv5 platform) with multi_v5_defconfig, a bisect appears to > > point to this patch (20aae9eff5acd8f5 in today's -next) as the culprit. > It's been in -next for quite a while, thanks for the report! Yeah, there's been some other things obscuring the issue. > Could you give the following a test? > > > From 8c4bdbd9862f85782d5919d044c172b584063e83 Mon Sep 17 00:00:00 2001 > From: David Hildenbrand > Date: Wed, 8 Feb 2023 15:08:01 +0100 > Subject: [PATCH] arm/mm: Fix swp type masking in __swp_entry() > > We're masking with the number of type bits instead of the type mask, which > is obviously wrong. Tested-by: Mark Brown but note that I had to manually apply it, though it's pretty trivial so I probably applied the right thing. signature.asc Description: PGP signature
Re: [PATCH v2 18/19] powerpc/rtas: introduce rtas_function_token() API
Michael Ellerman writes:
> Nathan Lynch via B4 Submission Endpoint
> writes:
>> From: Nathan Lynch
>>
>> Users of rtas_token() supply a string argument that can't be validated
>> at build time. A typo or misspelling has to be caught by inspection or
>> by observing wrong behavior at runtime.
>>
>> Since the core RTAS code now has consolidated the names of all
>> possible RTAS functions and mapped them to their tokens, token lookup
>> can be implemented using symbolic constants to index a static array.
>>
>> So introduce rtas_function_token(), a replacement API which does that,
>> along with a rtas_service_present()-equivalent helper,
>> rtas_function_implemented(). Callers supply an opaque predefined
>> function handle which is used internally to index the function
>> table. Typos or other inappropriate arguments yield build errors, and
>> the function handle is a type that can't be easily confused with RTAS
>> tokens or other integer types.
>
> Why not go all the way and have the rtas_call() signature be:
>
> int rtas_call(rtas_fn_handle_t fn, int, int, int *, ...);
>
>
> And have it do the token lookup internally? That way a caller can never
> inadvertantly pass a random integer to rtas_call().
>
> And instead of eg:
>
> error = rtas_call(rtas_function_token(RTAS_FN_GET_TIME_OF_DAY), 0, 8,
> ret);
>
> we'd just need:
>
> error = rtas_call(RTAS_FN_GET_TIME_OF_DAY, 0, 8, ret);
>
>
> Doing the conversion all at once might be tricky. So maybe we need to add
> rtas_fn_call() which takes rtas_fn_handle_t so we can convert cases
> individually?
>
> Anyway just a thought. I guess we could merge this as-is and then do a
> further change to use rtas_fn_handle_t later.
You read my mind :-) But I want to go further and make the eventual
replacement for rtas_call() non-variadic, which will eliminate another
class of usage error.
Getting more ambitious: the ideal situation IMO would be that every use
of rtas_call() or its replacement is tidily contained in a C function in
kernel/rtas.c, where complexities like retries and error code
translation can be performed in a uniform way.
Anyway, a transition away from rtas_call(), whatever form it takes,
probably needs to happen incrementally.
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 41c430dc40c2..17e59306ce63 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -453,6 +453,26 @@ static struct rtas_function rtas_function_table[]
>> __ro_after_init = {
>> },
>> };
>>
>> +/**
>> + * rtas_function_token() - RTAS function token lookup.
>> + * @handle: Function handle, e.g. RTAS_FN_EVENT_SCAN.
>> + *
>> + * Context: Any context.
>> + * Return: the token value for the function if implemented by this platform,
>> + * otherwise RTAS_UNKNOWN_SERVICE.
>> + */
>> +s32 rtas_function_token(const rtas_fn_handle_t handle)
>> +{
>> +const size_t index = handle.index;
>> +const bool out_of_bounds = index >= ARRAY_SIZE(rtas_function_table);
>> +
>> +if (WARN_ONCE(out_of_bounds, "invalid function index %zu", index))
>> +return RTAS_UNKNOWN_SERVICE;
>
> This needs:
>
> + // If RTAS is not present or not initialised (yet) return unknown
> + if (!rtas.dev)
> + return RTAS_UNKNOWN_SERVICE;
> +
>
> Otherwise powernv breaks because it looks up tokens and gets back 0,
> because we never got as far as rtas_function_table_init() (to set all the
> tokens to RTAS_UNKNOWN_SERVICE), because we bailed out at the start of
> rtas_initialize() when we found no /rtas node.
Oh! OK, thanks.
Re: [PATCH v3 06/12] gpiolib: split linux/gpio/driver.h out of linux/gpio.h
On Wed, Feb 8, 2023 at 3:51 PM Andy Shevchenko wrote: > On Wed, Feb 08, 2023 at 12:55:06AM +0200, Andy Shevchenko wrote: > > On Tue, Feb 07, 2023 at 03:55:23PM +0100, Linus Walleij wrote: > > > On Tue, Feb 7, 2023 at 3:29 PM Andy Shevchenko > > > wrote: > > > > > > > From: Arnd Bergmann > > > > > > > > Almost all gpio drivers include linux/gpio/driver.h, and other > > > > files should not rely on includes from this header. > > > > > > > > Remove the indirect include from here and include the correct > > > > headers directly from where they are used. > > > > ... > > > > > Make sure you push this to the kernel.org build servers (zeroday builds), > > > > Of course, that is the purpose of publishing this before the release (so we > > will have some TODO list that eventually this can be applied for v6.4-rc1). > > > > > I think this patch needs to hit some more files, in my tests with a > > > similar > > > patch at least these: > > > > Right. I forgot to also incorporate your stuff into this series. > > Do you have anything that I can take as is? > > I'm going to incorporate the following: > > gpio: Make the legacy consumer-only > ARM: s3c24xx: Use the right include > ARM: orion/gpio: Use the right include > hte: tegra-194: Use proper includes > pcmcia: pxa2xx_viper: Include dependency Excellent, thanks. I don't care about being credited, just want things to go smooth so you run into less snags. Yours, Linus Walleij
Re: [PATCH v3 06/12] gpiolib: split linux/gpio/driver.h out of linux/gpio.h
On Wed, Feb 08, 2023 at 12:55:06AM +0200, Andy Shevchenko wrote: > On Tue, Feb 07, 2023 at 03:55:23PM +0100, Linus Walleij wrote: > > On Tue, Feb 7, 2023 at 3:29 PM Andy Shevchenko > > wrote: > > > > > From: Arnd Bergmann > > > > > > Almost all gpio drivers include linux/gpio/driver.h, and other > > > files should not rely on includes from this header. > > > > > > Remove the indirect include from here and include the correct > > > headers directly from where they are used. > > ... > > > Make sure you push this to the kernel.org build servers (zeroday builds), > > Of course, that is the purpose of publishing this before the release (so we > will have some TODO list that eventually this can be applied for v6.4-rc1). > > > I think this patch needs to hit some more files, in my tests with a similar > > patch at least these: > > Right. I forgot to also incorporate your stuff into this series. > Do you have anything that I can take as is? I'm going to incorporate the following: gpio: Make the legacy consumer-only ARM: s3c24xx: Use the right include ARM: orion/gpio: Use the right include hte: tegra-194: Use proper includes pcmcia: pxa2xx_viper: Include dependency -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 11/19] powerpc/rtas: add work area allocator
Michael Ellerman writes:
> Nathan Lynch via B4 Submission Endpoint
> writes:
>> diff --git a/arch/powerpc/include/asm/rtas-work-area.h
>> b/arch/powerpc/include/asm/rtas-work-area.h
>> new file mode 100644
>> index ..76ccb039cc37
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/rtas-work-area.h
>> @@ -0,0 +1,45 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef POWERPC_RTAS_WORK_AREA_H
>> +#define POWERPC_RTAS_WORK_AREA_H
>
> The usual style would be _ASM_POWERPC_RTAS_WORK_AREA_H.
OK. (will change in all new headers)
>> +static struct rtas_work_area_allocator_state {
>> +struct gen_pool *gen_pool;
>> +char *arena;
>> +struct mutex mutex; /* serializes allocations */
>> +struct wait_queue_head wqh;
>> +mempool_t descriptor_pool;
>> +bool available;
>> +} rwa_state_ = {
>> +.mutex = __MUTEX_INITIALIZER(rwa_state_.mutex),
>> +.wqh = __WAIT_QUEUE_HEAD_INITIALIZER(rwa_state_.wqh),
>> +};
>> +static struct rtas_work_area_allocator_state *rwa_state = _state_;
>
> I assumed the pointer was so you could swap this out at runtime or
> something, but I don't think you do.
>
> Any reason not to drop the pointer and just use rwa_state.foo accessors?
> That would also allow the struct to be anonymous.
>
> Or if you have the pointer you can at least make it NULL prior to init
> and avoid the need for "available".
I think it's there because earlier versions of this that I never posted
had unit tests. I'll either resurrect those or reduce the indirection.
>> +/*
>> + * A single work area buffer and descriptor to serve requests early in
>> + * boot before the allocator is fully initialized.
>> + */
>> +static bool early_work_area_in_use __initdata;
>> +static char early_work_area_buf[SZ_4K] __initdata;
>
> That should be page aligned I think?
Yes. It happens to be safe in this version because ibm,get-system-parameter,
which has no alignment requirement, is the only function used early
enough to use the buffer. But that's too fragile.
>> +static struct rtas_work_area early_work_area __initdata = {
>> +.buf = early_work_area_buf,
>> +.size = sizeof(early_work_area_buf),
>> +};
>> +
>> +
>> +static struct rtas_work_area * __init rtas_work_area_alloc_early(size_t
>> size)
>> +{
>> +WARN_ON(size > early_work_area.size);
>> +WARN_ON(early_work_area_in_use);
>> +early_work_area_in_use = true;
>> +memset(early_work_area.buf, 0, early_work_area.size);
>> +return _work_area;
>> +}
>> +
>> +static void __init rtas_work_area_free_early(struct rtas_work_area
>> *work_area)
>> +{
>> +WARN_ON(work_area != _work_area);
>> +WARN_ON(!early_work_area_in_use);
>> +early_work_area_in_use = false;
>> +}
>> +
>> +struct rtas_work_area * __ref rtas_work_area_alloc(size_t size)
>> +{
>> +struct rtas_work_area *area;
>> +unsigned long addr;
>> +
>> +might_sleep();
>> +
>> +WARN_ON(size > RTAS_WORK_AREA_MAX_ALLOC_SZ);
>> +size = min_t(size_t, size, RTAS_WORK_AREA_MAX_ALLOC_SZ);
>
> This seems unsafe.
>
> If you return a buffer smaller than the caller asks for they're likely
> to read/write past the end of it and corrupt memory.
OK, let's figure out another way to handle this.
> AFAIK genalloc doesn't have guard pages or anything fancy to save us
> from that - but maybe I'm wrong, I've never used it.
Yeah we would have to build our own thing on top of it. And I don't
think it could be something that traps on access, it would have to be a
check in rtas_work_area_free(), after the fact.
> There's only three callers in the end, seems like we should just return
> NULL if the size is too large and have callers check the return value.
There are more conversions to do, and a property I hope to maintain is
that requests can't fail. Existing users of rtas_data_buf don't have
error paths for failure to acquire the buffer.
I believe the allocation size passed to rtas_work_area_alloc() can be
known at build time in all cases. Maybe we could prevent inappropriate
requests from being built with a compile-time assertion (untested):
/* rtas-work-area.h */
static inline struct rtas_work_area *rtas_work_area_alloc(size_t sz)
{
static_assert(sz < RTAS_WORK_AREA_MAX_ALLOC_SZ);
return __rtas_work_area_alloc(sz);
}
I think this would be OK? If I can't make it work I'll fall back to
returning NULL as you suggest, but it will make for more churn (and
risk) in the conversions.
>> +if (!rwa_state->available) {
>> +area = rtas_work_area_alloc_early(size);
>> +goto out;
>> +}
>> +
>> +/*
>> + * To ensure FCFS behavior and prevent a high rate of smaller
>> + * requests from starving larger ones, use the mutex to queue
>> + * allocations.
>> + */
>> +mutex_lock(_state->mutex);
>> +wait_event(rwa_state->wqh,
>> + (addr = gen_pool_alloc(rwa_state->gen_pool, size)) != 0);
>> +mutex_unlock(_state->mutex);
>> +
>> +area =
Re: [PATCH mm-unstable v1 04/26] arm/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE
On 07.02.23 01:32, Mark Brown wrote:
On Fri, Jan 13, 2023 at 06:10:04PM +0100, David Hildenbrand wrote:
Let's support __HAVE_ARCH_PTE_SWP_EXCLUSIVE by stealing one bit from the
offset. This reduces the maximum swap space per file to 64 GiB (was 128
GiB).
While at it drop the PTE_TYPE_FAULT from __swp_entry_to_pte() which is
defined to be 0 and is rather confusing because we should be dealing
with "Linux PTEs" not "hardware PTEs". Also, properly mask the type in
__swp_entry().
Today's -next (and at least back to Friday, older logs are unclear - I
only noticed -next issues today) fails to NFS boot on an AT91SAM9G20-EK
(an old ARMv5 platform) with multi_v5_defconfig, a bisect appears to
point to this patch (20aae9eff5acd8f5 in today's -next) as the culprit.
It's been in -next for quite a while, thanks for the report!
The failure happens at some point after starting userspace, the kernel
starts spamming the console with messages in the form:
get_swap_device: Bad swap file entry 10120d20
_swap_info_get() tells us that the swp type seems to be bad.
I assume we're dealing with a migration entry, if swap is disabled, and fail to
detect is_migration_entry() correctly because the type is messed up.
Could you give the following a test?
From 8c4bdbd9862f85782d5919d044c172b584063e83 Mon Sep 17 00:00:00 2001
From: David Hildenbrand
Date: Wed, 8 Feb 2023 15:08:01 +0100
Subject: [PATCH] arm/mm: Fix swp type masking in __swp_entry()
We're masking with the number of type bits instead of the type mask, which
is obviously wrong.
Fixes: 20aae9eff5ac ("arm/mm: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE")
Reported-by: Mark Brown
Signed-off-by: David Hildenbrand
---
arch/arm/include/asm/pgtable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 2e626e6da9a3..a58ccbb406ad 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -292,7 +292,7 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
#define __swp_type(x) (((x).val >> __SWP_TYPE_SHIFT) & __SWP_TYPE_MASK)
#define __swp_offset(x)((x).val >> __SWP_OFFSET_SHIFT)
-#define __swp_entry(type, offset) ((swp_entry_t) { (((type) & __SWP_TYPE_BITS)
<< __SWP_TYPE_SHIFT) | \
+#define __swp_entry(type, offset) ((swp_entry_t) { (((type) & __SWP_TYPE_MASK)
<< __SWP_TYPE_SHIFT) | \
((offset) <<
__SWP_OFFSET_SHIFT) })
#define __pte_to_swp_entry(pte) ((swp_entry_t) { pte_val(pte) })
--
2.39.1
--
Thanks,
David / dhildenb
Re: API for setting multiple PTEs at once
On Wed, Feb 08, 2023 at 08:09:00PM +0800, Yin, Fengwei wrote: > > > On 2/8/2023 7:23 PM, Alexandre Ghiti wrote: > > Hi Matthew, > > > > On 2/7/23 21:27, Matthew Wilcox wrote: > >> On Thu, Feb 02, 2023 at 09:14:23PM +, Matthew Wilcox wrote: > >>> For those of you not subscribed, linux-mm is currently discussing > >>> how best to handle page faults on large folios. I simply made it work > >>> when adding large folio support. Now Yin Fengwei is working on > >>> making it fast. > >> OK, here's an actual implementation: > >> > >> https://lore.kernel.org/linux-mm/20230207194937.122543-3-wi...@infradead.org/ > >> > >> It survives a run of xfstests. If your architecture doesn't store its > >> PFNs at PAGE_SHIFT, you're going to want to implement your own set_ptes(), > > > > > > riscv stores its pfn at PAGE_PFN_SHIFT instead of PAGE_SHIFT, se we need to > > reimplement set_ptes. But I have been playing with your patchset and we > > never fall into the case where set_ptes is called with nr > 1, any idea > > why? I booted a large ubuntu defconfig and launched > > will_it_scale.page_fault4. > Need to use xfs filesystem to get large folio for file mapping. > Other filesystem may be also OK. But I just tried xfs. Thanks. XFS is certainly the flagship filesystem to support large folios, but others have added support, AFS and EROFS. You can also get large folios in tmpfs (which is slightly different as it focuses on THPs rather than generic large folios). You also have to have CONFIG_TRANSPARENT_HUGEPAGE selected, which riscv can do. That restriction will be lifted at some point, but for now large folios depends on the THP infrastructure.
Re: [PATCH v2 07/19] powerpc/rtas: improve function information lookups
Michael Ellerman writes: > Nathan Lynch via B4 Submission Endpoint > writes: >> From: Nathan Lynch >> >> The core RTAS support code and its clients perform two types of lookup >> for RTAS firmware function information. >> > ... >> diff --git a/arch/powerpc/include/asm/rtas.h >> b/arch/powerpc/include/asm/rtas.h >> index 479a95cb2770..14fe79217c26 100644 >> --- a/arch/powerpc/include/asm/rtas.h >> +++ b/arch/powerpc/include/asm/rtas.h >> @@ -16,6 +16,93 @@ >> * Copyright (C) 2001 PPC 64 Team, IBM Corp >> */ >> >> +#define rtas_fnidx(x_) RTAS_FNIDX__ ## x_ > > I'd prefer we just spelt it out in full, to aid grepability and > cscope/tags etc. OK.
Re: [PATCH v2 01/19] powerpc/rtas: handle extended delays safely in early boot
Michael Ellerman writes:
> Nathan Lynch via B4 Submission Endpoint
> writes:
>> From: Nathan Lynch
>>
>> Some code that runs early in boot calls RTAS functions that can return
>> -2 or 990x statuses, which mean the caller should retry. An example is
>> pSeries_cmo_feature_init(), which invokes ibm,get-system-parameter but
>> treats these benign statuses as errors instead of retrying.
>>
>> pSeries_cmo_feature_init() and similar code should be made to retry
>> until they succeed or receive a real error, using the usual pattern:
>>
>> do {
>> rc = rtas_call(token, etc...);
>> } while (rtas_busy_delay(rc));
>>
>> But rtas_busy_delay() will perform a timed sleep on any 990x
>> status. This isn't safe so early in boot, before the CPU scheduler and
>> timer subsystem have initialized.
>>
>> The -2 RTAS status is much more likely to occur during single-threaded
>> boot than 990x in practice, at least on PowerVM. This is because -2
>> usually means that RTAS made progress but exhausted its self-imposed
>> timeslice, while 990x is associated with concurrent requests from the
>> OS causing internal contention. Regardless, according to the language
>> in PAPR, the OS should be prepared to handle either type of status at
>> any time.
>>
>> Add a fallback path to rtas_busy_delay() to handle this as safely as
>> possible, performing a small delay on 990x. Include a counter to
>> detect retry loops that aren't making progress and bail out.
>>
>> This was found by inspection and I'm not aware of any real
>> failures. However, the implementation of rtas_busy_delay() before
>> commit 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
>> was not susceptible to this problem, so let's treat this as a
>> regression.
>>
>> Signed-off-by: Nathan Lynch
>> Fixes: 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
>> ---
>> arch/powerpc/kernel/rtas.c | 48
>> +-
>> 1 file changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>> index 795225d7f138..ec2df09a70cf 100644
>> --- a/arch/powerpc/kernel/rtas.c
>> +++ b/arch/powerpc/kernel/rtas.c
>> @@ -606,6 +606,46 @@ unsigned int rtas_busy_delay_time(int status)
>> return ms;
>> }
>>
>> +/*
>> + * Early boot fallback for rtas_busy_delay().
>> + */
>> +static bool __init rtas_busy_delay_early(int status)
>> +{
>> +static size_t successive_ext_delays __initdata;
>> +bool ret;
>
> I think the logic would be easier to read if this was called "wait", but
> maybe that's just me.
Maybe "retry"? That communicates what the function is telling callers to do.
>
>> +switch (status) {
>> +case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
>> +/*
>> + * In the unlikely case that we receive an extended
>> + * delay status in early boot, the OS is probably not
>> + * the cause, and there's nothing we can do to clear
>> + * the condition. Best we can do is delay for a bit
>> + * and hope it's transient. Lie to the caller if it
>> + * seems like we're stuck in a retry loop.
>> + */
>> +mdelay(1);
>> +ret = true;
>> +successive_ext_delays += 1;
>> +if (successive_ext_delays > 1000) {
>> +pr_err("too many extended delays, giving up\n");
>> +dump_stack();
>> +ret = false;
>
> Shouldn't we zero successive_ext_delays here?
>
> Otherwise a subsequent (possibly different) RTAS call will immediately
> fail out here if it gets a single extended delay from RTAS, won't it?
Yes, will fix. Thanks.
>
>> +}
>> +break;
>> +case RTAS_BUSY:
>> +ret = true;
>> +successive_ext_delays = 0;
>> +break;
>> +default:
>> +ret = false;
>> +successive_ext_delays = 0;
>> +break;
>> +}
>> +
>> +return ret;
>> +}
>> +
>> /**
>> * rtas_busy_delay() - helper for RTAS busy and extended delay statuses
>> *
>> @@ -624,11 +664,17 @@ unsigned int rtas_busy_delay_time(int status)
>> * * false - @status is not @RTAS_BUSY nor an extended delay hint. The
>> * caller is responsible for handling @status.
>> */
>> -bool rtas_busy_delay(int status)
>> +bool __ref rtas_busy_delay(int status)
>
> Can you explain the __ref in the change log.
Yes, will add that.
>> {
>> unsigned int ms;
>> bool ret;
>>
>> +/*
>> + * Can't do timed sleeps before timekeeping is up.
>> + */
>> +if (system_state < SYSTEM_SCHEDULING)
>> +return rtas_busy_delay_early(status);
>> +
>> switch (status) {
>> case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
>> ret = true;
>>
>
> cheers
Re: API for setting multiple PTEs at once
On 2/8/2023 7:23 PM, Alexandre Ghiti wrote: > Hi Matthew, > > On 2/7/23 21:27, Matthew Wilcox wrote: >> On Thu, Feb 02, 2023 at 09:14:23PM +, Matthew Wilcox wrote: >>> For those of you not subscribed, linux-mm is currently discussing >>> how best to handle page faults on large folios. I simply made it work >>> when adding large folio support. Now Yin Fengwei is working on >>> making it fast. >> OK, here's an actual implementation: >> >> https://lore.kernel.org/linux-mm/20230207194937.122543-3-wi...@infradead.org/ >> >> It survives a run of xfstests. If your architecture doesn't store its >> PFNs at PAGE_SHIFT, you're going to want to implement your own set_ptes(), > > > riscv stores its pfn at PAGE_PFN_SHIFT instead of PAGE_SHIFT, se we need to > reimplement set_ptes. But I have been playing with your patchset and we never > fall into the case where set_ptes is called with nr > 1, any idea why? I > booted a large ubuntu defconfig and launched will_it_scale.page_fault4. Need to use xfs filesystem to get large folio for file mapping. Other filesystem may be also OK. But I just tried xfs. Thanks. Regards Yin, Fengwei > > I'll come up with the proper implementation of set_ptes anyway soon. > > Thanks, > > Alex > > >> or you'll see entirely the wrong pages mapped into userspace. You may >> also wish to implement set_ptes() if it can be done more efficiently >> than __pte(pteval(pte) + PAGE_SIZE). >> >> Architectures that implement things like flush_icache_page() and >> update_mmu_cache() may want to propose batched versions of those. >> That's alpha, csky, m68k, mips, nios2, parisc, sh, >> arm, loongarch, openrisc, powerpc, riscv, sparc and xtensa. >> Maintainers BCC'd, mailing lists CC'd. >> >> I'm happy to collect implementations and submit them as part of a v6. >> >> ___ >> linux-riscv mailing list >> linux-ri...@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v3 08/12] gpio: aggregator: Add missing header(s)
Hi Andy,
Thanks for your patch!
On Tue, Feb 7, 2023 at 3:29 PM Andy Shevchenko
wrote:
> Do not imply that some of the generic headers may be always included.
> Instead, include explicitly what we are direct user of.
That applies only to the addition of #include ...
Please also describe the other changes.
> Signed-off-by: Andy Shevchenko
> ---
> drivers/gpio/gpio-aggregator.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> index 6d17d262ad91..20a686f12df7 100644
> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -10,19 +10,20 @@
> #include
> #include
> #include
> -#include
> -#include
> -#include
> -#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
>
> +#include
> +#include
> +#include
> +
> #define AGGREGATOR_MAX_GPIOS 512
For the actual changes:
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 0/3] ALSA: core: Make some functions return void
On 07. 02. 23 20:19, Uwe Kleine-König wrote: Hello, while checking in which cases hda_tegra_remove() can return a non-zero value, I found that actually cannot happen. This series makes the involved functions return void to make this obvious. This is a preparation for making platform_driver::remove return void, too. Best regards Uwe Uwe Kleine-König (3): ALSA: core: Make snd_card_disconnect() return void ALSA: core: Make snd_card_free_when_closed() return void ALSA: core: Make snd_card_free() return void include/sound/core.h | 6 +++--- sound/core/init.c | 40 ++- sound/pci/hda/hda_tegra.c | 6 ++ sound/ppc/snd_ps3.c | 4 +--- 4 files changed, 20 insertions(+), 36 deletions(-) Reviewed-by: Jaroslav Kysela -- Jaroslav Kysela Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
Re: [PATCH 0/3] ALSA: core: Make some functions return void
On Tue, 07 Feb 2023 20:19:04 +0100, Uwe Kleine-König wrote: > > Hello, > > while checking in which cases hda_tegra_remove() can return a non-zero value, > I > found that actually cannot happen. This series makes the involved functions > return void to make this obvious. > > This is a preparation for making platform_driver::remove return void, too. > > Best regards > Uwe > > Uwe Kleine-König (3): > ALSA: core: Make snd_card_disconnect() return void > ALSA: core: Make snd_card_free_when_closed() return void > ALSA: core: Make snd_card_free() return void Applied all three patches now. Thanks. Takashi
Re: [PATCH v4 7/7] powerpc: kcsan: Add KCSAN Support
Le 08/02/2023 à 04:22, Rohan McLure a écrit : > Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the > kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options. > KCSAN requires compiler builtins __atomic_* 64-bit values, and so only > report support on PPC64. Copy/pasted from v3 ? In v4 PPC32 is supported as well. > > See documentation in Documentation/dev-tools/kcsan.rst for more > information. > > Signed-off-by: Rohan McLure > --- > v3: Restrict support to 64-bit, as TSAN expects 64-bit __atomic_* compiler > built-ins. > --- > arch/powerpc/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index b8c4ac56bddc..55bc2d724c73 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -198,6 +198,7 @@ config PPC > select HAVE_ARCH_KASAN if PPC_RADIX_MMU > select HAVE_ARCH_KASAN if PPC_BOOK3E_64 > select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > + select HAVE_ARCH_KCSANif PPC64 That if PPC64 should go away as we now have the builtins. > select HAVE_ARCH_KFENCE if ARCH_SUPPORTS_DEBUG_PAGEALLOC > select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET > select HAVE_ARCH_KGDB
Re: [PATCH v4 1/7] kcsan: Add atomic builtin stubs for 32-bit systems
Le 08/02/2023 à 04:21, Rohan McLure a écrit :
> KCSAN instruments calls to atomic builtins, and will in turn call these
> builtins itself. As such, architectures supporting KCSAN must have
> compiler support for these atomic primitives.
>
> Since 32-bit systems are unlikely to have 64-bit compiler builtins,
> provide a stub for each missing builtin, and use BUG() to assert
> unreachability.
>
> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
> locally, but does not advertise the fact with preprocessor macros. To
> avoid production of duplicate symbols, do not build the stubs on xtensa.
> A future patch will remove the xtensa implementation of these stubs.
>
> Signed-off-by: Rohan McLure
> ---
> v4: New patch
> ---
> kernel/kcsan/Makefile | 3 ++
> kernel/kcsan/stubs.c | 78 +++
> 2 files changed, 81 insertions(+)
> create mode 100644 kernel/kcsan/stubs.c
I think it would be better to merge patch 1 and patch 2, that way we
would keep the history and see that stubs.c almost comes from xtensa.
The summary would then be:
arch/xtensa/lib/Makefile | 1 -
kernel/kcsan/Makefile | 2 +-
arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 26
+-
3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
> index 8cf70f068d92..5dfc5825aae9 100644
> --- a/kernel/kcsan/Makefile
> +++ b/kernel/kcsan/Makefile
> @@ -12,6 +12,9 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
> -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>
> obj-y := core.o debugfs.o report.o
> +ifndef XTENSA
> + obj-y += stubs.o
obj-$(CONFIG_32BIT) += stubs.o
That would avoid the #if CONFIG_32BIT in stubs.c
> +endif
>
> KCSAN_INSTRUMENT_BARRIERS_selftest.o := y
> obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
> diff --git a/kernel/kcsan/stubs.c b/kernel/kcsan/stubs.c
> new file mode 100644
> index ..ec5cd99be422
> --- /dev/null
> +++ b/kernel/kcsan/stubs.c
> @@ -0,0 +1,78 @@
> +// SPDX-License Identifier: GPL-2.0
Missing - between License and Identifier ?
> +
> +#include
> +#include
> +
> +#ifdef CONFIG_32BIT
Should be handled in Makefile
> +
> +#if !__has_builtin(__atomic_store_8)
Does any 32 bit ARCH support that ? Is that #if required ?
If yes, do we really need the #if for each and every function, can't we
just check for one and assume that if we don't have __atomic_store_8 we
don't have any of the functions ?
> +void __atomic_store_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_load_8)
> +u64 __atomic_load_8(const volatile void *p, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_exchange_8)
> +u64 __atomic_exchange_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_compare_exchange_8)
> +bool __atomic_compare_exchange_8(volatile void *p1, void *p2, u64 v, bool b,
> int i1, int i2)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_fetch_add_8)
> +u64 __atomic_fetch_add_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_fetch_sub_8)
> +u64 __atomic_fetch_sub_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_fetch_and_8)
> +u64 __atomic_fetch_and_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_fetch_or_8)
> +u64 __atomic_fetch_or_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_fetch_xor_8)
> +u64 __atomic_fetch_xor_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#if !__has_builtin(__atomic_fetch_nand_8)
> +u64 __atomic_fetch_nand_8(volatile void *p, u64 v, int i)
> +{
> + BUG();
> +}
> +#endif
> +
> +#endif /* CONFIG_32BIT */
Re: [PATCH 6/7] KVM: arm64: Change return type of kvm_vm_ioctl_mte_copy_tags() to "int"
On 08/02/2023 12.51, Steven Price wrote:
On 08/02/2023 08:49, Cornelia Huck wrote:
On Wed, Feb 08 2023, Gavin Shan wrote:
On 2/7/23 9:09 PM, Thomas Huth wrote:
Oh, drat, I thought I had checked all return statements ... this must have
fallen through the cracks, sorry!
Anyway, this is already a problem now: The function is called from kvm_arch_vm_ioctl()
(which still returns a long), which in turn is called from kvm_vm_ioctl() in
virt/kvm/kvm_main.c. And that functions stores the return value in an "int r"
variable. So the upper bits are already lost there.
Sorry about that, I was caught out by kvm_arch_vm_ioctl() returning long...
That's why I'm trying to fix that return type mess with my series, to avoid
such problems in the future :-)
Also, how is this supposed to work from user space? The normal "ioctl()" libc function
just returns an "int" ? Is this ioctl already used in a userspace application somewhere?
... at least in QEMU, I didn't spot it yet...
We will need it in QEMU to implement migration with MTE (the current
proposal simply adds a migration blocker when MTE is enabled, as there
are various other things that need to be figured out for this to work.)
But maybe other VMMs already use it (and have been lucky because they
always dealt with shorter lengths?)
The ioctl command KVM_ARM_MTE_COPY_TAGS was merged recently and not used
by QEMU yet. I think struct kvm_arm_copy_mte_tags::length needs to be
'__u32' instead of '__u64' in order to standardize the return value.
Something like below. Documentation/virt/kvm/api.rst::section-4.130
needs update accordingly.
struct kvm_arm_copy_mte_tags {
__u64 guest_ipa;
__u32 pad;
__u32 length;
void __user *addr;
__u64 flags;
__u64 reserved[2];
};
Can we do this in a more compatible way, as we are dealing with an API?
Like returning -EINVAL if length is too big?
I agree the simplest fix for the problem is simply to reject any
lengths>INT_MAX:
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index cf4c495a4321..94aed7ce85c4 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -1032,6 +1032,13 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST)
return -EINVAL;
+ /*
+* ioctl returns int, so lengths above INT_MAX cannot be
+* represented in the return value
+*/
+ if (length > INT_MAX)
+ return -EINVAL;
+
if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK)
return -EINVAL;
This could also be fixed in a useable way by including a new flag which
returns the length in an output field of the ioctl structure. I'm
guessing a 2GB limit would be annoying to work around.
I agree that checking for length > INT_MAX is likely the best thing to do
here right now. I'll add that in v2 of my series.
But actually, this might even be a good thing from another point of view (so
I'm not sure whether your idea with the flag should really be pursued): The
code here takes a mutex and then runs a while loop that depends on the
length - which could cause the lock to be held for a rather long time if
length is a 64-bit value. Forcing the user space to limit the length here
could help to avoid taking the lock for too long.
Thomas
Re: [PATCH v2 18/19] powerpc/rtas: introduce rtas_function_token() API
Nathan Lynch via B4 Submission Endpoint
writes:
> From: Nathan Lynch
>
> Users of rtas_token() supply a string argument that can't be validated
> at build time. A typo or misspelling has to be caught by inspection or
> by observing wrong behavior at runtime.
>
> Since the core RTAS code now has consolidated the names of all
> possible RTAS functions and mapped them to their tokens, token lookup
> can be implemented using symbolic constants to index a static array.
>
> So introduce rtas_function_token(), a replacement API which does that,
> along with a rtas_service_present()-equivalent helper,
> rtas_function_implemented(). Callers supply an opaque predefined
> function handle which is used internally to index the function
> table. Typos or other inappropriate arguments yield build errors, and
> the function handle is a type that can't be easily confused with RTAS
> tokens or other integer types.
Why not go all the way and have the rtas_call() signature be:
int rtas_call(rtas_fn_handle_t fn, int, int, int *, ...);
And have it do the token lookup internally? That way a caller can never
inadvertantly pass a random integer to rtas_call().
And instead of eg:
error = rtas_call(rtas_function_token(RTAS_FN_GET_TIME_OF_DAY), 0, 8,
ret);
we'd just need:
error = rtas_call(RTAS_FN_GET_TIME_OF_DAY, 0, 8, ret);
Doing the conversion all at once might be tricky. So maybe we need to add
rtas_fn_call() which takes rtas_fn_handle_t so we can convert cases
individually?
Anyway just a thought. I guess we could merge this as-is and then do a
further change to use rtas_fn_handle_t later.
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 14fe79217c26..fe400438c1fb 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -103,6 +103,99 @@ enum rtas_function_index {
> rtas_fnidx(WRITE_PCI_CONFIG),
> };
>
> +/*
> + * Opaque handle for client code to refer to RTAS functions. All valid
> + * function handles are build-time constants prefixed with RTAS_FN_.
> + */
> +typedef struct {
> + const enum rtas_function_index index;
> +} rtas_fn_handle_t;
> +
> +#define rtas_fn_handle(x_) ((const rtas_fn_handle_t) { .index =
> rtas_fnidx(x_), })
> +
> +#define RTAS_FN_CHECK_EXCEPTION
> rtas_fn_handle(CHECK_EXCEPTION)
> +#define RTAS_FN_DISPLAY_CHARACTER
> rtas_fn_handle(DISPLAY_CHARACTER)
> +#define RTAS_FN_EVENT_SCANrtas_fn_handle(EVENT_SCAN)
> +#define RTAS_FN_FREEZE_TIME_BASE
> rtas_fn_handle(FREEZE_TIME_BASE)
> +#define RTAS_FN_GET_POWER_LEVEL
> rtas_fn_handle(GET_POWER_LEVEL)
> +#define RTAS_FN_GET_SENSOR_STATE
> rtas_fn_handle(GET_SENSOR_STATE)
> +#define RTAS_FN_GET_TERM_CHAR
> rtas_fn_handle(GET_TERM_CHAR)
> +#define RTAS_FN_GET_TIME_OF_DAY
> rtas_fn_handle(GET_TIME_OF_DAY)
> +#define RTAS_FN_IBM_ACTIVATE_FIRMWARE
> rtas_fn_handle(IBM_ACTIVATE_FIRMWARE)
> +#define RTAS_FN_IBM_CBE_START_PTCAL
> rtas_fn_handle(IBM_CBE_START_PTCAL)
> +#define RTAS_FN_IBM_CBE_STOP_PTCAL
> rtas_fn_handle(IBM_CBE_STOP_PTCAL)
> +#define RTAS_FN_IBM_CHANGE_MSI
> rtas_fn_handle(IBM_CHANGE_MSI)
> +#define RTAS_FN_IBM_CLOSE_ERRINJCT
> rtas_fn_handle(IBM_CLOSE_ERRINJCT)
> +#define RTAS_FN_IBM_CONFIGURE_BRIDGE
> rtas_fn_handle(IBM_CONFIGURE_BRIDGE)
> +#define RTAS_FN_IBM_CONFIGURE_CONNECTOR
> rtas_fn_handle(IBM_CONFIGURE_CONNECTOR)
> +#define RTAS_FN_IBM_CONFIGURE_KERNEL_DUMP
> rtas_fn_handle(IBM_CONFIGURE_KERNEL_DUMP)
> +#define RTAS_FN_IBM_CONFIGURE_PE
> rtas_fn_handle(IBM_CONFIGURE_PE)
> +#define RTAS_FN_IBM_CREATE_PE_DMA_WINDOW
> rtas_fn_handle(IBM_CREATE_PE_DMA_WINDOW)
> +#define RTAS_FN_IBM_DISPLAY_MESSAGE
> rtas_fn_handle(IBM_DISPLAY_MESSAGE)
> +#define RTAS_FN_IBM_ERRINJCT
> rtas_fn_handle(IBM_ERRINJCT)
> +#define RTAS_FN_IBM_EXTI2Crtas_fn_handle(IBM_EXTI2C)
> +#define RTAS_FN_IBM_GET_CONFIG_ADDR_INFO
> rtas_fn_handle(IBM_GET_CONFIG_ADDR_INFO)
> +#define RTAS_FN_IBM_GET_CONFIG_ADDR_INFO2
> rtas_fn_handle(IBM_GET_CONFIG_ADDR_INFO2)
> +#define RTAS_FN_IBM_GET_DYNAMIC_SENSOR_STATE
> rtas_fn_handle(IBM_GET_DYNAMIC_SENSOR_STATE)
> +#define RTAS_FN_IBM_GET_INDICES
> rtas_fn_handle(IBM_GET_INDICES)
> +#define RTAS_FN_IBM_GET_RIO_TOPOLOGY
> rtas_fn_handle(IBM_GET_RIO_TOPOLOGY)
> +#define RTAS_FN_IBM_GET_SYSTEM_PARAMETER
> rtas_fn_handle(IBM_GET_SYSTEM_PARAMETER)
> +#define RTAS_FN_IBM_GET_VPD rtas_fn_handle(IBM_GET_VPD)
> +#define RTAS_FN_IBM_GET_XIVE
> rtas_fn_handle(IBM_GET_XIVE)
> +#define RTAS_FN_IBM_INT_OFF rtas_fn_handle(IBM_INT_OFF)
> +#define
Re: [PATCH 6/7] KVM: arm64: Change return type of kvm_vm_ioctl_mte_copy_tags() to "int"
On 08/02/2023 08:49, Cornelia Huck wrote:
> On Wed, Feb 08 2023, Gavin Shan wrote:
>
>> On 2/7/23 9:09 PM, Thomas Huth wrote:
>>> Oh, drat, I thought I had checked all return statements ... this must have
>>> fallen through the cracks, sorry!
>>>
>>> Anyway, this is already a problem now: The function is called from
>>> kvm_arch_vm_ioctl() (which still returns a long), which in turn is called
>>> from kvm_vm_ioctl() in virt/kvm/kvm_main.c. And that functions stores the
>>> return value in an "int r" variable. So the upper bits are already lost
>>> there.
Sorry about that, I was caught out by kvm_arch_vm_ioctl() returning long...
>>> Also, how is this supposed to work from user space? The normal "ioctl()"
>>> libc function just returns an "int" ? Is this ioctl already used in a
>>> userspace application somewhere? ... at least in QEMU, I didn't spot it
>>> yet...
>>>
>
> We will need it in QEMU to implement migration with MTE (the current
> proposal simply adds a migration blocker when MTE is enabled, as there
> are various other things that need to be figured out for this to work.)
> But maybe other VMMs already use it (and have been lucky because they
> always dealt with shorter lengths?)
>
>>
>> The ioctl command KVM_ARM_MTE_COPY_TAGS was merged recently and not used
>> by QEMU yet. I think struct kvm_arm_copy_mte_tags::length needs to be
>> '__u32' instead of '__u64' in order to standardize the return value.
>> Something like below. Documentation/virt/kvm/api.rst::section-4.130
>> needs update accordingly.
>>
>> struct kvm_arm_copy_mte_tags {
>> __u64 guest_ipa;
>> __u32 pad;
>> __u32 length;
>> void __user *addr;
>> __u64 flags;
>> __u64 reserved[2];
>>};
>
> Can we do this in a more compatible way, as we are dealing with an API?
> Like returning -EINVAL if length is too big?
>
I agree the simplest fix for the problem is simply to reject any
lengths>INT_MAX:
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index cf4c495a4321..94aed7ce85c4 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -1032,6 +1032,13 @@ long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST)
return -EINVAL;
+ /*
+* ioctl returns int, so lengths above INT_MAX cannot be
+* represented in the return value
+*/
+ if (length > INT_MAX)
+ return -EINVAL;
+
if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK)
return -EINVAL;
This could also be fixed in a useable way by including a new flag which
returns the length in an output field of the ioctl structure. I'm
guessing a 2GB limit would be annoying to work around.
Steve
Re: [PATCH v2 11/19] powerpc/rtas: add work area allocator
Nathan Lynch via B4 Submission Endpoint
writes:
> diff --git a/arch/powerpc/include/asm/rtas-work-area.h
> b/arch/powerpc/include/asm/rtas-work-area.h
> new file mode 100644
> index ..76ccb039cc37
> --- /dev/null
> +++ b/arch/powerpc/include/asm/rtas-work-area.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef POWERPC_RTAS_WORK_AREA_H
> +#define POWERPC_RTAS_WORK_AREA_H
The usual style would be _ASM_POWERPC_RTAS_WORK_AREA_H.
> diff --git a/arch/powerpc/kernel/rtas-work-area.c
> b/arch/powerpc/kernel/rtas-work-area.c
> new file mode 100644
> index ..75950e13a0fe
> --- /dev/null
> +++ b/arch/powerpc/kernel/rtas-work-area.c
> @@ -0,0 +1,208 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#define pr_fmt(fmt) "rtas-work-area: " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +enum {
> + /*
> + * Ensure the pool is page-aligned.
> + */
> + RTAS_WORK_AREA_ARENA_ALIGN = PAGE_SIZE,
> +
> + RTAS_WORK_AREA_ARENA_SZ = SZ_256K,
> + /*
> + * The smallest known work area size is for ibm,get-vpd's
> + * location code argument, which is limited to 79 characters
> + * plus 1 nul terminator.
> + *
> + * PAPR+ 7.3.20 ibm,get-vpd RTAS Call
> + * PAPR+ 12.3.2.4 Converged Location Code Rules - Length Restrictions
> + */
> + RTAS_WORK_AREA_MIN_ALLOC_SZ = roundup_pow_of_two(80),
> + /*
> + * Don't let a single allocation claim the whole arena.
> + */
> + RTAS_WORK_AREA_MAX_ALLOC_SZ = RTAS_WORK_AREA_ARENA_SZ / 2,
> +};
> +
> +static struct rtas_work_area_allocator_state {
> + struct gen_pool *gen_pool;
> + char *arena;
> + struct mutex mutex; /* serializes allocations */
> + struct wait_queue_head wqh;
> + mempool_t descriptor_pool;
> + bool available;
> +} rwa_state_ = {
> + .mutex = __MUTEX_INITIALIZER(rwa_state_.mutex),
> + .wqh = __WAIT_QUEUE_HEAD_INITIALIZER(rwa_state_.wqh),
> +};
> +static struct rtas_work_area_allocator_state *rwa_state = _state_;
I assumed the pointer was so you could swap this out at runtime or
something, but I don't think you do.
Any reason not to drop the pointer and just use rwa_state.foo accessors?
That would also allow the struct to be anonymous.
Or if you have the pointer you can at least make it NULL prior to init
and avoid the need for "available".
> +/*
> + * A single work area buffer and descriptor to serve requests early in
> + * boot before the allocator is fully initialized.
> + */
> +static bool early_work_area_in_use __initdata;
> +static char early_work_area_buf[SZ_4K] __initdata;
That should be page aligned I think?
> +static struct rtas_work_area early_work_area __initdata = {
> + .buf = early_work_area_buf,
> + .size = sizeof(early_work_area_buf),
> +};
> +
> +
> +static struct rtas_work_area * __init rtas_work_area_alloc_early(size_t size)
> +{
> + WARN_ON(size > early_work_area.size);
> + WARN_ON(early_work_area_in_use);
> + early_work_area_in_use = true;
> + memset(early_work_area.buf, 0, early_work_area.size);
> + return _work_area;
> +}
> +
> +static void __init rtas_work_area_free_early(struct rtas_work_area
> *work_area)
> +{
> + WARN_ON(work_area != _work_area);
> + WARN_ON(!early_work_area_in_use);
> + early_work_area_in_use = false;
> +}
> +
> +struct rtas_work_area * __ref rtas_work_area_alloc(size_t size)
> +{
> + struct rtas_work_area *area;
> + unsigned long addr;
> +
> + might_sleep();
> +
> + WARN_ON(size > RTAS_WORK_AREA_MAX_ALLOC_SZ);
> + size = min_t(size_t, size, RTAS_WORK_AREA_MAX_ALLOC_SZ);
This seems unsafe.
If you return a buffer smaller than the caller asks for they're likely
to read/write past the end of it and corrupt memory.
AFAIK genalloc doesn't have guard pages or anything fancy to save us
from that - but maybe I'm wrong, I've never used it.
There's only three callers in the end, seems like we should just return
NULL if the size is too large and have callers check the return value.
> + if (!rwa_state->available) {
> + area = rtas_work_area_alloc_early(size);
> + goto out;
> + }
> +
> + /*
> + * To ensure FCFS behavior and prevent a high rate of smaller
> + * requests from starving larger ones, use the mutex to queue
> + * allocations.
> + */
> + mutex_lock(_state->mutex);
> + wait_event(rwa_state->wqh,
> +(addr = gen_pool_alloc(rwa_state->gen_pool, size)) != 0);
> + mutex_unlock(_state->mutex);
> +
> + area = mempool_alloc(_state->descriptor_pool, GFP_KERNEL);
> + *area = (typeof(*area)){
> + .size = size,
> + .buf = (char *)addr,
> + };
That is an odd way to write that :)
> +out:
> + pr_devel("%ps -> %s() -> buf=%p size=%zu\n",
> + (void
Re: [PATCH v2 07/19] powerpc/rtas: improve function information lookups
Nathan Lynch via B4 Submission Endpoint
writes:
> From: Nathan Lynch
>
> The core RTAS support code and its clients perform two types of lookup
> for RTAS firmware function information.
>
...
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 479a95cb2770..14fe79217c26 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -16,6 +16,93 @@
> * Copyright (C) 2001 PPC 64 Team, IBM Corp
> */
>
> +#define rtas_fnidx(x_) RTAS_FNIDX__ ## x_
I'd prefer we just spelt it out in full, to aid grepability and
cscope/tags etc.
> +enum rtas_function_index {
> + rtas_fnidx(CHECK_EXCEPTION),
> + rtas_fnidx(DISPLAY_CHARACTER),
> + rtas_fnidx(EVENT_SCAN),
> + rtas_fnidx(FREEZE_TIME_BASE),
> + rtas_fnidx(GET_POWER_LEVEL),
> + rtas_fnidx(GET_SENSOR_STATE),
> + rtas_fnidx(GET_TERM_CHAR),
> + rtas_fnidx(GET_TIME_OF_DAY),
> + rtas_fnidx(IBM_ACTIVATE_FIRMWARE),
> + rtas_fnidx(IBM_CBE_START_PTCAL),
> + rtas_fnidx(IBM_CBE_STOP_PTCAL),
> + rtas_fnidx(IBM_CHANGE_MSI),
cheers
Re: [PATCH 3/3] ALSA: core: Make snd_card_free() return void
On Tue, Feb 07, 2023 at 08:19:07PM +0100, Uwe Kleine-König wrote: > The function returns 0 unconditionally. Make it return void instead and > simplify all callers accordingly. > > Signed-off-by: Uwe Kleine-König > --- > include/sound/core.h | 2 +- > sound/core/init.c | 6 ++ > sound/pci/hda/hda_tegra.c | 6 ++ > sound/ppc/snd_ps3.c | 4 +--- > 4 files changed, 6 insertions(+), 12 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: API for setting multiple PTEs at once
Hi Matthew, On 2/7/23 21:27, Matthew Wilcox wrote: On Thu, Feb 02, 2023 at 09:14:23PM +, Matthew Wilcox wrote: For those of you not subscribed, linux-mm is currently discussing how best to handle page faults on large folios. I simply made it work when adding large folio support. Now Yin Fengwei is working on making it fast. OK, here's an actual implementation: https://lore.kernel.org/linux-mm/20230207194937.122543-3-wi...@infradead.org/ It survives a run of xfstests. If your architecture doesn't store its PFNs at PAGE_SHIFT, you're going to want to implement your own set_ptes(), riscv stores its pfn at PAGE_PFN_SHIFT instead of PAGE_SHIFT, se we need to reimplement set_ptes. But I have been playing with your patchset and we never fall into the case where set_ptes is called with nr > 1, any idea why? I booted a large ubuntu defconfig and launched will_it_scale.page_fault4. I'll come up with the proper implementation of set_ptes anyway soon. Thanks, Alex or you'll see entirely the wrong pages mapped into userspace. You may also wish to implement set_ptes() if it can be done more efficiently than __pte(pteval(pte) + PAGE_SIZE). Architectures that implement things like flush_icache_page() and update_mmu_cache() may want to propose batched versions of those. That's alpha, csky, m68k, mips, nios2, parisc, sh, arm, loongarch, openrisc, powerpc, riscv, sparc and xtensa. Maintainers BCC'd, mailing lists CC'd. I'm happy to collect implementations and submit them as part of a v6. ___ linux-riscv mailing list linux-ri...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH v2 01/19] powerpc/rtas: handle extended delays safely in early boot
Nathan Lynch via B4 Submission Endpoint
writes:
> From: Nathan Lynch
>
> Some code that runs early in boot calls RTAS functions that can return
> -2 or 990x statuses, which mean the caller should retry. An example is
> pSeries_cmo_feature_init(), which invokes ibm,get-system-parameter but
> treats these benign statuses as errors instead of retrying.
>
> pSeries_cmo_feature_init() and similar code should be made to retry
> until they succeed or receive a real error, using the usual pattern:
>
> do {
> rc = rtas_call(token, etc...);
> } while (rtas_busy_delay(rc));
>
> But rtas_busy_delay() will perform a timed sleep on any 990x
> status. This isn't safe so early in boot, before the CPU scheduler and
> timer subsystem have initialized.
>
> The -2 RTAS status is much more likely to occur during single-threaded
> boot than 990x in practice, at least on PowerVM. This is because -2
> usually means that RTAS made progress but exhausted its self-imposed
> timeslice, while 990x is associated with concurrent requests from the
> OS causing internal contention. Regardless, according to the language
> in PAPR, the OS should be prepared to handle either type of status at
> any time.
>
> Add a fallback path to rtas_busy_delay() to handle this as safely as
> possible, performing a small delay on 990x. Include a counter to
> detect retry loops that aren't making progress and bail out.
>
> This was found by inspection and I'm not aware of any real
> failures. However, the implementation of rtas_busy_delay() before
> commit 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
> was not susceptible to this problem, so let's treat this as a
> regression.
>
> Signed-off-by: Nathan Lynch
> Fixes: 38f7b7067dae ("powerpc/rtas: rtas_busy_delay() improvements")
> ---
> arch/powerpc/kernel/rtas.c | 48
> +-
> 1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 795225d7f138..ec2df09a70cf 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -606,6 +606,46 @@ unsigned int rtas_busy_delay_time(int status)
> return ms;
> }
>
> +/*
> + * Early boot fallback for rtas_busy_delay().
> + */
> +static bool __init rtas_busy_delay_early(int status)
> +{
> + static size_t successive_ext_delays __initdata;
> + bool ret;
I think the logic would be easier to read if this was called "wait", but
maybe that's just me.
> + switch (status) {
> + case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> + /*
> + * In the unlikely case that we receive an extended
> + * delay status in early boot, the OS is probably not
> + * the cause, and there's nothing we can do to clear
> + * the condition. Best we can do is delay for a bit
> + * and hope it's transient. Lie to the caller if it
> + * seems like we're stuck in a retry loop.
> + */
> + mdelay(1);
> + ret = true;
> + successive_ext_delays += 1;
> + if (successive_ext_delays > 1000) {
> + pr_err("too many extended delays, giving up\n");
> + dump_stack();
> + ret = false;
Shouldn't we zero successive_ext_delays here?
Otherwise a subsequent (possibly different) RTAS call will immediately
fail out here if it gets a single extended delay from RTAS, won't it?
> + }
> + break;
> + case RTAS_BUSY:
> + ret = true;
> + successive_ext_delays = 0;
> + break;
> + default:
> + ret = false;
> + successive_ext_delays = 0;
> + break;
> + }
> +
> + return ret;
> +}
> +
> /**
> * rtas_busy_delay() - helper for RTAS busy and extended delay statuses
> *
> @@ -624,11 +664,17 @@ unsigned int rtas_busy_delay_time(int status)
> * * false - @status is not @RTAS_BUSY nor an extended delay hint. The
> * caller is responsible for handling @status.
> */
> -bool rtas_busy_delay(int status)
> +bool __ref rtas_busy_delay(int status)
Can you explain the __ref in the change log.
> {
> unsigned int ms;
> bool ret;
>
> + /*
> + * Can't do timed sleeps before timekeeping is up.
> + */
> + if (system_state < SYSTEM_SCHEDULING)
> + return rtas_busy_delay_early(status);
> +
> switch (status) {
> case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> ret = true;
>
cheers
Re: [PATCH v4 7/7] powerpc: kcsan: Add KCSAN Support
On Wed, 8 Feb 2023 at 04:23, Rohan McLure wrote: > > Enable HAVE_ARCH_KCSAN on all powerpc platforms, permitting use of the > kernel concurrency sanitiser through the CONFIG_KCSAN_* kconfig options. > KCSAN requires compiler builtins __atomic_* 64-bit values, and so only > report support on PPC64. I thought the stubs solve that? In general the whole builtin atomic support is only there to support some odd corner cases today (Clang GCOV generates builtin atomic ops, occasionally some driver sneaks in a builtin atomic although it's technically strongly discouraged, and probably Rust stuff in future). So I'd like to keep them, although they shouldn't cause issues. > See documentation in Documentation/dev-tools/kcsan.rst for more > information. Do the tests pass? > Signed-off-by: Rohan McLure > --- > v3: Restrict support to 64-bit, as TSAN expects 64-bit __atomic_* compiler > built-ins. > --- > arch/powerpc/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index b8c4ac56bddc..55bc2d724c73 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -198,6 +198,7 @@ config PPC > select HAVE_ARCH_KASAN if PPC_RADIX_MMU > select HAVE_ARCH_KASAN if PPC_BOOK3E_64 > select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > + select HAVE_ARCH_KCSANif PPC64 > select HAVE_ARCH_KFENCE if > ARCH_SUPPORTS_DEBUG_PAGEALLOC > select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET > select HAVE_ARCH_KGDB > -- > 2.37.2 >
Re: [PATCH 0/3] ALSA: core: Make some functions return void
Hello, On Wed, Feb 08, 2023 at 05:33:48PM +0900, Takashi Sakamoto wrote: > On Tue, Feb 07, 2023 at 08:19:04PM +0100, Uwe Kleine-König wrote: > > Hello, > > > > while checking in which cases hda_tegra_remove() can return a non-zero > > value, I > > found that actually cannot happen. This series makes the involved functions > > return void to make this obvious. > > > > This is a preparation for making platform_driver::remove return void, too. > > > > Best regards > > Uwe > > > > Uwe Kleine-König (3): > > ALSA: core: Make snd_card_disconnect() return void > > ALSA: core: Make snd_card_free_when_closed() return void > > ALSA: core: Make snd_card_free() return void > > > > include/sound/core.h | 6 +++--- > > sound/core/init.c | 40 ++- > > sound/pci/hda/hda_tegra.c | 6 ++ > > sound/ppc/snd_ps3.c | 4 +--- > > 4 files changed, 20 insertions(+), 36 deletions(-) > > All of the changes in the patches look good to me, as the return value > seems not to be so effective for a long time (15 years or more) and > expected so for future. To give a more complete picture: Returning an error code in a cleanup function does active harm. It makes driver authors expect that there must be error handing and that they can/should return that error from the remove function. This is wrong however and likely yields resource leaks. See bbc126ae381cf0a27822c1f822d0aeed74cc40d9 for an example. > Reviewed-by: Takashi Sakamoto > > One of my concern is that we are mostly in the last week for v6.3 > development. When taking influence of the changes into account, it > would be possible to postpone to v6.4 development. But it's up to the > maintainer. There is no rush from my side. Eventually I want to modify platform_driver::remove which depends on this patch series, but the 6.4 merge window is fine for me, as this effort will likely take some more time. > > base-commit: 05ecb680708a1dbe6554d6fc17e5d9a8a7cb5e6a > > A small nitpicking. I think you use Linus' tree or so: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=05ecb680708a > > Because the hash is not reachable from the branch for next merge window on > the tree of sound subsystem upstream: > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=for-next=05ecb680708a > > I guess it is safer to use his tree as your work-base, even > if the three patches can be applied to it as is (fortunately). I see your point. My reason not to do that is that finding out the right tree (and branch) to base a patch on is not always trivial. As someone who sends patches touching the whole tree this is a considerable overhead and most of the time it's not worth the effort in my experience. Even if I had based my patch on tiwai's tree, there might be a patch still in flux that is picked up first and conflicts with my change. Most of the time the patch still applies and stating the base is just for the benefit of the auto builders (which might have problems finding the base commit if it's not in next) and if it doesn't apply the person picking up the patch probably knows about Linus' tree and so git is helpful resolving the resulting conflict. It's still more complicated for patches that might or might not be considered a fix. Then it's up to the maintainer if they apply it to their fixes or next branch. (And that situation is just about to happen as I found a problem in a driver I touched here. So stay tuned :-) So I gave up thinking too much about the right base and take the opportunistic route of just using Linus' tree (or the last -rc1) and if there really a merge conflict pops up, support the maintainer only then. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH 6/7] KVM: arm64: Change return type of kvm_vm_ioctl_mte_copy_tags() to "int"
On Wed, Feb 08 2023, Gavin Shan wrote:
> On 2/7/23 9:09 PM, Thomas Huth wrote:
>> Oh, drat, I thought I had checked all return statements ... this must have
>> fallen through the cracks, sorry!
>>
>> Anyway, this is already a problem now: The function is called from
>> kvm_arch_vm_ioctl() (which still returns a long), which in turn is called
>> from kvm_vm_ioctl() in virt/kvm/kvm_main.c. And that functions stores the
>> return value in an "int r" variable. So the upper bits are already lost
>> there.
>>
>> Also, how is this supposed to work from user space? The normal "ioctl()"
>> libc function just returns an "int" ? Is this ioctl already used in a
>> userspace application somewhere? ... at least in QEMU, I didn't spot it
>> yet...
>>
We will need it in QEMU to implement migration with MTE (the current
proposal simply adds a migration blocker when MTE is enabled, as there
are various other things that need to be figured out for this to work.)
But maybe other VMMs already use it (and have been lucky because they
always dealt with shorter lengths?)
>
> The ioctl command KVM_ARM_MTE_COPY_TAGS was merged recently and not used
> by QEMU yet. I think struct kvm_arm_copy_mte_tags::length needs to be
> '__u32' instead of '__u64' in order to standardize the return value.
> Something like below. Documentation/virt/kvm/api.rst::section-4.130
> needs update accordingly.
>
> struct kvm_arm_copy_mte_tags {
> __u64 guest_ipa;
> __u32 pad;
> __u32 length;
> void __user *addr;
> __u64 flags;
> __u64 reserved[2];
>};
Can we do this in a more compatible way, as we are dealing with an API?
Like returning -EINVAL if length is too big?
Re: [PATCH 0/3] ALSA: core: Make some functions return void
Hi, On Tue, Feb 07, 2023 at 08:19:04PM +0100, Uwe Kleine-König wrote: > Hello, > > while checking in which cases hda_tegra_remove() can return a non-zero value, > I > found that actually cannot happen. This series makes the involved functions > return void to make this obvious. > > This is a preparation for making platform_driver::remove return void, too. > > Best regards > Uwe > > Uwe Kleine-König (3): > ALSA: core: Make snd_card_disconnect() return void > ALSA: core: Make snd_card_free_when_closed() return void > ALSA: core: Make snd_card_free() return void > > include/sound/core.h | 6 +++--- > sound/core/init.c | 40 ++- > sound/pci/hda/hda_tegra.c | 6 ++ > sound/ppc/snd_ps3.c | 4 +--- > 4 files changed, 20 insertions(+), 36 deletions(-) All of the changes in the patches look good to me, as the return value seems not to be so effective for a long time (15 years or more) and expected so for future. Reviewed-by: Takashi Sakamoto One of my concern is that we are mostly in the last week for v6.3 development. When taking influence of the changes into account, it would be possible to postpone to v6.4 development. But it's up to the maintainer. > base-commit: 05ecb680708a1dbe6554d6fc17e5d9a8a7cb5e6a A small nitpicking. I think you use Linus' tree or so: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=05ecb680708a Because the hash is not reachable from the branch for next merge window on the tree of sound subsystem upstream: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=for-next=05ecb680708a I guess it is safer to use his tree as your work-base, even if the three patches can be applied to it as is (fortunately). Regards Takashi Sakamoto
