Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On 04. 04. 24, 0:29, Andy Shevchenko wrote: Cc: Benjamin Herrenschmidt Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Christophe Leroy Cc: "Aneesh Kumar K.V" Cc: "Naveen N. Rao" Cc: linux-m...@lists.linux-m68k.org Second, please move these Cc to be after the '---' line Sorry, but why? -- js suse labs
[PATCH 2/2] powerpc/64: Only warn for kuap locked when KCSAN not present
Arbitrary instrumented locations, including syscall handlers, can call arch_local_irq_restore() transitively when KCSAN is enabled, and in turn also replay_soft_interrupts_irqrestore(). The precondition on entry to this routine that is checked is that KUAP is enabled (user access prohibited). Failure to meet this condition only triggers a warning however, and afterwards KUAP is enabled anyway. That is, KUAP being disabled on entry is in fact permissable, but not possible on an uninstrumented kernel. Disable this assertion only when KCSAN is enabled. Suggested-by: Nicholas Piggin Signed-off-by: Rohan McLure --- arch/powerpc/kernel/irq_64.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/irq_64.c b/arch/powerpc/kernel/irq_64.c index d5c48d1b0a31..18b2048389a2 100644 --- a/arch/powerpc/kernel/irq_64.c +++ b/arch/powerpc/kernel/irq_64.c @@ -189,7 +189,8 @@ static inline __no_kcsan void replay_soft_interrupts_irqrestore(void) * and re-locking AMR but we shouldn't get here in the first place, * hence the warning. */ - kuap_assert_locked(); + if (!IS_ENABLED(CONFIG_KCSAN)) + kuap_assert_locked(); if (kuap_state != AMR_KUAP_BLOCKED) set_kuap(AMR_KUAP_BLOCKED); -- 2.44.0
[PATCH 1/2] powerpc: Apply __always_inline to interrupt_{enter,exit}_prepare()
In keeping with the advice given by Documentation/core-api/entry.rst,
entry and exit handlers for interrupts should not be instrumented.
Guarantee that the interrupt_{enter,exit}_prepare() routines are inlined
so that they will inheret instrumentation from their caller.
KCSAN kernels were observed to compile without inlining these routines,
which would lead to grief on NMI handlers.
Signed-off-by: Rohan McLure
---
arch/powerpc/include/asm/interrupt.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/interrupt.h
b/arch/powerpc/include/asm/interrupt.h
index 7b610864b364..f4343e0bfb13 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -150,7 +150,7 @@ static inline void booke_restore_dbcr0(void)
#endif
}
-static inline void interrupt_enter_prepare(struct pt_regs *regs)
+static __always_inline void interrupt_enter_prepare(struct pt_regs *regs)
{
#ifdef CONFIG_PPC64
irq_soft_mask_set(IRQS_ALL_DISABLED);
@@ -215,11 +215,11 @@ static inline void interrupt_enter_prepare(struct pt_regs
*regs)
* However interrupt_nmi_exit_prepare does return directly to regs, because
* NMIs do not do "exit work" or replay soft-masked interrupts.
*/
-static inline void interrupt_exit_prepare(struct pt_regs *regs)
+static __always_inline void interrupt_exit_prepare(struct pt_regs *regs)
{
}
-static inline void interrupt_async_enter_prepare(struct pt_regs *regs)
+static __always_inline void interrupt_async_enter_prepare(struct pt_regs *regs)
{
#ifdef CONFIG_PPC64
/* Ensure interrupt_enter_prepare does not enable MSR[EE] */
@@ -238,7 +238,7 @@ static inline void interrupt_async_enter_prepare(struct
pt_regs *regs)
irq_enter();
}
-static inline void interrupt_async_exit_prepare(struct pt_regs *regs)
+static __always_inline void interrupt_async_exit_prepare(struct pt_regs *regs)
{
/*
* Adjust at exit so the main handler sees the true NIA. This must
@@ -278,7 +278,7 @@ static inline bool nmi_disables_ftrace(struct pt_regs *regs)
return true;
}
-static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct
interrupt_nmi_state *state)
+static __always_inline void interrupt_nmi_enter_prepare(struct pt_regs *regs,
struct interrupt_nmi_state *state)
{
#ifdef CONFIG_PPC64
state->irq_soft_mask = local_paca->irq_soft_mask;
@@ -340,7 +340,7 @@ static inline void interrupt_nmi_enter_prepare(struct
pt_regs *regs, struct inte
nmi_enter();
}
-static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct
interrupt_nmi_state *state)
+static __always_inline void interrupt_nmi_exit_prepare(struct pt_regs *regs,
struct interrupt_nmi_state *state)
{
if (mfmsr() & MSR_DR) {
// nmi_exit if relocations are on
--
2.44.0
[PATCH] asm-generic/mmiowb: Mark accesses to fix KCSAN warnings
Prior to this patch, data races are detectable by KCSAN of the following
forms:
[1] Asynchronous calls to mmiowb_set_pending() from an interrupt context
or otherwise outside of a critical section
[2] Interrupted critical sections, where the interrupt will itself
acquire a lock
In case [1], calling context does not need an mmiowb() call to be
issued, otherwise it would do so itself. Such calls to
mmiowb_set_pending() are either idempotent or no-ops.
In case [2], irrespective of when the interrupt occurs, the interrupt
will acquire and release its locks prior to its return, nesting_count
will continue balanced. In the worst case, the interrupted critical
section during a mmiowb_spin_unlock() call observes an mmiowb to be
pending and afterward is interrupted, leading to an extraneous call to
mmiowb(). This data race is clearly innocuous.
Resolve KCSAN warnings of type [1] by means of READ_ONCE, WRITE_ONCE.
As increments and decrements to nesting_count are balanced by interrupt
contexts, resolve type [2] warnings by simply revoking instrumentation,
with data_race() rather than READ_ONCE() and WRITE_ONCE(), the memory
consistency semantics of plain-accesses will still lead to correct
behaviour.
Signed-off-by: Rohan McLure
Reported-by: Michael Ellerman
Reported-by: Gautam Menghani
Tested-by: Gautam Menghani
Acked-by: Arnd Bergmann
---
Previously discussed here:
https://lore.kernel.org/linuxppc-dev/20230510033117.1395895-4-rmcl...@linux.ibm.com/
But pushed back due to affecting other architectures. Reissuing, to
linuxppc-dev, as it does not enact a functional change.
---
include/asm-generic/mmiowb.h | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/include/asm-generic/mmiowb.h b/include/asm-generic/mmiowb.h
index 5698fca3bf56..f8c7c8a84e9e 100644
--- a/include/asm-generic/mmiowb.h
+++ b/include/asm-generic/mmiowb.h
@@ -37,25 +37,28 @@ static inline void mmiowb_set_pending(void)
struct mmiowb_state *ms = __mmiowb_state();
if (likely(ms->nesting_count))
- ms->mmiowb_pending = ms->nesting_count;
+ WRITE_ONCE(ms->mmiowb_pending, ms->nesting_count);
}
static inline void mmiowb_spin_lock(void)
{
struct mmiowb_state *ms = __mmiowb_state();
- ms->nesting_count++;
+
+ /* Increment need not be atomic. Nestedness is balanced over
interrupts. */
+ data_race(ms->nesting_count++);
}
static inline void mmiowb_spin_unlock(void)
{
struct mmiowb_state *ms = __mmiowb_state();
+ u16 pending = READ_ONCE(ms->mmiowb_pending);
- if (unlikely(ms->mmiowb_pending)) {
- ms->mmiowb_pending = 0;
+ WRITE_ONCE(ms->mmiowb_pending, 0);
+ if (unlikely(pending))
mmiowb();
- }
- ms->nesting_count--;
+ /* Decrement need not be atomic. Nestedness is balanced over
interrupts. */
+ data_race(ms->nesting_count--);
}
#else
#define mmiowb_set_pending() do { } while (0)
--
2.44.0
Re: [PATCH v3 15/15] powerpc: Add support for suppressing warning backtraces
Guenter Roeck writes: > Add name of functions triggering warning backtraces to the __bug_table > object section to enable support for suppressing WARNING backtraces. > > To limit image size impact, the pointer to the function name is only added > to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and > CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly > parameter is replaced with a (dummy) NULL parameter to avoid an image size > increase due to unused __func__ entries (this is necessary because __func__ > is not a define but a virtual variable). > > Tested-by: Linux Kernel Functional Testing > Acked-by: Dan Carpenter > Cc: Michael Ellerman > Signed-off-by: Guenter Roeck > --- > v2: > - Rebased to v6.9-rc1 > - Added Tested-by:, Acked-by:, and Reviewed-by: tags > - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option > v3: > - Rebased to v6.9-rc2 > > arch/powerpc/include/asm/bug.h | 37 +- > 1 file changed, 28 insertions(+), 9 deletions(-) I ran it through some build and boot tests, LGTM. Acked-by: Michael Ellerman (powerpc) cheers
Re: [PATCH v3 06/15] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests
On Wed, 3 Apr 2024 06:19:27 -0700 Guenter Roeck wrote: > dev_addr_lists_test generates lock warning noise at the end of tests > if lock debugging is enabled. There are two sets of warnings. > > WARNING: CPU: 0 PID: 689 at kernel/locking/mutex.c:923 > __mutex_unlock_slowpath.constprop.0+0x13c/0x368 > DEBUG_LOCKS_WARN_ON(__owner_task(owner) != __get_current()) > > WARNING: kunit_try_catch/1336 still has locks held! > > KUnit test cleanup is not guaranteed to run in the same thread as the test > itself. For this test, this means that rtnl_lock() and rtnl_unlock() may > be called from different threads. This triggers the warnings. > Suppress the warnings because they are irrelevant for the test and just > confusing and distracting. > > The first warning can be suppressed by using START_SUPPRESSED_WARNING() > and END_SUPPRESSED_WARNING() around the call to rtnl_unlock(). To suppress > the second warning, it is necessary to set debug_locks_silent while the > rtnl lock is held. Is it okay if I move the locking into the tests, instead? It's only 4 lines more and no magic required, seems to work fine.
Re: [PATCH v12 11/11] powerpc: mm: Support page table check
On Tue, Apr 2, 2024 at 1:13 AM Rohan McLure wrote:
>
> On creation and clearing of a page table mapping, instrument such calls
> by invoking page_table_check_pte_set and page_table_check_pte_clear
> respectively. These calls serve as a sanity check against illegal
> mappings.
>
> Enable ARCH_SUPPORTS_PAGE_TABLE_CHECK for all platforms.
>
> See also:
>
> riscv support in commit 3fee229a8eb9 ("riscv/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> arm64 in commit 42b2547137f5 ("arm64/mm: enable
> ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> x86_64 in commit d283d422c6c4 ("x86: mm: add x86_64 support for page table
> check")
>
> Reviewed-by: Christophe Leroy
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
Thank you for this work!
Pasha
Re: [PATCH v12 10/11] powerpc: mm: Use set_pte_at_unchecked() for early-boot / internal usages
On Tue, Apr 2, 2024 at 1:13 AM Rohan McLure wrote:
>
> In the new set_ptes() API, set_pte_at() (a special case of set_ptes())
> is intended to be instrumented by the page table check facility. There
> are however several other routines that constitute the API for setting
> page table entries, including set_pmd_at() among others. Such routines
> are themselves implemented in terms of set_ptes_at().
>
> A future patch providing support for page table checking on powerpc
> must take care to avoid duplicate calls to
> page_table_check_p{te,md,ud}_set(). Allow for assignment of pte entries
> without instrumentation through the set_pte_at_unchecked() routine
> introduced in this patch.
>
> Cause API-facing routines that call set_pte_at() to instead call
> set_pte_at_unchecked(), which will remain uninstrumented by page
> table check. set_ptes() is itself implemented by calls to
> __set_pte_at(), so this eliminates redundant code.
>
> Also prefer set_pte_at_unchecked() in early-boot usages which should not be
> instrumented.
Would not the early-boot usage be all kernel mappings that are ignored
by page table check anways? Sounds like it is better to only use the
set_pte_at_unchecked() version where it is really needed, which is to
avoid double counting. This will keep the usage of this function only
for one purpose that is easy to follow.
Pasha
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
On Thu, 4 Apr 2024, Andy Shevchenko wrote:
> ...
>
> First of all, please read this
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
> and amend the commit message accordingly.
>
Right -- the call chain is described in the commit log message so the
backtrace does not add value. And the timestamps, stack dump etc. are
irrelevant.
> > Cc: Benjamin Herrenschmidt
> > Cc: Michael Ellerman
> > Cc: Nicholas Piggin
> > Cc: Christophe Leroy
> > Cc: "Aneesh Kumar K.V"
> > Cc: "Naveen N. Rao"
> > Cc: linux-m...@lists.linux-m68k.org
>
> Second, please move these Cc to be after the '---' line
>
I thought they were placed above the line for audit (and signing)
purposes. There are thousands of Cc lines in the mainline commit messages
since v6.8.
> > Link: https://github.com/vivier/qemu-m68k/issues/44
> > Link: https://lore.kernel.org/all/1078874617.9746.36.camel@gaston/
>
> Missed Fixes tag?
>
Would this be ok: Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
I have to ask because some reviewers do not like to see a Fixes tag cite
that commit.
> > Signed-off-by: Finn Thain
> > ---
> (here is a good location for Cc:)
>
Documentation/process/submitting-patches.rst indicats that it should be
above the "---" separator together with Acked-by etc. Has this convention
changed recently?
Thanks for your review.
Re: [PATCH v12 09/11] poweprc: mm: Implement *_user_accessible_page() for ptes
On Tue, Apr 2, 2024 at 1:13 AM Rohan McLure wrote:
>
> Page table checking depends on architectures providing an
> implementation of p{te,md,ud}_user_accessible_page. With
> refactorisations made on powerpc/mm, the pte_access_permitted() and
> similar methods verify whether a userland page is accessible with the
> required permissions.
>
> Since page table checking is the only user of
> p{te,md,ud}_user_accessible_page(), implement these for all platforms,
> using some of the same preliminary checks taken by pte_access_permitted()
> on that platform.
>
> Since Commit 8e9bd41e4ce1 ("powerpc/nohash: Replace pte_user() by pte_read()")
> pte_user() is no longer required to be present on all platforms as it
> may be equivalent to or implied by pte_read(). Hence implementations of
> pte_user_accessible_page() are specialised.
>
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
Re: [PATCH v12 08/11] powerpc: mm: Add pud_pfn() stub
On Tue, Apr 2, 2024 at 1:11 AM Rohan McLure wrote: > > The page table check feature requires that pud_pfn() be defined > on each consuming architecture. Since only 64-bit, Book3S platforms > allow for hugepages at this upper level, and since the calling code is > gated by a call to pud_user_accessible_page(), which will return zero, > include this stub as a BUILD_BUG(). > > Signed-off-by: Rohan McLure Reviewed-by: Pasha Tatashin
Re: [PATCH v12 07/11] mm: Provide address parameter to p{te,md,ud}_user_accessible_page()
On Tue, Apr 2, 2024 at 1:13 AM Rohan McLure wrote: > > On several powerpc platforms, a page table entry may not imply whether > the relevant mapping is for userspace or kernelspace. Instead, such > platforms infer this by the address which is being accessed. > > Add an additional address argument to each of these routines in order to > provide support for page table check on powerpc. > > Signed-off-by: Rohan McLure Reviewed-by: Pasha Tatashin
Re: [PATCH v12 06/11] mm/page_table_check: Reinstate address parameter in [__]page_table_check_pte_clear()
On Tue, Apr 2, 2024 at 1:11 AM Rohan McLure wrote:
>
> This reverts commit aa232204c468 ("mm/page_table_check: remove unused
> parameter in [__]page_table_check_pte_clear").
>
> Reinstate previously unused parameters for the purpose of supporting
> powerpc platforms, as many do not encode user/kernel ownership of the
> page in the pte, but instead in the address of the access.
>
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
Re: [PATCH v12 05/11] mm/page_table_check: Reinstate address parameter in [__]page_table_check_pmd_clear()
On Tue, Apr 2, 2024 at 1:11 AM Rohan McLure wrote:
>
> This reverts commit 1831414cd729 ("mm/page_table_check: remove unused
> parameter in [__]page_table_check_pmd_clear").
>
> Reinstate previously unused parameters for the purpose of supporting
> powerpc platforms, as many do not encode user/kernel ownership of the
> page in the pte, but instead in the address of the access.
>
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
Re: [PATCH v12 00/11] Support page table check PowerPC
All patches in this series should be also CC'd to linux-ker...@vger.kernel.org.
Re: [PATCH v3] scsi: sg: Avoid race in error handling & drop bogus warn
On 4/1/24 12:10 PM, Alexander Wetzel wrote: @@ -301,11 +302,12 @@ sg_open(struct inode *inode, struct file *filp) /* This driver's module count bumped by fops_get in */ /* Prevent the device driver from vanishing while we sleep */ - retval = scsi_device_get(sdp->device); + device = sdp->device; + retval = scsi_device_get(device); if (retval) goto sg_put; Are all the sdp->device -> device changes essential? Isn't there a preference to minimize patches that will end up in the stable trees? Thanks, Bart.
Re: [PATCH v12 04/11] mm/page_table_check: Reinstate address parameter in [__]page_table_check_pud_clear()
On Tue, Apr 2, 2024 at 1:13 AM Rohan McLure wrote:
>
> This reverts commit 931c38e16499 ("mm/page_table_check: remove unused
> parameter in [__]page_table_check_pud_clear").
>
> Reinstate previously unused parameters for the purpose of supporting
> powerpc platforms, as many do not encode user/kernel ownership of the
> page in the pte, but instead in the address of the access.
>
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
Re: [PATCH v12 03/11] mm/page_table_check: Provide addr parameter to page_table_check_pte_set()
On Tue, Apr 2, 2024 at 1:11 AM Rohan McLure wrote: > > To provide support for powerpc platforms, provide an addr parameter to > the page_table_check_pte_set() routine. This parameter is needed on some > powerpc platforms which do not encode whether a mapping is for user or > kernel in the pte. On such platforms, this can be inferred form the > addr parameter. > > Signed-off-by: Rohan McLure Reviewed-by: Pasha Tatashin
Re: [PATCH v12 02/11] mm/page_table_check: Reinstate address parameter in [__]page_table_check_pmd_set()
On Tue, Apr 2, 2024 at 1:13 AM Rohan McLure wrote:
>
> This reverts commit a3b837130b58 ("mm/page_table_check: remove unused
> parameter in [__]page_table_check_pmd_set").
>
> Reinstate previously unused parameters for the purpose of supporting
> powerpc platforms, as many do not encode user/kernel ownership of the
> page in the pte, but instead in the address of the access.
>
> riscv: Respect change to delete mm, addr parameters from __set_pte_at()
>
> This commit also changed calls to __set_pte_at() to use fewer parameters
> on riscv. Keep that change rather than reverting it, as the signature of
> __set_pte_at() is changed in a different commit.
>
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
Re: [PATCH v12 01/11] mm/page_table_check: Reinstate address parameter in [__]page_table_check_pud_set()
On Tue, Apr 2, 2024 at 1:11 AM Rohan McLure wrote:
>
> This reverts commit 6d144436d954 ("mm/page_table_check: remove unused
> parameter in [__]page_table_check_pud_set").
>
> Reinstate previously unused parameters for the purpose of supporting
> powerpc platforms, as many do not encode user/kernel ownership of the
> page in the pte, but instead in the address of the access.
>
> riscv: Respect change to delete mm, addr parameters from __set_pte_at()
>
> This commit also changed calls to __set_pte_at() to use fewer parameters
> on riscv. Keep that change rather than reverting it, as the signature of
> __set_pte_at() is changed in a different commit.
>
> Signed-off-by: Rohan McLure
Reviewed-by: Pasha Tatashin
For some reason this one was not delivered to the linux-mm mailing list.
Pasha
Re: [PATCH] serial/pmac_zilog: Remove flawed mitigation for rx irq flood
Thu, Mar 28, 2024 at 03:11:33PM +1100, Finn Thain kirjoitti: > The mitigation was intended to stop the irq completely. That might have > been better than a hard lock-up but it turns out that you get a crash > anyway if you're using pmac_zilog as a serial console. > > That's because the pr_err() call in pmz_receive_chars() results in > pmz_console_write() attempting to lock a spinlock already locked in > pmz_interrupt(). With CONFIG_DEBUG_SPINLOCK=y, this produces a fatal > BUG splat like the one below. (The spinlock at 0x62e140 is the one in > struct uart_port.) > > Even when it's not fatal, the serial port rx function ceases to work. > Also, the iteration limit doesn't play nicely with QEMU. Please see > bug report linked below. > > A web search for reports of the error message "pmz: rx irq flood" didn't > produce anything. So I don't think this code is needed any more. Remove it. > [ 14.56] ttyPZ0: pmz: rx irq flood ! > [ 14.56] BUG: spinlock recursion on CPU#0, swapper/0 > [ 14.56] lock: 0x62e140, .magic: dead4ead, .owner: swapper/0, > .owner_cpu: 0 > [ 14.56] CPU: 0 PID: 0 Comm: swapper Not tainted > 6.8.0-mac-dbg-preempt-4-g4143b7b9144a #1 > [ 14.56] Stack from 0059bcc4: > [ 14.56] 0059bcc4 0056316f 0056316f 2700 004b6444 0059bce4 > 004ad8c6 0056316f > [ 14.56] 0059bd10 004a6546 00556759 0062e140 dead4ead 0059f892 > > [ 14.56] 0062e140 0059bde8 005c03d0 0059bd24 0004daf6 0062e140 > 005567bf 0062e140 > [ 14.56] 0059bd34 004b64c2 0062e140 0001 0059bd50 002e15ea > 0062e140 0001 > [ 14.56] 0059bde7 0059bde8 005c03d0 0059bdac 0005124e 005c03d0 > 005cdc00 002b > [ 14.56] 005a3caa 005a3caa 0059bde8 0004ff00 0059be8b > 00038200 000529ba > [ 14.56] Call Trace: [<2700>] ret_from_kernel_thread+0xc/0x14 > [ 14.56] [<004b6444>] _raw_spin_lock+0x0/0x28 > [ 14.56] [<004ad8c6>] dump_stack+0x10/0x16 > [ 14.56] [<004a6546>] spin_dump+0x6e/0x7c > [ 14.56] [<0004daf6>] do_raw_spin_lock+0x9c/0xa6 > [ 14.56] [<004b64c2>] _raw_spin_lock_irqsave+0x2a/0x34 > [ 14.56] [<002e15ea>] pmz_console_write+0x32/0x9a > [ 14.56] [<0005124e>] console_flush_all+0x112/0x3a2 > [ 14.56] [<0004ff00>] console_trylock+0x0/0x7a > [ 14.56] [<00038200>] parameq+0x48/0x6e > [ 14.56] [<000529ba>] __printk_safe_enter+0x0/0x36 > [ 14.56] [<0005113c>] console_flush_all+0x0/0x3a2 > [ 14.56] [<000542c4>] prb_read_valid+0x0/0x1a > [ 14.56] [<004b65a4>] _raw_spin_unlock+0x0/0x38 > [ 14.56] [<0005151e>] console_unlock+0x40/0xb8 > [ 14.56] [<00038200>] parameq+0x48/0x6e > [ 14.56] [<002c778c>] __tty_insert_flip_string_flags+0x0/0x14e > [ 14.56] [<00051798>] vprintk_emit+0x156/0x238 > [ 14.56] [<00051894>] vprintk_default+0x1a/0x1e > [ 14.56] [<000529a8>] vprintk+0x74/0x86 > [ 14.56] [<004a6596>] _printk+0x12/0x16 > [ 14.56] [<002e23be>] pmz_receive_chars+0x1cc/0x394 > [ 14.56] [<004b6444>] _raw_spin_lock+0x0/0x28 > [ 14.56] [<00038226>] parse_args+0x0/0x3a6 > [ 14.56] [<004b6466>] _raw_spin_lock+0x22/0x28 > [ 14.56] [<002e26b4>] pmz_interrupt+0x12e/0x1e0 > [ 14.56] [<00048680>] arch_cpu_idle_enter+0x0/0x8 > [ 14.56] [<00054ebc>] __handle_irq_event_percpu+0x24/0x106 > [ 14.56] [<004ae576>] default_idle_call+0x0/0x46 > [ 14.56] [<00055020>] handle_irq_event+0x30/0x90 > [ 14.56] [<00058320>] handle_simple_irq+0x5e/0xc0 > [ 14.56] [<00048688>] arch_cpu_idle_exit+0x0/0x8 > [ 14.56] [<00054800>] generic_handle_irq+0x3c/0x4a > [ 14.56] [<2978>] do_IRQ+0x24/0x3a > [ 14.56] [<004ae508>] cpu_idle_poll.isra.0+0x0/0x6e > [ 14.56] [<2874>] auto_irqhandler_fixup+0x4/0xc > [ 14.56] [<004ae508>] cpu_idle_poll.isra.0+0x0/0x6e > [ 14.56] [<004ae576>] default_idle_call+0x0/0x46 > [ 14.56] [<004ae598>] default_idle_call+0x22/0x46 > [ 14.56] [<00048710>] do_idle+0x6a/0xf0 > [ 14.56] [<000486a6>] do_idle+0x0/0xf0 > [ 14.56] [<000367d2>] find_task_by_pid_ns+0x0/0x2a > [ 14.56] [<0005d064>] __rcu_read_lock+0x0/0x12 > [ 14.56] [<00048a5a>] cpu_startup_entry+0x18/0x1c > [ 14.56] [<00063a06>] __rcu_read_unlock+0x0/0x26 > [ 14.56] [<004ae65a>] kernel_init+0x0/0xfa > [ 14.56] [<0049c5a8>] strcpy+0x0/0x1e > [ 14.56] [<004a6584>] _printk+0x0/0x16 > [ 14.56] [<0049c72a>] strlen+0x0/0x22 > [ 14.56] [<006452d4>] memblock_alloc_try_nid+0x0/0x82 > [ 14.56] [<0063939a>] arch_post_acpi_subsys_init+0x0/0x8 > [ 14.56] [<0063991e>] console_on_rootfs+0x0/0x60 > [ 14.56] [<00638410>] _sinittext+0x410/0xadc > [ 14.56] First of all, please read this https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
Re: [PATCH v3 00/15] Add support for suppressing warning backtraces
On Wed, Apr 03, 2024 at 06:19:21AM -0700, Guenter Roeck wrote: > Some unit tests intentionally trigger warning backtraces by passing bad > parameters to kernel API functions. Such unit tests typically check the > return value from such calls, not the existence of the warning backtrace. > > Such intentionally generated warning backtraces are neither desirable > nor useful for a number of reasons. > - They can result in overlooked real problems. > - A warning that suddenly starts to show up in unit tests needs to be > investigated and has to be marked to be ignored, for example by > adjusting filter scripts. Such filters are ad-hoc because there is > no real standard format for warnings. On top of that, such filter > scripts would require constant maintenance. > > One option to address problem would be to add messages such as "expected > warning backtraces start / end here" to the kernel log. However, that > would again require filter scripts, it might result in missing real > problematic warning backtraces triggered while the test is running, and > the irrelevant backtrace(s) would still clog the kernel log. > > Solve the problem by providing a means to identify and suppress specific > warning backtraces while executing test code. Support suppressing multiple > backtraces while at the same time limiting changes to generic code to the > absolute minimum. Architecture specific changes are kept at minimum by > retaining function names only if both CONFIG_DEBUG_BUGVERBOSE and > CONFIG_KUNIT are enabled. > > The first patch of the series introduces the necessary infrastructure. > The second patch introduces support for counting suppressed backtraces. > This capability is used in patch three to implement unit tests. > Patch four documents the new API. > The next two patches add support for suppressing backtraces in drm_rect > and dev_addr_lists unit tests. These patches are intended to serve as > examples for the use of the functionality introduced with this series. > The remaining patches implement the necessary changes for all > architectures with GENERIC_BUG support. > > With CONFIG_KUNIT enabled, image size increase with this series applied is > approximately 1%. The image size increase (and with it the functionality > introduced by this series) can be avoided by disabling > CONFIG_KUNIT_SUPPRESS_BACKTRACE. > > This series is based on the RFC patch and subsequent discussion at > https://patchwork.kernel.org/project/linux-kselftest/patch/02546e59-1afe-4b08-ba81-d94f3b691c9a@moroto.mountain/ > and offers a more comprehensive solution of the problem discussed there. > > Design note: > Function pointers are only added to the __bug_table section if both > CONFIG_KUNIT_SUPPRESS_BACKTRACE and CONFIG_DEBUG_BUGVERBOSE are enabled > to avoid image size increases if CONFIG_KUNIT is disabled. There would be > some benefits to adding those pointers all the time (reduced complexity, > ability to display function names in BUG/WARNING messages). That change, > if desired, can be made later. > > Checkpatch note: > Remaining checkpatch errors and warnings were deliberately ignored. > Some are triggered by matching coding style or by comments interpreted > as code, others by assembler macros which are disliked by checkpatch. > Suggestions for improvements are welcome. > > Changes since RFC: > - Introduced CONFIG_KUNIT_SUPPRESS_BACKTRACE > - Minor cleanups and bug fixes > - Added support for all affected architectures > - Added support for counting suppressed warnings > - Added unit tests using those counters > - Added patch to suppress warning backtraces in dev_addr_lists tests > > Changes since v1: > - Rebased to v6.9-rc1 > - Added Tested-by:, Acked-by:, and Reviewed-by: tags > [I retained those tags since there have been no functional changes] > - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option, enabled by > default. > > Changes since v2: > - Rebased to v6.9-rc2 > - Added comments to drm warning suppression explaining why it is needed. > - Added patch to move conditional code in arch/sh/include/asm/bug.h > to avoid kerneldoc warning > - Added architecture maintainers to Cc: for architecture specific patches > - No functional changes > > > Guenter Roeck (15): > bug/kunit: Core support for suppressing warning backtraces > kunit: bug: Count suppressed warning backtraces > kunit: Add test cases for backtrace warning suppression > kunit: Add documentation for warning backtrace suppression API > drm: Suppress intentional warning backtraces in scaling unit tests > net: kunit: Suppress lock warning noise at end of dev_addr_lists tests > x86: Add support for suppressing warning backtraces > arm64: Add support for suppressing warning backtraces > loongarch: Add support for suppressing warning backtraces > parisc: Add support for suppressing warning
Re: [PATCH 1/9] hyperv: Convert from tasklet to BH workqueue
> The only generic interface to execute asynchronously in the BH context is > tasklet; however, it's marked deprecated and has some design flaws. To > replace tasklets, BH workqueue support was recently added. A BH workqueue > behaves similarly to regular workqueues except that the queued work items > are executed in the BH context. > > This patch converts drivers/hv/* from tasklet to BH workqueue. > > Based on the work done by Tejun Heo > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10 > > Signed-off-by: Allen Pais > --- > drivers/hv/channel.c | 8 > drivers/hv/channel_mgmt.c | 5 ++--- > drivers/hv/connection.c | 9 + > drivers/hv/hv.c | 3 +-- > drivers/hv/hv_balloon.c | 4 ++-- > drivers/hv/hv_fcopy.c | 8 > drivers/hv/hv_kvp.c | 8 > drivers/hv/hv_snapshot.c | 8 > drivers/hv/hyperv_vmbus.h | 9 + > drivers/hv/vmbus_drv.c| 19 ++- > include/linux/hyperv.h| 2 +- > 11 files changed, 42 insertions(+), 41 deletions(-) Wei, I need to send out a v2 as I did not include the second patch that updates drivers/pci/controller/pci-hyperv.c Thanks.
Re: [PATCH 7/9] s390: Convert from tasklet to BH workqueue
> > > > Signed-off-by: Allen Pais > > --- > > drivers/s390/block/dasd.c | 42 > > drivers/s390/block/dasd_int.h | 10 +++--- > > drivers/s390/char/con3270.c| 27 > > drivers/s390/crypto/ap_bus.c | 24 +++--- > > drivers/s390/crypto/ap_bus.h | 2 +- > > drivers/s390/crypto/zcrypt_msgtype50.c | 2 +- > > drivers/s390/crypto/zcrypt_msgtype6.c | 4 +-- > > drivers/s390/net/ctcm_fsms.c | 4 +-- > > drivers/s390/net/ctcm_main.c | 15 - > > drivers/s390/net/ctcm_main.h | 5 +-- > > drivers/s390/net/ctcm_mpc.c| 12 +++ > > drivers/s390/net/ctcm_mpc.h| 7 ++-- > > drivers/s390/net/lcs.c | 26 +++ > > drivers/s390/net/lcs.h | 2 +- > > drivers/s390/net/qeth_core_main.c | 2 +- > > drivers/s390/scsi/zfcp_qdio.c | 45 +- > > drivers/s390/scsi/zfcp_qdio.h | 9 +++--- > > 17 files changed, 117 insertions(+), 121 deletions(-) > > > > > We're looking into the best way to test this. > > For drivers/s390/net/ctcm* and drivers/s390/net/lcs*: > Acked-by: Alexandra Winter Thank you very much. > > > [...] > > diff --git a/drivers/s390/net/qeth_core_main.c > > b/drivers/s390/net/qeth_core_main.c > > index a0cce6872075..10ea95abc753 100644 > > --- a/drivers/s390/net/qeth_core_main.c > > +++ b/drivers/s390/net/qeth_core_main.c > > @@ -2911,7 +2911,7 @@ static int qeth_init_input_buffer(struct qeth_card > > *card, > > } > > > > /* > > - * since the buffer is accessed only from the input_tasklet > > + * since the buffer is accessed only from the input_work > >* there shouldn't be a need to synchronize; also, since we use > >* the QETH_IN_BUF_REQUEUE_THRESHOLD we should never run out off > >* buffers > > I propose to delete the whole comment block. There have been many changes and > I don't think it is helpful for the current qeth driver. Sure, I will have it fixed in v2. - Allen
Re: [PATCH v2 0/7] arch/mm/fault: accelerate pagefault when badaccess
On Wed, 3 Apr 2024 16:37:58 +0800 Kefeng Wang wrote: > After VMA lock-based page fault handling enabled, if bad access met > under per-vma lock, it will fallback to mmap_lock-based handling, > so it leads to unnessary mmap lock and vma find again. A test from > lmbench shows 34% improve after this changes on arm64, > > lat_sig -P 1 prot lat_sig 0.29194 -> 0.19198 > > Only build test on other archs except arm64. Thanks. So we now want a bunch of architectures to runtime test this. Do we have a selftest in place which will adequately do this?
Re: [PATCH 2/7] arm64: mm: accelerate pagefault when VM_FAULT_BADACCESS
On Tue, Apr 02, 2024 at 03:51:37PM +0800, Kefeng Wang wrote:
> The vm_flags of vma already checked under per-VMA lock, if it is a
> bad access, directly set fault to VM_FAULT_BADACCESS and handle error,
> no need to lock_mm_and_find_vma() and check vm_flags again, the latency
> time reduce 34% in lmbench 'lat_sig -P 1 prot lat_sig'.
>
> Signed-off-by: Kefeng Wang
> ---
> arch/arm64/mm/fault.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 9bb9f395351a..405f9aa831bd 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -572,7 +572,9 @@ static int __kprobes do_page_fault(unsigned long far,
> unsigned long esr,
>
> if (!(vma->vm_flags & vm_flags)) {
> vma_end_read(vma);
> - goto lock_mmap;
> + fault = VM_FAULT_BADACCESS;
> + count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> + goto done;
> }
> fault = handle_mm_fault(vma, addr, mm_flags | FAULT_FLAG_VMA_LOCK,
> regs);
> if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
I think this makes sense. A concurrent modification of vma->vm_flags
(e.g. mprotect()) would do a vma_start_write(), so no need to recheck
again with the mmap lock held.
Reviewed-by: Catalin Marinas
Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback
On Wed, Apr 03, 2024 at 09:08:41AM -0300, Jason Gunthorpe wrote: > On Tue, Apr 02, 2024 at 07:35:45PM -0400, Peter Xu wrote: > > On Tue, Apr 02, 2024 at 07:53:20PM -0300, Jason Gunthorpe wrote: > > > On Tue, Apr 02, 2024 at 06:43:56PM -0400, Peter Xu wrote: > > > > > > > I actually tested this without hitting the issue (even though I didn't > > > > mention it in the cover letter..). I re-kicked the build test, it turns > > > > out my "make alldefconfig" on loongarch will generate a config with both > > > > HUGETLB=n && THP=n, while arch/loongarch/configs/loongson3_defconfig has > > > > THP=y (which I assume was the one above build used). I didn't further > > > > check how "make alldefconfig" generated the config; a bit surprising > > > > that > > > > it didn't fetch from there. > > > > > > I suspect it is weird compiler variations.. Maybe something is not > > > being inlined. > > > > > > > (and it also surprises me that this BUILD_BUG can trigger.. I used to > > > > try > > > > triggering it elsewhere but failed..) > > > > > > As the pud_leaf() == FALSE should result in the BUILD_BUG never being > > > called and the optimizer removing it. > > > > Good point, for some reason loongarch defined pud_leaf() without defining > > pud_pfn(), which does look strange. > > > > #define pud_leaf(pud) ((pud_val(pud) & _PAGE_HUGE) != 0) > > > > But I noticed at least MIPS also does it.. Logically I think one arch > > should define either none of both. > > Wow, this is definately an arch issue. You can't define pud_leaf() and > not have a pud_pfn(). It makes no sense at all.. > > I'd say the BUILD_BUG has done it's job and found an issue, fix it by > not defining pud_leaf? I don't see any calls to pud_leaf in loongarch > at least Yes, that sounds better too to me, however it means we may also risk other archs that can fail another defconfig build.. and I worry I bring trouble to multiple such cases. Fundamentally it's indeed my patch that broke those builds, so I still sent the change and leave that for arch developers to decide the best for the archs. I think if wanted, we can add that BUILD_BUG() back when we're sure no arch will break with it. So such changes from arch can still be proposed alongside of removal of BUILD_BUG() (and I'd guess some other arch will start to notice such build issue soon if existed.. so it still more or less has similar effect of a reminder..). Thanks, -- Peter Xu
Re: [RFC PATCH 1/8] mm: Provide pagesize to pmd_populate()
Le 27/03/2024 à 17:57, Jason Gunthorpe a écrit :
> On Wed, Mar 27, 2024 at 09:58:35AM +, Christophe Leroy wrote:
>>> Just general remarks on the ones with huge pages:
>>>
>>>hash 64k and hugepage 16M/16G
>>>radix 64k/radix hugepage 2M/1G
>>>radix 4k/radix hugepage 2M/1G
>>>nohash 32
>>> - I think this is just a normal x86 like scheme? PMD/PUD can be a
>>> leaf with the same size as a next level table.
>>>
>>> Do any of these cases need to know the higher level to parse the
>>> lower? eg is there a 2M bit in the PUD indicating that the PMD
>>> is a table of 2M leafs or does each PMD entry have a bit
>>> indicating it is a leaf?
>>
>> For hash and radix there is a bit that tells it is leaf (_PAGE_PTE)
>>
>> For nohash32/e500 I think the drawing is not full right, there is a huge
>> page directory (hugepd) with a single entry. I think it should be
>> possible to change it to a leaf entry, it seems we have bit _PAGE_SW1
>> available in the PTE.
>
> It sounds to me like PPC breaks down into only a couple fundamental
> behaviors
> - x86 like leaf in many page levels. Use the pgd/pud/pmd_leaf() and
> related to implement it
> - ARM like contig PTE within a single page table level. Use the
> contig sutff to implement it
> - Contig PTE across two page table levels with a bit in the
> PMD. Needs new support like you showed
> - Page table levels with a variable page size. Ie a PUD can point to
> a directory of 8 pages or 512 pages of different size. Probbaly
> needs some new core support, but I think your changes to the
> *_offset go a long way already.
>
>>>
>>>hash 4k and hugepage 16M/16G
>>>nohash 64
>>> - How does this work? I guess since 8xx explicitly calls out
>>> consecutive this is actually the pgd can point to 512 256M
>>> entries or 8 16G entries? Ie the table size at each level is
>>> varable? Or is it the same and the table size is still 512 and
>>> each 16G entry is replicated 64 times?
>>
>> For those it is using the huge page directory (hugepd) which can be
>> hooked at any level and is a directory of huge pages on its own. There
>> is no consecutive entries involved here I think, allthough I'm not
>> completely sure.
>>
>> For hash4k I'm not sure how it works, this was changed by commit
>> e2b3d202d1db ("powerpc: Switch 16GB and 16MB explicit hugepages to a
>> different page table format")
>>
>> For the nohash/64, a PGD entry points either to a regular PUD directory
>> or to a HUGEPD directory. The size of the HUGEPD directory is encoded in
>> the 6 lower bits of the PGD entry.
>
> If it is a software walker there might be value in just aligning to
> the contig pte scheme in all levels and forgetting about the variable
> size page table levels. That quarter page stuff is a PITA to manage
> the memory allocation for on PPC anyhow..
Looking one step further, into nohash/32, I see a challenge: on that
platform, a PTE is 64 bits while a PGD/PMD entry is 32 bits. It is
therefore not possible as such to do PMD leaf or cont-PMD leaf.
I see two possible solutions:
- Double the size of PGD/PMD entries, but then we loose atomicity when
reading or writing an entry, could this be a problem ?
- Do as for the 8xx, ie go down to PTEs even for pages greater than 4M.
Any thought ?
Christophe
Re: [PATCH 1/7] arm64: mm: cleanup __do_page_fault()
On Tue, Apr 02, 2024 at 03:51:36PM +0800, Kefeng Wang wrote: > The __do_page_fault() only check vma->flags and call handle_mm_fault(), > and only called by do_page_fault(), let's squash it into do_page_fault() > to cleanup code. > > Signed-off-by: Kefeng Wang Reviewed-by: Catalin Marinas
Re: [PATCH v8 6/6] docs: trusted-encrypted: add DCP as new trust source
On Wed Apr 3, 2024 at 10:21 AM EEST, David Gstir wrote: > Update the documentation for trusted and encrypted KEYS with DCP as new > trust source: > > - Describe security properties of DCP trust source > - Describe key usage > - Document blob format > > Co-developed-by: Richard Weinberger > Signed-off-by: Richard Weinberger > Co-developed-by: David Oberhollenzer > Signed-off-by: David Oberhollenzer > Signed-off-by: David Gstir > --- > .../security/keys/trusted-encrypted.rst | 53 +++ > security/keys/trusted-keys/trusted_dcp.c | 19 +++ > 2 files changed, 72 insertions(+) > > diff --git a/Documentation/security/keys/trusted-encrypted.rst > b/Documentation/security/keys/trusted-encrypted.rst > index e989b9802f92..f4d7e162d5e4 100644 > --- a/Documentation/security/keys/trusted-encrypted.rst > +++ b/Documentation/security/keys/trusted-encrypted.rst > @@ -42,6 +42,14 @@ safe. > randomly generated and fused into each SoC at manufacturing time. > Otherwise, a common fixed test key is used instead. > > + (4) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs) > + > + Rooted to a one-time programmable key (OTP) that is generally burnt > + in the on-chip fuses and is accessible to the DCP encryption engine > only. > + DCP provides two keys that can be used as root of trust: the OTP key > + and the UNIQUE key. Default is to use the UNIQUE key, but selecting > + the OTP key can be done via a module parameter (dcp_use_otp_key). > + >* Execution isolation > > (1) TPM > @@ -57,6 +65,12 @@ safe. > > Fixed set of operations running in isolated execution environment. > > + (4) DCP > + > + Fixed set of cryptographic operations running in isolated execution > + environment. Only basic blob key encryption is executed there. > + The actual key sealing/unsealing is done on main processor/kernel > space. > + >* Optional binding to platform integrity state > > (1) TPM > @@ -79,6 +93,11 @@ safe. > Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs > for platform integrity. > > + (4) DCP > + > + Relies on Secure/Trusted boot process (called HAB by vendor) for > + platform integrity. > + >* Interfaces and APIs > > (1) TPM > @@ -94,6 +113,11 @@ safe. > > Interface is specific to silicon vendor. > > + (4) DCP > + > + Vendor-specific API that is implemented as part of the DCP crypto > driver in > + ``drivers/crypto/mxs-dcp.c``. > + >* Threat model > > The strength and appropriateness of a particular trust source for a > given > @@ -129,6 +153,13 @@ selected trust source: > CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and ensure the device > is probed. > > + * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs) > + > + The DCP hardware device itself does not provide a dedicated RNG > interface, > + so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do > have > + a dedicated hardware RNG that is independent from DCP which can be > enabled > + to back the kernel RNG. > + > Users may override this by specifying ``trusted.rng=kernel`` on the kernel > command-line to override the used RNG with the kernel's random number pool. > > @@ -231,6 +262,19 @@ Usage:: > CAAM-specific format. The key length for new keys is always in bytes. > Trusted Keys can be 32 - 128 bytes (256 - 1024 bits). > > +Trusted Keys usage: DCP > +--- > + > +Usage:: > + > +keyctl add trusted name "new keylen" ring > +keyctl add trusted name "load hex_blob" ring > +keyctl print keyid > + > +"keyctl print" returns an ASCII hex copy of the sealed key, which is in > format > +specific to this DCP key-blob implementation. The key length for new keys is > +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits). > + > Encrypted Keys usage > > > @@ -426,3 +470,12 @@ string length. > privkey is the binary representation of TPM2B_PUBLIC excluding the > initial TPM2B header which can be reconstructed from the ASN.1 octed > string length. > + > +DCP Blob Format > +--- > + > +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c > + :doc: dcp blob format > + > +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c > + :identifiers: struct dcp_blob_fmt > diff --git a/security/keys/trusted-keys/trusted_dcp.c > b/security/keys/trusted-keys/trusted_dcp.c > index 16c44aafeab3..b5f81a05be36 100644 > --- a/security/keys/trusted-keys/trusted_dcp.c > +++ b/security/keys/trusted-keys/trusted_dcp.c > @@ -19,6 +19,25 @@ > #define DCP_BLOB_VERSION 1 > #define DCP_BLOB_AUTHLEN 16 > > +/** > + * DOC: dcp blob format > + * > + * The Data Co-Processor (DCP) provides hardware-bound AES keys using its > + * AES encryption engine only.
Re: [PATCH 7/7] x86: mm: accelerate pagefault when badaccess
On Wed, Apr 3, 2024 at 12:58 AM Kefeng Wang wrote:
>
>
>
> On 2024/4/3 13:59, Suren Baghdasaryan wrote:
> > On Tue, Apr 2, 2024 at 12:53 AM Kefeng Wang
> > wrote:
> >>
> >> The vm_flags of vma already checked under per-VMA lock, if it is a
> >> bad access, directly handle error and return, there is no need to
> >> lock_mm_and_find_vma() and check vm_flags again.
> >>
> >> Signed-off-by: Kefeng Wang
> >
> > Looks safe to me.
> > Using (mm != NULL) to indicate that we are holding mmap_lock is not
> > ideal but I guess that works.
> >
>
> Yes, I will add this part it into change too,
>
> The access_error() of vma already checked under per-VMA lock, if it
> is a bad access, directly handle error, no need to retry with mmap_lock
> again. In order to release the correct lock, pass the mm_struct into
> bad_area_access_error(), if mm is NULL, release vma lock, or release
> mmap_lock. Since the page faut is handled under per-VMA lock, count it
> as a vma lock event with VMA_LOCK_SUCCESS.
The part about passing mm_struct is unnecessary IMHO. It explains "how
you do things" but changelog should describe only "what you do" and
"why you do that". The rest we can see from the code.
>
> Thanks.
>
>
> > Reviewed-by: Suren Baghdasaryan
> >
> >> ---
> >> arch/x86/mm/fault.c | 23 ++-
> >> 1 file changed, 14 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> >> index a4cc20d0036d..67b18adc75dd 100644
> >> --- a/arch/x86/mm/fault.c
> >> +++ b/arch/x86/mm/fault.c
> >> @@ -866,14 +866,17 @@ bad_area_nosemaphore(struct pt_regs *regs, unsigned
> >> long error_code,
> >>
> >> static void
> >> __bad_area(struct pt_regs *regs, unsigned long error_code,
> >> - unsigned long address, u32 pkey, int si_code)
> >> + unsigned long address, struct mm_struct *mm,
> >> + struct vm_area_struct *vma, u32 pkey, int si_code)
> >> {
> >> - struct mm_struct *mm = current->mm;
> >> /*
> >> * Something tried to access memory that isn't in our memory map..
> >> * Fix it, but check if it's kernel or user first..
> >> */
> >> - mmap_read_unlock(mm);
> >> + if (mm)
> >> + mmap_read_unlock(mm);
> >> + else
> >> + vma_end_read(vma);
> >>
> >> __bad_area_nosemaphore(regs, error_code, address, pkey, si_code);
> >> }
> >> @@ -897,7 +900,8 @@ static inline bool bad_area_access_from_pkeys(unsigned
> >> long error_code,
> >>
> >> static noinline void
> >> bad_area_access_error(struct pt_regs *regs, unsigned long error_code,
> >> - unsigned long address, struct vm_area_struct *vma)
> >> + unsigned long address, struct mm_struct *mm,
> >> + struct vm_area_struct *vma)
> >> {
> >> /*
> >> * This OSPKE check is not strictly necessary at runtime.
> >> @@ -927,9 +931,9 @@ bad_area_access_error(struct pt_regs *regs, unsigned
> >> long error_code,
> >> */
> >> u32 pkey = vma_pkey(vma);
> >>
> >> - __bad_area(regs, error_code, address, pkey, SEGV_PKUERR);
> >> + __bad_area(regs, error_code, address, mm, vma, pkey,
> >> SEGV_PKUERR);
> >> } else {
> >> - __bad_area(regs, error_code, address, 0, SEGV_ACCERR);
> >> + __bad_area(regs, error_code, address, mm, vma, 0,
> >> SEGV_ACCERR);
> >> }
> >> }
> >>
> >> @@ -1357,8 +1361,9 @@ void do_user_addr_fault(struct pt_regs *regs,
> >> goto lock_mmap;
> >>
> >> if (unlikely(access_error(error_code, vma))) {
> >> - vma_end_read(vma);
> >> - goto lock_mmap;
> >> + bad_area_access_error(regs, error_code, address, NULL,
> >> vma);
> >> + count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> >> + return;
> >> }
> >> fault = handle_mm_fault(vma, address, flags |
> >> FAULT_FLAG_VMA_LOCK, regs);
> >> if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
> >> @@ -1394,7 +1399,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> >> * we can handle it..
> >> */
> >> if (unlikely(access_error(error_code, vma))) {
> >> - bad_area_access_error(regs, error_code, address, vma);
> >> + bad_area_access_error(regs, error_code, address, mm, vma);
> >> return;
> >> }
> >>
> >> --
> >> 2.27.0
> >>
Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback
On Wed, Apr 03, 2024 at 01:17:06PM +, Christophe Leroy wrote:
> > That commit makes it sounds like the arch supports huge PUD's through
> > the hugepte mechanism - it says a LTP test failed so something
> > populated a huge PUD at least??
>
> Not sure, I more see it just like a copy/paste of commit 501b81046701
> ("mips: mm: add p?d_leaf() definitions").
>
> The commit message says that the test failed because pmd_leaf() is
> missing, it says nothing about PUD.
AH fair enough, it is probably a C then
Jason
Re: [PATCH v8 6/6] docs: trusted-encrypted: add DCP as new trust source
On Wed, Apr 03, 2024 at 09:21:22AM +0200, David Gstir wrote: > diff --git a/Documentation/security/keys/trusted-encrypted.rst > b/Documentation/security/keys/trusted-encrypted.rst > index e989b9802f92..f4d7e162d5e4 100644 > --- a/Documentation/security/keys/trusted-encrypted.rst > +++ b/Documentation/security/keys/trusted-encrypted.rst > @@ -42,6 +42,14 @@ safe. > randomly generated and fused into each SoC at manufacturing time. > Otherwise, a common fixed test key is used instead. > > + (4) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs) > + > + Rooted to a one-time programmable key (OTP) that is generally burnt > + in the on-chip fuses and is accessible to the DCP encryption engine > only. > + DCP provides two keys that can be used as root of trust: the OTP key > + and the UNIQUE key. Default is to use the UNIQUE key, but selecting > + the OTP key can be done via a module parameter (dcp_use_otp_key). > + >* Execution isolation > > (1) TPM > @@ -57,6 +65,12 @@ safe. > > Fixed set of operations running in isolated execution environment. > > + (4) DCP > + > + Fixed set of cryptographic operations running in isolated execution > + environment. Only basic blob key encryption is executed there. > + The actual key sealing/unsealing is done on main processor/kernel > space. > + >* Optional binding to platform integrity state > > (1) TPM > @@ -79,6 +93,11 @@ safe. > Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs > for platform integrity. > > + (4) DCP > + > + Relies on Secure/Trusted boot process (called HAB by vendor) for > + platform integrity. > + >* Interfaces and APIs > > (1) TPM > @@ -94,6 +113,11 @@ safe. > > Interface is specific to silicon vendor. > > + (4) DCP > + > + Vendor-specific API that is implemented as part of the DCP crypto > driver in > + ``drivers/crypto/mxs-dcp.c``. > + >* Threat model > > The strength and appropriateness of a particular trust source for a > given > @@ -129,6 +153,13 @@ selected trust source: > CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and ensure the device > is probed. > > + * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs) > + > + The DCP hardware device itself does not provide a dedicated RNG > interface, > + so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do > have > + a dedicated hardware RNG that is independent from DCP which can be > enabled > + to back the kernel RNG. > + > Users may override this by specifying ``trusted.rng=kernel`` on the kernel > command-line to override the used RNG with the kernel's random number pool. > > @@ -231,6 +262,19 @@ Usage:: > CAAM-specific format. The key length for new keys is always in bytes. > Trusted Keys can be 32 - 128 bytes (256 - 1024 bits). > > +Trusted Keys usage: DCP > +--- > + > +Usage:: > + > +keyctl add trusted name "new keylen" ring > +keyctl add trusted name "load hex_blob" ring > +keyctl print keyid > + > +"keyctl print" returns an ASCII hex copy of the sealed key, which is in > format > +specific to this DCP key-blob implementation. The key length for new keys is > +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits). > + > Encrypted Keys usage > > > @@ -426,3 +470,12 @@ string length. > privkey is the binary representation of TPM2B_PUBLIC excluding the > initial TPM2B header which can be reconstructed from the ASN.1 octed > string length. > + > +DCP Blob Format > +--- > + > +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c > + :doc: dcp blob format > + > +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c > + :identifiers: struct dcp_blob_fmt > diff --git a/security/keys/trusted-keys/trusted_dcp.c > b/security/keys/trusted-keys/trusted_dcp.c > index 16c44aafeab3..b5f81a05be36 100644 > --- a/security/keys/trusted-keys/trusted_dcp.c > +++ b/security/keys/trusted-keys/trusted_dcp.c > @@ -19,6 +19,25 @@ > #define DCP_BLOB_VERSION 1 > #define DCP_BLOB_AUTHLEN 16 > > +/** > + * DOC: dcp blob format > + * > + * The Data Co-Processor (DCP) provides hardware-bound AES keys using its > + * AES encryption engine only. It does not provide direct key > sealing/unsealing. > + * To make DCP hardware encryption keys usable as trust source, we define > + * our own custom format that uses a hardware-bound key to secure the sealing > + * key stored in the key blob. > + * > + * Whenever a new trusted key using DCP is generated, we generate a random > 128-bit > + * blob encryption key (BEK) and 128-bit nonce. The BEK and nonce are used to > + * encrypt the trusted key payload using AES-128-GCM. > + * > + * The BEK itself is encrypted using the hardware-bound key using
[PATCH v3 15/15] powerpc: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Michael Ellerman
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/powerpc/include/asm/bug.h | 37 +-
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 1db485aacbd9..5b06745d20aa 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -14,6 +14,9 @@
.section __bug_table,"aw"
5001: .4byte \addr - .
.4byte 5002f - .
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+.4byte 0
+#endif
.short \line, \flags
.org 5001b+BUG_ENTRY_SIZE
.previous
@@ -32,30 +35,46 @@
#endif /* verbose */
#else /* !__ASSEMBLY__ */
-/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3 to be FILE, LINE, flags and
- sizeof(struct bug_entry), respectively */
+/* _EMIT_BUG_ENTRY expects args %0,%1,%2,%3,%4 to be FILE, __func__, LINE,
flags
+ and sizeof(struct bug_entry), respectively */
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .4byte %1 - .\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
" .4byte %0 - .\n"\
- " .short %1, %2\n"\
- ".org 2b+%3\n" \
+ __BUG_FUNC_PTR \
+ " .short %2, %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#else
#define _EMIT_BUG_ENTRY\
".section __bug_table,\"aw\"\n" \
"2: .4byte 1b - .\n"\
- " .short %2\n"\
- ".org 2b+%3\n" \
+ " .short %3\n"\
+ ".org 2b+%4\n" \
".previous\n"
#endif
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define BUG_ENTRY(insn, flags, ...)\
__asm__ __volatile__( \
"1: " insn "\n" \
_EMIT_BUG_ENTRY \
- : : "i" (__FILE__), "i" (__LINE__), \
+ : : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
##__VA_ARGS__)
@@ -80,7 +99,7 @@
if (x) \
BUG(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", 0, "r" ((__force long)(x)));
\
+ BUG_ENTRY(PPC_TLNEI " %5, 0", 0, "r" ((__force long)(x)));
\
} \
} while (0)
@@ -90,7 +109,7 @@
if (__ret_warn_on) \
__WARN(); \
} else {\
- BUG_ENTRY(PPC_TLNEI " %4, 0", \
+ BUG_ENTRY(PPC_TLNEI " %5, 0", \
BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), \
"r" (__ret_warn_on)); \
} \
--
2.39.2
[PATCH v3 14/15] riscv: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
To simplify the implementation, unify the __BUG_ENTRY_ADDR and
__BUG_ENTRY_FILE macros into a single macro named __BUG_REL() which takes
the address, file, or function reference as parameter.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Paul Walmsley
Cc: Palmer Dabbelt
Cc: Albert Ou
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/riscv/include/asm/bug.h | 38
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
index 1aaea81fb141..79f360af4ad8 100644
--- a/arch/riscv/include/asm/bug.h
+++ b/arch/riscv/include/asm/bug.h
@@ -30,26 +30,39 @@
typedef u32 bug_insn_t;
#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
-#define __BUG_ENTRY_ADDR RISCV_INT " 1b - ."
-#define __BUG_ENTRY_FILE RISCV_INT " %0 - ."
+#define __BUG_REL(val) RISCV_INT " " __stringify(val) " - ."
#else
-#define __BUG_ENTRY_ADDR RISCV_PTR " 1b"
-#define __BUG_ENTRY_FILE RISCV_PTR " %0"
+#define __BUG_REL(val) RISCV_PTR " " __stringify(val)
#endif
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%1)
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- __BUG_ENTRY_FILE "\n\t" \
- RISCV_SHORT " %1\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ __BUG_REL(%0) "\n\t"\
+ __BUG_FUNC_PTR "\n\t" \
+ RISCV_SHORT " %2\n\t" \
+ RISCV_SHORT " %3"
#else
#define __BUG_ENTRY\
- __BUG_ENTRY_ADDR "\n\t" \
- RISCV_SHORT " %2"
+ __BUG_REL(1b) "\n\t"\
+ RISCV_SHORT " %3"
#endif
#ifdef CONFIG_GENERIC_BUG
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
do { \
__asm__ __volatile__ ( \
@@ -58,10 +71,11 @@ do {
\
".pushsection __bug_table,\"aw\"\n\t" \
"2:\n\t"\
__BUG_ENTRY "\n\t" \
- ".org 2b + %3\n\t" \
+ ".org 2b + %4\n\t" \
".popsection" \
: \
- : "i" (__FILE__), "i" (__LINE__), \
+ : "i" (__FILE__), "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
--
2.39.2
[PATCH v3 13/15] sh: Move defines needed for suppressing warning backtraces
Declaring the defines needed for suppressing warning inside '#ifdef CONFIG_DEBUG_BUGVERBOSE' results in a kerneldoc warning. .../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY(). Prototype was for HAVE_BUG_FUNCTION() instead Move the defines above the kerneldoc entry for _EMIT_BUG_ENTRY to make kerneldoc happy. Reported-by: Simon Horman Cc: Simon Horman Cc: Yoshinori Sato Cc: Rich Felker Cc: John Paul Adrian Glaubitz Signed-off-by: Guenter Roeck --- v3: Added patch. Possibly squash into previous patch. arch/sh/include/asm/bug.h | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h index 470ce6567d20..bf4947d51d69 100644 --- a/arch/sh/include/asm/bug.h +++ b/arch/sh/include/asm/bug.h @@ -11,6 +11,15 @@ #define HAVE_ARCH_BUG #define HAVE_ARCH_WARN_ON +#ifdef CONFIG_DEBUG_BUGVERBOSE +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE +# define HAVE_BUG_FUNCTION +# define __BUG_FUNC_PTR"\t.long %O2\n" +#else +# define __BUG_FUNC_PTR +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */ +#endif /* CONFIG_DEBUG_BUGVERBOSE */ + /** * _EMIT_BUG_ENTRY * %1 - __FILE__ @@ -25,13 +34,6 @@ */ #ifdef CONFIG_DEBUG_BUGVERBOSE -#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE -# define HAVE_BUG_FUNCTION -# define __BUG_FUNC_PTR"\t.long %O2\n" -#else -# define __BUG_FUNC_PTR -#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */ - #define _EMIT_BUG_ENTRY\ "\t.pushsection __bug_table,\"aw\"\n" \ "2:\t.long 1b, %O1\n" \ -- 2.39.2
[PATCH v3 12/15] sh: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Yoshinori Sato
Cc: Rich Felker
Cc: John Paul Adrian Glaubitz
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/sh/include/asm/bug.h | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
index 05a485c4fabc..470ce6567d20 100644
--- a/arch/sh/include/asm/bug.h
+++ b/arch/sh/include/asm/bug.h
@@ -24,21 +24,36 @@
* The offending file and line are encoded in the __bug_table section.
*/
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR"\t.long %O2\n"
+#else
+# define __BUG_FUNC_PTR
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _EMIT_BUG_ENTRY\
"\t.pushsection __bug_table,\"aw\"\n" \
"2:\t.long 1b, %O1\n" \
- "\t.short %O2, %O3\n" \
- "\t.org 2b+%O4\n" \
+ __BUG_FUNC_PTR \
+ "\t.short %O3, %O4\n" \
+ "\t.org 2b+%O5\n" \
"\t.popsection\n"
#else
#define _EMIT_BUG_ENTRY\
"\t.pushsection __bug_table,\"aw\"\n" \
"2:\t.long 1b\n"\
- "\t.short %O3\n"\
- "\t.org 2b+%O4\n" \
+ "\t.short %O4\n"\
+ "\t.org 2b+%O5\n" \
"\t.popsection\n"
#endif
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define BUG() \
do { \
__asm__ __volatile__ ( \
@@ -47,6 +62,7 @@ do { \
: \
: "n" (TRAPA_BUG_OPCODE), \
"i" (__FILE__), \
+ "i" (__BUG_FUNC),\
"i" (__LINE__), "i" (0), \
"i" (sizeof(struct bug_entry))); \
unreachable(); \
@@ -60,6 +76,7 @@ do { \
: \
: "n" (TRAPA_BUG_OPCODE), \
"i" (__FILE__), \
+ "i" (__BUG_FUNC),\
"i" (__LINE__), \
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry))); \
@@ -85,6 +102,7 @@ do { \
: \
: "n" (TRAPA_BUG_OPCODE), \
"i" (__FILE__), \
+ "i" (__BUG_FUNC),\
"i" (__LINE__), \
"i" (BUGFLAG_UNWINDER), \
"i" (sizeof(struct bug_entry))); \
--
2.39.2
[PATCH v3 11/15] s390: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because
__func__ is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Heiko Carstens
Cc: Vasily Gorbik
Cc: Alexander Gordeev
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1 (simplified assembler changes after upstream commit
3938490e78f4 ("s390/bug: remove entry size from __bug_table section")
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/s390/include/asm/bug.h | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index c500d45fb465..44d4e9f24ae0 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -8,6 +8,15 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR" .long %0-.\n"
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define __EMIT_BUG(x) do { \
asm_inline volatile(\
"0: mc 0,0\n" \
@@ -17,10 +26,12 @@
".section __bug_table,\"aw\"\n" \
"2: .long 0b-.\n" \
" .long 1b-.\n" \
- " .short %0,%1\n"\
- " .org2b+%2\n"\
+ __BUG_FUNC_PTR \
+ " .short %1,%2\n"\
+ " .org2b+%3\n"\
".previous\n" \
- : : "i" (__LINE__), \
+ : : "i" (__BUG_FUNC), \
+ "i" (__LINE__), \
"i" (x),\
"i" (sizeof(struct bug_entry)));\
} while (0)
--
2.39.2
[PATCH v3 10/15] parisc: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
While at it, declare assembler parameters as constants where possible.
Refine .blockz instructions to calculate the necessary padding instead
of using fixed values.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Acked-by: Helge Deller
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/parisc/include/asm/bug.h | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 833555f74ffa..b59c3f7380bf 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -23,8 +23,17 @@
# define __BUG_REL(val) ".word " __stringify(val)
#endif
-
#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define BUG() \
do {\
asm volatile("\n" \
@@ -33,10 +42,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %c3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (0), "i" (sizeof(struct bug_entry)) ); \
unreachable(); \
} while(0)
@@ -58,10 +69,12 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t" __BUG_REL(%c0) "\n" \
-"\t.short %1, %2\n"\
-"\t.blockz %3-2*4-2*2\n" \
+"\t" __BUG_FUNC_PTR "\n" \
+"\t.short %c2, %3\n" \
+"\t.blockz %c4-(.-2b)\n" \
"\t.popsection"\
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), \
+"i" (__LINE__),\
"i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
} while(0)
@@ -74,7 +87,7 @@
"\t.align 4\n" \
"2:\t" __BUG_REL(1b) "\n" \
"\t.short %0\n"\
-"\t.blockz %1-4-2\n" \
+"\t.blockz %c1-(.-2b)\n" \
"\t.popsection"\
: : "i" (BUGFLAG_WARNING|(flags)), \
"i" (sizeof(struct bug_entry)) ); \
--
2.39.2
[PATCH v3 09/15] loongarch: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Huacai Chen
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1; resolved context conflict
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2; resolved context conflict
arch/loongarch/include/asm/bug.h | 38 +++-
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index 08388876ade4..193f396d81a0 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -3,47 +3,63 @@
#define __ASM_BUG_H
#include
+#include
#include
#ifndef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#else
-#define __BUGVERBOSE_LOCATION(file, line) \
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str, "aMS", @progbits, 1; \
10002: .string file; \
.popsection;\
\
.long 10002b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
#endif
#ifndef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#else
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table, "aw"; \
.align 2; \
1: .long 10001f - .; \
- _BUGVERBOSE_LOCATION(__FILE__, __LINE__)\
+ _BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
.short flags; \
.popsection;\
10001:
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
break BRK_BUG
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
+
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
#define __BUG_FLAGS(flags) \
- asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags, %0)) : : "i"
(__BUG_FUNC));
#define __WARN_FLAGS(flags)\
do { \
instrumentation_begin();\
- __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ __BUG_FLAGS(BUGFLAG_WARNING|(flags)); \
annotate_reachable(); \
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH v3 08/15] arm64: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Catalin Marinas
Cc: Will Deacon
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/arm64/include/asm/asm-bug.h | 29 +++--
arch/arm64/include/asm/bug.h | 8 +++-
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index c762038ba400..c6d22e3cd840 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -8,36 +8,45 @@
#include
#ifdef CONFIG_DEBUG_BUGVERBOSE
-#define _BUGVERBOSE_LOCATION(file, line) __BUGVERBOSE_LOCATION(file, line)
-#define __BUGVERBOSE_LOCATION(file, line) \
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR(func) .long func - .;
+#else
+# define __BUG_FUNC_PTR(func)
+#endif
+
+#define _BUGVERBOSE_LOCATION(file, func, line) __BUGVERBOSE_LOCATION(file,
func, line)
+#define __BUGVERBOSE_LOCATION(file, func, line)\
.pushsection .rodata.str,"aMS",@progbits,1; \
14472: .string file; \
.popsection;\
\
.long 14472b - .; \
+ __BUG_FUNC_PTR(func)\
.short line;
#else
-#define _BUGVERBOSE_LOCATION(file, line)
+#define _BUGVERBOSE_LOCATION(file, func, line)
#endif
#ifdef CONFIG_GENERIC_BUG
-#define __BUG_ENTRY(flags) \
+#define __BUG_ENTRY(flags, func) \
.pushsection __bug_table,"aw"; \
.align 2; \
14470: .long 14471f - .; \
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__) \
- .short flags; \
+_BUGVERBOSE_LOCATION(__FILE__, func, __LINE__) \
+ .short flags; \
.popsection;\
14471:
#else
-#define __BUG_ENTRY(flags)
+#define __BUG_ENTRY(flags, func)
#endif
-#define ASM_BUG_FLAGS(flags) \
- __BUG_ENTRY(flags) \
+#define ASM_BUG_FLAGS(flags, func) \
+ __BUG_ENTRY(flags, func)\
brk BUG_BRK_IMM
-#define ASM_BUG() ASM_BUG_FLAGS(0)
+#define ASM_BUG() ASM_BUG_FLAGS(0, .)
#endif /* __ASM_ASM_BUG_H */
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 28be048db3f6..044c5e24a17d 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -11,8 +11,14 @@
#include
+#ifdef HAVE_BUG_FUNCTION
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNCNULL
+#endif
+
#define __BUG_FLAGS(flags) \
- asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
+ asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i"
(__BUG_FUNC));
#define BUG() do { \
__BUG_FLAGS(0); \
--
2.39.2
[PATCH v3 07/15] x86: Add support for suppressing warning backtraces
Add name of functions triggering warning backtraces to the __bug_table
object section to enable support for suppressing WARNING backtraces.
To limit image size impact, the pointer to the function name is only added
to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
parameter is replaced with a (dummy) NULL parameter to avoid an image size
increase due to unused __func__ entries (this is necessary because __func__
is not a define but a virtual variable).
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: Dave Hansen
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
arch/x86/include/asm/bug.h | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index a3ec87d198ac..7698dfa74c98 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -23,18 +23,28 @@
#ifdef CONFIG_DEBUG_BUGVERBOSE
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+# define HAVE_BUG_FUNCTION
+# define __BUG_FUNC_PTR__BUG_REL(%c1)
+# define __BUG_FUNC__func__
+#else
+# define __BUG_FUNC_PTR
+# define __BUG_FUNCNULL
+#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
+
#define _BUG_FLAGS(ins, flags, extra) \
do { \
asm_inline volatile("1:\t" ins "\n" \
".pushsection __bug_table,\"aw\"\n"\
"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \
"\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \
-"\t.word %c1""\t# bug_entry::line\n" \
-"\t.word %c2""\t# bug_entry::flags\n" \
-"\t.org 2b+%c3\n" \
+"\t" __BUG_FUNC_PTR "\t# bug_entry::function\n" \
+"\t.word %c2""\t# bug_entry::line\n" \
+"\t.word %c3""\t# bug_entry::flags\n" \
+"\t.org 2b+%c4\n" \
".popsection\n"\
extra \
-: : "i" (__FILE__), "i" (__LINE__),\
+: : "i" (__FILE__), "i" (__BUG_FUNC), "i" (__LINE__),\
"i" (flags), \
"i" (sizeof(struct bug_entry))); \
} while (0)
@@ -80,7 +90,8 @@ do {
\
do { \
__auto_type __flags = BUGFLAG_WARNING|(flags); \
instrumentation_begin();\
- _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);\
instrumentation_end(); \
} while (0)
--
2.39.2
[PATCH v3 06/15] net: kunit: Suppress lock warning noise at end of dev_addr_lists tests
dev_addr_lists_test generates lock warning noise at the end of tests
if lock debugging is enabled. There are two sets of warnings.
WARNING: CPU: 0 PID: 689 at kernel/locking/mutex.c:923
__mutex_unlock_slowpath.constprop.0+0x13c/0x368
DEBUG_LOCKS_WARN_ON(__owner_task(owner) != __get_current())
WARNING: kunit_try_catch/1336 still has locks held!
KUnit test cleanup is not guaranteed to run in the same thread as the test
itself. For this test, this means that rtnl_lock() and rtnl_unlock() may
be called from different threads. This triggers the warnings.
Suppress the warnings because they are irrelevant for the test and just
confusing and distracting.
The first warning can be suppressed by using START_SUPPRESSED_WARNING()
and END_SUPPRESSED_WARNING() around the call to rtnl_unlock(). To suppress
the second warning, it is necessary to set debug_locks_silent while the
rtnl lock is held.
Tested-by: Linux Kernel Functional Testing
Cc: David Gow
Cc: Jakub Kicinski
Cc: Eric Dumazet
Acked-by: Dan Carpenter
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
v3:
- Rebased to v6.9-rc2
net/core/dev_addr_lists_test.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 4dbd0dc6aea2..b427dd1a3c93 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
#include
+#include
#include
#include
#include
@@ -49,6 +50,7 @@ static int dev_addr_test_init(struct kunit *test)
KUNIT_FAIL(test, "Can't register netdev %d", err);
}
+ debug_locks_silent = 1;
rtnl_lock();
return 0;
}
@@ -56,8 +58,12 @@ static int dev_addr_test_init(struct kunit *test)
static void dev_addr_test_exit(struct kunit *test)
{
struct net_device *netdev = test->priv;
+ DEFINE_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ START_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
rtnl_unlock();
+ END_SUPPRESSED_WARNING(__mutex_unlock_slowpath);
+ debug_locks_silent = 0;
unregister_netdev(netdev);
free_netdev(netdev);
}
--
2.39.2
[PATCH v3 05/15] drm: Suppress intentional warning backtraces in scaling unit tests
The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
intentionally trigger warning backtraces by providing bad parameters to
the tested functions. What is tested is the return value, not the existence
of a warning backtrace. Suppress the backtraces to avoid clogging the
kernel log and distraction from real problems.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Acked-by: Maíra Canal
Cc: Maarten Lankhorst
Cc: David Airlie
Cc: Daniel Vetter
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
v3:
- Rebased to v6.9-rc2
drivers/gpu/drm/tests/drm_rect_test.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/tests/drm_rect_test.c
b/drivers/gpu/drm/tests/drm_rect_test.c
index 76332cd2ead8..66851769ee32 100644
--- a/drivers/gpu/drm/tests/drm_rect_test.c
+++ b/drivers/gpu/drm/tests/drm_rect_test.c
@@ -406,22 +406,38 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases,
drm_rect_scale_case_desc
static void drm_test_rect_calc_hscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ /*
+* drm_rect_calc_hscale() generates a warning backtrace whenever bad
+* parameters are passed to it. This affects all unit tests with an
+* error code in expected_scaling_factor.
+*/
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_hscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
static void drm_test_rect_calc_vscale(struct kunit *test)
{
+ DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
const struct drm_rect_scale_case *params = test->param_value;
int scaling_factor;
+ /*
+* drm_rect_calc_vscale() generates a warning backtrace whenever bad
+* parameters are passed to it. This affects all unit tests with an
+* error code in expected_scaling_factor.
+*/
+ START_SUPPRESSED_WARNING(drm_calc_scale);
scaling_factor = drm_rect_calc_vscale(>src, >dst,
params->min_range,
params->max_range);
+ END_SUPPRESSED_WARNING(drm_calc_scale);
KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
}
--
2.39.2
[PATCH v3 04/15] kunit: Add documentation for warning backtrace suppression API
Document API functions for suppressing warning backtraces.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
v3:
- Rebased to v6.9-rc2
Documentation/dev-tools/kunit/usage.rst | 30 -
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst
b/Documentation/dev-tools/kunit/usage.rst
index 22955d56b379..8d3d36d4103d 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -157,6 +157,34 @@ Alternatively, one can take full control over the error
message by using
if (some_setup_function())
KUNIT_FAIL(test, "Failed to setup thing for testing");
+Suppressing warning backtraces
+--
+
+Some unit tests trigger warning backtraces either intentionally or as side
+effect. Such backtraces are normally undesirable since they distract from
+the actual test and may result in the impression that there is a problem.
+
+Such backtraces can be suppressed. To suppress a backtrace in some_function(),
+use the following code.
+
+.. code-block:: c
+
+ static void some_test(struct kunit *test)
+ {
+ DEFINE_SUPPRESSED_WARNING(some_function);
+
+ START_SUPPRESSED_WARNING(some_function);
+ trigger_backtrace();
+ END_SUPPRESSED_WARNING(some_function);
+ }
+
+SUPPRESSED_WARNING_COUNT() returns the number of suppressed backtraces. If the
+suppressed backtrace was triggered on purpose, this can be used to check if
+the backtrace was actually triggered.
+
+.. code-block:: c
+
+ KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(some_function), 1);
Test Suites
~~~
@@ -857,4 +885,4 @@ For example:
dev_managed_string = devm_kstrdup(fake_device, "Hello, World!");
// Everything is cleaned up automatically when the test ends.
- }
\ No newline at end of file
+ }
--
2.39.2
[PATCH v3 03/15] kunit: Add test cases for backtrace warning suppression
Add unit tests to verify that warning backtrace suppression works.
If backtrace suppression does _not_ work, the unit tests will likely
trigger unsuppressed backtraces, which should actually help to get
the affected architectures / platforms fixed.
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
lib/kunit/Makefile | 7 +-
lib/kunit/backtrace-suppression-test.c | 104 +
2 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 lib/kunit/backtrace-suppression-test.c
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 545b57c3be48..3eee1bd0ce5e 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -16,10 +16,13 @@ endif
# KUnit 'hooks' and bug handling are built-in even when KUnit is built
# as a module.
-obj-y += hooks.o \
- bug.o
+obj-y += hooks.o
+obj-$(CONFIG_KUNIT_SUPPRESS_BACKTRACE) += bug.o
obj-$(CONFIG_KUNIT_TEST) +=kunit-test.o
+ifeq ($(CCONFIG_KUNIT_SUPPRESS_BACKTRACE),y)
+obj-$(CONFIG_KUNIT_TEST) +=backtrace-suppression-test.o
+endif
# string-stream-test compiles built-in only.
ifeq ($(CONFIG_KUNIT_TEST),y)
diff --git a/lib/kunit/backtrace-suppression-test.c
b/lib/kunit/backtrace-suppression-test.c
new file mode 100644
index ..47c619283802
--- /dev/null
+++ b/lib/kunit/backtrace-suppression-test.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for suppressing warning tracebacks
+ *
+ * Copyright (C) 2024, Guenter Roeck
+ * Author: Guenter Roeck
+ */
+
+#include
+#include
+
+static void backtrace_suppression_test_warn_direct(struct kunit *test)
+{
+ DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
+
+ START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
+ WARN(1, "This backtrace should be suppressed");
+ END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_direct);
+
+ KUNIT_EXPECT_EQ(test,
SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_direct), 1);
+}
+
+static void trigger_backtrace_warn(void)
+{
+ WARN(1, "This backtrace should be suppressed");
+}
+
+static void backtrace_suppression_test_warn_indirect(struct kunit *test)
+{
+ DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
+
+ START_SUPPRESSED_WARNING(trigger_backtrace_warn);
+ trigger_backtrace_warn();
+ END_SUPPRESSED_WARNING(trigger_backtrace_warn);
+
+ KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn),
1);
+}
+
+static void backtrace_suppression_test_warn_multi(struct kunit *test)
+{
+ DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn);
+ DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
+
+ START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
+ START_SUPPRESSED_WARNING(trigger_backtrace_warn);
+ WARN(1, "This backtrace should be suppressed");
+ trigger_backtrace_warn();
+ END_SUPPRESSED_WARNING(trigger_backtrace_warn);
+ END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_multi);
+
+ KUNIT_EXPECT_EQ(test,
SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_multi), 1);
+ KUNIT_EXPECT_EQ(test, SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn),
1);
+}
+
+static void backtrace_suppression_test_warn_on_direct(struct kunit *test)
+{
+ DEFINE_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
+
+ if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE) &&
!IS_ENABLED(CONFIG_KALLSYMS))
+ kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE or
CONFIG_KALLSYMS");
+
+ START_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
+ WARN_ON(1);
+ END_SUPPRESSED_WARNING(backtrace_suppression_test_warn_on_direct);
+
+ KUNIT_EXPECT_EQ(test,
+
SUPPRESSED_WARNING_COUNT(backtrace_suppression_test_warn_on_direct), 1);
+}
+
+static void trigger_backtrace_warn_on(void)
+{
+ WARN_ON(1);
+}
+
+static void backtrace_suppression_test_warn_on_indirect(struct kunit *test)
+{
+ DEFINE_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
+
+ if (!IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE))
+ kunit_skip(test, "requires CONFIG_DEBUG_BUGVERBOSE");
+
+ START_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
+ trigger_backtrace_warn_on();
+ END_SUPPRESSED_WARNING(trigger_backtrace_warn_on);
+
+ KUNIT_EXPECT_EQ(test,
SUPPRESSED_WARNING_COUNT(trigger_backtrace_warn_on), 1);
+}
+
+static struct kunit_case backtrace_suppression_test_cases[] = {
+ KUNIT_CASE(backtrace_suppression_test_warn_direct),
+
[PATCH v3 02/15] kunit: bug: Count suppressed warning backtraces
Count suppressed warning backtraces to enable code which suppresses
warning backtraces to check if the expected backtrace(s) have been
observed.
Using atomics for the backtrace count resulted in build errors on some
architectures due to include file recursion, so use a plain integer
for now.
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Tested-by: Linux Kernel Functional Testing
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
v3:
- Rebased to v6.9-rc2
include/kunit/bug.h | 7 ++-
lib/kunit/bug.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
index bd0fe047572b..72e9fb23bbd5 100644
--- a/include/kunit/bug.h
+++ b/include/kunit/bug.h
@@ -20,6 +20,7 @@
struct __suppressed_warning {
struct list_head node;
const char *function;
+ int counter;
};
void __start_suppress_warning(struct __suppressed_warning *warning);
@@ -28,7 +29,7 @@ bool __is_suppressed_warning(const char *function);
#define DEFINE_SUPPRESSED_WARNING(func)\
struct __suppressed_warning __kunit_suppress_##func = \
- { .function = __stringify(func) }
+ { .function = __stringify(func), .counter = 0 }
#define START_SUPPRESSED_WARNING(func) \
__start_suppress_warning(&__kunit_suppress_##func)
@@ -39,12 +40,16 @@ bool __is_suppressed_warning(const char *function);
#define IS_SUPPRESSED_WARNING(func) \
__is_suppressed_warning(func)
+#define SUPPRESSED_WARNING_COUNT(func) \
+ (__kunit_suppress_##func.counter)
+
#else /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#define DEFINE_SUPPRESSED_WARNING(func)
#define START_SUPPRESSED_WARNING(func)
#define END_SUPPRESSED_WARNING(func)
#define IS_SUPPRESSED_WARNING(func) (false)
+#define SUPPRESSED_WARNING_COUNT(func) (0)
#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
#endif /* __ASSEMBLY__ */
diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
index f93544d7a9d1..13b3d896c114 100644
--- a/lib/kunit/bug.c
+++ b/lib/kunit/bug.c
@@ -32,8 +32,10 @@ bool __is_suppressed_warning(const char *function)
return false;
list_for_each_entry(warning, _warnings, node) {
- if (!strcmp(function, warning->function))
+ if (!strcmp(function, warning->function)) {
+ warning->counter++;
return true;
+ }
}
return false;
}
--
2.39.2
[PATCH v3 01/15] bug/kunit: Core support for suppressing warning backtraces
Some unit tests intentionally trigger warning backtraces by passing
bad parameters to API functions. Such unit tests typically check the
return value from those calls, not the existence of the warning backtrace.
Such intentionally generated warning backtraces are neither desirable
nor useful for a number of reasons.
- They can result in overlooked real problems.
- A warning that suddenly starts to show up in unit tests needs to be
investigated and has to be marked to be ignored, for example by
adjusting filter scripts. Such filters are ad-hoc because there is
no real standard format for warnings. On top of that, such filter
scripts would require constant maintenance.
One option to address problem would be to add messages such as "expected
warning backtraces start / end here" to the kernel log. However, that
would again require filter scripts, it might result in missing real
problematic warning backtraces triggered while the test is running, and
the irrelevant backtrace(s) would still clog the kernel log.
Solve the problem by providing a means to identify and suppress specific
warning backtraces while executing test code. Since the new functionality
results in an image size increase of about 1% if CONFIG_KUNIT is enabled,
provide configuration option KUNIT_SUPPRESS_BACKTRACE to be able to disable
the new functionality. This option is by default enabled since almost all
systems with CONFIG_KUNIT enabled will want to benefit from it.
Cc: Dan Carpenter
Cc: Daniel Diaz
Cc: Naresh Kamboju
Cc: Kees Cook
Tested-by: Linux Kernel Functional Testing
Acked-by: Dan Carpenter
Reviewed-by: Kees Cook
Signed-off-by: Guenter Roeck
---
v2:
- Rebased to v6.9-rc1
- Added Tested-by:, Acked-by:, and Reviewed-by: tags
- Added CONFIG_KUNIT_SUPPRESS_BACKTRACE configuration option,
enabled by default
v3:
- Rebased to v6.9-rc2
include/asm-generic/bug.h | 16 +---
include/kunit/bug.h | 51 +++
include/kunit/test.h | 1 +
include/linux/bug.h | 13 ++
lib/bug.c | 51 ---
lib/kunit/Kconfig | 9 +++
lib/kunit/Makefile| 6 +++--
lib/kunit/bug.c | 40 ++
8 files changed, 178 insertions(+), 9 deletions(-)
create mode 100644 include/kunit/bug.h
create mode 100644 lib/kunit/bug.c
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 6e794420bd39..c170b6477689 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -18,6 +18,7 @@
#endif
#ifndef __ASSEMBLY__
+#include
#include
#include
@@ -39,8 +40,14 @@ struct bug_entry {
#ifdef CONFIG_DEBUG_BUGVERBOSE
#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
const char *file;
+#ifdef HAVE_BUG_FUNCTION
+ const char *function;
+#endif
#else
signed int file_disp;
+#ifdef HAVE_BUG_FUNCTION
+ signed int function_disp;
+#endif
#endif
unsigned short line;
#endif
@@ -96,15 +103,18 @@ extern __printf(1, 2) void __warn_printk(const char *fmt,
...);
#define __WARN() __WARN_printf(TAINT_WARN, NULL)
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \
+ if (!IS_SUPPRESSED_WARNING(__func__)) \
+ warn_slowpath_fmt(__FILE__, __LINE__, taint, arg);\
instrumentation_end(); \
} while (0)
#else
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
#define __WARN_printf(taint, arg...) do { \
instrumentation_begin();\
- __warn_printk(arg); \
- __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
+ if (!IS_SUPPRESSED_WARNING(__func__)) { \
+ __warn_printk(arg); \
+ __WARN_FLAGS(BUGFLAG_NO_CUT_HERE |
BUGFLAG_TAINT(taint));\
+ } \
instrumentation_end(); \
} while (0)
#define WARN_ON_ONCE(condition) ({ \
diff --git a/include/kunit/bug.h b/include/kunit/bug.h
new file mode 100644
index ..bd0fe047572b
--- /dev/null
+++ b/include/kunit/bug.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit helpers for backtrace suppression
+ *
+ * Copyright (c) 2024 Guenter Roeck
+ */
+
+#ifndef _KUNIT_BUG_H
+#define _KUNIT_BUG_H
+
+#ifndef __ASSEMBLY__
+
+#include
+
+#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
+
+#include
+#include
+
+struct __suppressed_warning {
+ struct list_head node;
+
[PATCH v3 00/15] Add support for suppressing warning backtraces
Some unit tests intentionally trigger warning backtraces by passing bad parameters to kernel API functions. Such unit tests typically check the return value from such calls, not the existence of the warning backtrace. Such intentionally generated warning backtraces are neither desirable nor useful for a number of reasons. - They can result in overlooked real problems. - A warning that suddenly starts to show up in unit tests needs to be investigated and has to be marked to be ignored, for example by adjusting filter scripts. Such filters are ad-hoc because there is no real standard format for warnings. On top of that, such filter scripts would require constant maintenance. One option to address problem would be to add messages such as "expected warning backtraces start / end here" to the kernel log. However, that would again require filter scripts, it might result in missing real problematic warning backtraces triggered while the test is running, and the irrelevant backtrace(s) would still clog the kernel log. Solve the problem by providing a means to identify and suppress specific warning backtraces while executing test code. Support suppressing multiple backtraces while at the same time limiting changes to generic code to the absolute minimum. Architecture specific changes are kept at minimum by retaining function names only if both CONFIG_DEBUG_BUGVERBOSE and CONFIG_KUNIT are enabled. The first patch of the series introduces the necessary infrastructure. The second patch introduces support for counting suppressed backtraces. This capability is used in patch three to implement unit tests. Patch four documents the new API. The next two patches add support for suppressing backtraces in drm_rect and dev_addr_lists unit tests. These patches are intended to serve as examples for the use of the functionality introduced with this series. The remaining patches implement the necessary changes for all architectures with GENERIC_BUG support. With CONFIG_KUNIT enabled, image size increase with this series applied is approximately 1%. The image size increase (and with it the functionality introduced by this series) can be avoided by disabling CONFIG_KUNIT_SUPPRESS_BACKTRACE. This series is based on the RFC patch and subsequent discussion at https://patchwork.kernel.org/project/linux-kselftest/patch/02546e59-1afe-4b08-ba81-d94f3b691c9a@moroto.mountain/ and offers a more comprehensive solution of the problem discussed there. Design note: Function pointers are only added to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and CONFIG_DEBUG_BUGVERBOSE are enabled to avoid image size increases if CONFIG_KUNIT is disabled. There would be some benefits to adding those pointers all the time (reduced complexity, ability to display function names in BUG/WARNING messages). That change, if desired, can be made later. Checkpatch note: Remaining checkpatch errors and warnings were deliberately ignored. Some are triggered by matching coding style or by comments interpreted as code, others by assembler macros which are disliked by checkpatch. Suggestions for improvements are welcome. Changes since RFC: - Introduced CONFIG_KUNIT_SUPPRESS_BACKTRACE - Minor cleanups and bug fixes - Added support for all affected architectures - Added support for counting suppressed warnings - Added unit tests using those counters - Added patch to suppress warning backtraces in dev_addr_lists tests Changes since v1: - Rebased to v6.9-rc1 - Added Tested-by:, Acked-by:, and Reviewed-by: tags [I retained those tags since there have been no functional changes] - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option, enabled by default. Changes since v2: - Rebased to v6.9-rc2 - Added comments to drm warning suppression explaining why it is needed. - Added patch to move conditional code in arch/sh/include/asm/bug.h to avoid kerneldoc warning - Added architecture maintainers to Cc: for architecture specific patches - No functional changes Guenter Roeck (15): bug/kunit: Core support for suppressing warning backtraces kunit: bug: Count suppressed warning backtraces kunit: Add test cases for backtrace warning suppression kunit: Add documentation for warning backtrace suppression API drm: Suppress intentional warning backtraces in scaling unit tests net: kunit: Suppress lock warning noise at end of dev_addr_lists tests x86: Add support for suppressing warning backtraces arm64: Add support for suppressing warning backtraces loongarch: Add support for suppressing warning backtraces parisc: Add support for suppressing warning backtraces s390: Add support for suppressing warning backtraces sh: Add support for suppressing warning backtraces sh: Move defines needed for suppressing warning backtraces riscv: Add support for suppressing warning backtraces
Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback
Le 03/04/2024 à 15:07, Jason Gunthorpe a écrit :
> On Wed, Apr 03, 2024 at 12:26:43PM +, Christophe Leroy wrote:
>>
>>
>> Le 03/04/2024 à 14:08, Jason Gunthorpe a écrit :
>>> On Tue, Apr 02, 2024 at 07:35:45PM -0400, Peter Xu wrote:
On Tue, Apr 02, 2024 at 07:53:20PM -0300, Jason Gunthorpe wrote:
> On Tue, Apr 02, 2024 at 06:43:56PM -0400, Peter Xu wrote:
>
>> I actually tested this without hitting the issue (even though I didn't
>> mention it in the cover letter..). I re-kicked the build test, it turns
>> out my "make alldefconfig" on loongarch will generate a config with both
>> HUGETLB=n && THP=n, while arch/loongarch/configs/loongson3_defconfig has
>> THP=y (which I assume was the one above build used). I didn't further
>> check how "make alldefconfig" generated the config; a bit surprising that
>> it didn't fetch from there.
>
> I suspect it is weird compiler variations.. Maybe something is not
> being inlined.
>
>> (and it also surprises me that this BUILD_BUG can trigger.. I used to try
>>triggering it elsewhere but failed..)
>
> As the pud_leaf() == FALSE should result in the BUILD_BUG never being
> called and the optimizer removing it.
Good point, for some reason loongarch defined pud_leaf() without defining
pud_pfn(), which does look strange.
#define pud_leaf(pud) ((pud_val(pud) & _PAGE_HUGE) != 0)
But I noticed at least MIPS also does it.. Logically I think one arch
should define either none of both.
>>>
>>> Wow, this is definately an arch issue. You can't define pud_leaf() and
>>> not have a pud_pfn(). It makes no sense at all..
>>>
>>> I'd say the BUILD_BUG has done it's job and found an issue, fix it by
>>> not defining pud_leaf? I don't see any calls to pud_leaf in loongarch
>>> at least
>>
>> As far as I can see it was added by commit 303be4b33562 ("LoongArch: mm:
>> Add p?d_leaf() definitions").
>
> That commit makes it sounds like the arch supports huge PUD's through
> the hugepte mechanism - it says a LTP test failed so something
> populated a huge PUD at least??
Not sure, I more see it just like a copy/paste of commit 501b81046701
("mips: mm: add p?d_leaf() definitions").
The commit message says that the test failed because pmd_leaf() is
missing, it says nothing about PUD.
When looking where _PAGE_HUGE is used in loongarch, I have the
impression that it is exclusively used at PMD level.
>
> So maybe this?
>
> #define pud_pfn pte_pfn
>
>> Not sure it was added for a good reason, and I'm not sure what was added
>> is correct because arch/loongarch/include/asm/pgtable-bits.h has:
>>
>> #define _PAGE_HUGE_SHIFT6 /* HUGE is a PMD bit */
>>
>> So I'm not sure it is correct to use that bit for PUD, is it ?
>
> Could be, lots of arches repeat the bit layouts in each radix
> level.. It is essentially why the hugepte trick of pretending every
> level is a pte works.
>
> Jason
Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback
On Wed, Apr 03, 2024 at 12:26:43PM +, Christophe Leroy wrote:
>
>
> Le 03/04/2024 à 14:08, Jason Gunthorpe a écrit :
> > On Tue, Apr 02, 2024 at 07:35:45PM -0400, Peter Xu wrote:
> >> On Tue, Apr 02, 2024 at 07:53:20PM -0300, Jason Gunthorpe wrote:
> >>> On Tue, Apr 02, 2024 at 06:43:56PM -0400, Peter Xu wrote:
> >>>
> I actually tested this without hitting the issue (even though I didn't
> mention it in the cover letter..). I re-kicked the build test, it turns
> out my "make alldefconfig" on loongarch will generate a config with both
> HUGETLB=n && THP=n, while arch/loongarch/configs/loongson3_defconfig has
> THP=y (which I assume was the one above build used). I didn't further
> check how "make alldefconfig" generated the config; a bit surprising that
> it didn't fetch from there.
> >>>
> >>> I suspect it is weird compiler variations.. Maybe something is not
> >>> being inlined.
> >>>
> (and it also surprises me that this BUILD_BUG can trigger.. I used to try
> triggering it elsewhere but failed..)
> >>>
> >>> As the pud_leaf() == FALSE should result in the BUILD_BUG never being
> >>> called and the optimizer removing it.
> >>
> >> Good point, for some reason loongarch defined pud_leaf() without defining
> >> pud_pfn(), which does look strange.
> >>
> >> #define pud_leaf(pud) ((pud_val(pud) & _PAGE_HUGE) != 0)
> >>
> >> But I noticed at least MIPS also does it.. Logically I think one arch
> >> should define either none of both.
> >
> > Wow, this is definately an arch issue. You can't define pud_leaf() and
> > not have a pud_pfn(). It makes no sense at all..
> >
> > I'd say the BUILD_BUG has done it's job and found an issue, fix it by
> > not defining pud_leaf? I don't see any calls to pud_leaf in loongarch
> > at least
>
> As far as I can see it was added by commit 303be4b33562 ("LoongArch: mm:
> Add p?d_leaf() definitions").
That commit makes it sounds like the arch supports huge PUD's through
the hugepte mechanism - it says a LTP test failed so something
populated a huge PUD at least??
So maybe this?
#define pud_pfn pte_pfn
> Not sure it was added for a good reason, and I'm not sure what was added
> is correct because arch/loongarch/include/asm/pgtable-bits.h has:
>
> #define _PAGE_HUGE_SHIFT6 /* HUGE is a PMD bit */
>
> So I'm not sure it is correct to use that bit for PUD, is it ?
Could be, lots of arches repeat the bit layouts in each radix
level.. It is essentially why the hugepte trick of pretending every
level is a pte works.
Jason
Re: [PATCH v4 00/15] Unified cross-architecture kernel-mode FPU API
I only skimmed over the platform patches and spend only a few minutes on the amdgpu stuff. From what I've seen this series seems to make perfect sense to me, I just can't fully judge everything. So feel free to add Acked-by: Christian König but I strongly suggest that Harry and Rodrigo take a look as well. Regards, Christian. Am 29.03.24 um 08:18 schrieb Samuel Holland: This series unifies the kernel-mode FPU API across several architectures by wrapping the existing functions (where needed) in consistently-named functions placed in a consistent header location, with mostly the same semantics: they can be called from preemptible or non-preemptible task context, and are not assumed to be reentrant. Architectures are also expected to provide CFLAGS adjustments for compiling FPU-dependent code. For the moment, SIMD/vector units are out of scope for this common API. This allows us to remove the ifdeffery and duplicated Makefile logic at each FPU user. It then implements the common API on RISC-V, and converts a couple of users to the new API: the AMDGPU DRM driver, and the FPU self test. The underlying goal of this series is to allow using newer AMD GPUs (e.g. Navi) on RISC-V boards such as SiFive's HiFive Unmatched. Those GPUs need CONFIG_DRM_AMD_DC_FP to initialize, which requires kernel-mode FPU support. Previous versions: v3: https://lore.kernel.org/linux-kernel/20240327200157.1097089-1-samuel.holl...@sifive.com/ v2: https://lore.kernel.org/linux-kernel/20231228014220.3562640-1-samuel.holl...@sifive.com/ v1: https://lore.kernel.org/linux-kernel/20231208055501.2916202-1-samuel.holl...@sifive.com/ v0: https://lore.kernel.org/linux-kernel/20231122030621.3759313-1-samuel.holl...@sifive.com/ Changes in v4: - Add missed CFLAGS changes for recov_neon_inner.c (fixes arm build failures) - Fix x86 include guard issue (fixes x86 build failures) Changes in v3: - Rebase on v6.9-rc1 - Limit riscv ARCH_HAS_KERNEL_FPU_SUPPORT to 64BIT Changes in v2: - Add documentation explaining the built-time and runtime APIs - Add a linux/fpu.h header for generic isolation enforcement - Remove file name from header comment - Clean up arch/arm64/lib/Makefile, like for arch/arm - Remove RISC-V architecture-specific preprocessor check - Split altivec removal to a separate patch - Use linux/fpu.h instead of asm/fpu.h in consumers - Declare test_fpu() in a header Michael Ellerman (1): drm/amd/display: Only use hard-float, not altivec on powerpc Samuel Holland (14): arch: Add ARCH_HAS_KERNEL_FPU_SUPPORT ARM: Implement ARCH_HAS_KERNEL_FPU_SUPPORT ARM: crypto: Use CC_FLAGS_FPU for NEON CFLAGS arm64: Implement ARCH_HAS_KERNEL_FPU_SUPPORT arm64: crypto: Use CC_FLAGS_FPU for NEON CFLAGS lib/raid6: Use CC_FLAGS_FPU for NEON CFLAGS LoongArch: Implement ARCH_HAS_KERNEL_FPU_SUPPORT powerpc: Implement ARCH_HAS_KERNEL_FPU_SUPPORT x86/fpu: Fix asm/fpu/types.h include guard x86: Implement ARCH_HAS_KERNEL_FPU_SUPPORT riscv: Add support for kernel-mode FPU drm/amd/display: Use ARCH_HAS_KERNEL_FPU_SUPPORT selftests/fpu: Move FP code to a separate translation unit selftests/fpu: Allow building on other architectures Documentation/core-api/floating-point.rst | 78 +++ Documentation/core-api/index.rst | 1 + Makefile | 5 ++ arch/Kconfig | 6 ++ arch/arm/Kconfig | 1 + arch/arm/Makefile | 7 ++ arch/arm/include/asm/fpu.h| 15 arch/arm/lib/Makefile | 3 +- arch/arm64/Kconfig| 1 + arch/arm64/Makefile | 9 ++- arch/arm64/include/asm/fpu.h | 15 arch/arm64/lib/Makefile | 6 +- arch/loongarch/Kconfig| 1 + arch/loongarch/Makefile | 5 +- arch/loongarch/include/asm/fpu.h | 1 + arch/powerpc/Kconfig | 1 + arch/powerpc/Makefile | 5 +- arch/powerpc/include/asm/fpu.h| 28 +++ arch/riscv/Kconfig| 1 + arch/riscv/Makefile | 3 + arch/riscv/include/asm/fpu.h | 16 arch/riscv/kernel/Makefile| 1 + arch/riscv/kernel/kernel_mode_fpu.c | 28 +++ arch/x86/Kconfig | 1 + arch/x86/Makefile | 20 + arch/x86/include/asm/fpu.h| 13 arch/x86/include/asm/fpu/types.h | 6 +- drivers/gpu/drm/amd/display/Kconfig | 2 +- .../gpu/drm/amd/display/amdgpu_dm/dc_fpu.c| 35 + drivers/gpu/drm/amd/display/dc/dml/Makefile | 36 + drivers/gpu/drm/amd/display/dc/dml2/Makefile | 36
Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback
Le 03/04/2024 à 14:08, Jason Gunthorpe a écrit :
> On Tue, Apr 02, 2024 at 07:35:45PM -0400, Peter Xu wrote:
>> On Tue, Apr 02, 2024 at 07:53:20PM -0300, Jason Gunthorpe wrote:
>>> On Tue, Apr 02, 2024 at 06:43:56PM -0400, Peter Xu wrote:
>>>
I actually tested this without hitting the issue (even though I didn't
mention it in the cover letter..). I re-kicked the build test, it turns
out my "make alldefconfig" on loongarch will generate a config with both
HUGETLB=n && THP=n, while arch/loongarch/configs/loongson3_defconfig has
THP=y (which I assume was the one above build used). I didn't further
check how "make alldefconfig" generated the config; a bit surprising that
it didn't fetch from there.
>>>
>>> I suspect it is weird compiler variations.. Maybe something is not
>>> being inlined.
>>>
(and it also surprises me that this BUILD_BUG can trigger.. I used to try
triggering it elsewhere but failed..)
>>>
>>> As the pud_leaf() == FALSE should result in the BUILD_BUG never being
>>> called and the optimizer removing it.
>>
>> Good point, for some reason loongarch defined pud_leaf() without defining
>> pud_pfn(), which does look strange.
>>
>> #define pud_leaf(pud)((pud_val(pud) & _PAGE_HUGE) != 0)
>>
>> But I noticed at least MIPS also does it.. Logically I think one arch
>> should define either none of both.
>
> Wow, this is definately an arch issue. You can't define pud_leaf() and
> not have a pud_pfn(). It makes no sense at all..
>
> I'd say the BUILD_BUG has done it's job and found an issue, fix it by
> not defining pud_leaf? I don't see any calls to pud_leaf in loongarch
> at least
As far as I can see it was added by commit 303be4b33562 ("LoongArch: mm:
Add p?d_leaf() definitions").
Not sure it was added for a good reason, and I'm not sure what was added
is correct because arch/loongarch/include/asm/pgtable-bits.h has:
#define _PAGE_HUGE_SHIFT6 /* HUGE is a PMD bit */
So I'm not sure it is correct to use that bit for PUD, is it ?
Probably pud_leaf() should always return false.
Christophe
Re: [PATCH v4 05/13] mm/arch: Provide pud_pfn() fallback
On Tue, Apr 02, 2024 at 07:35:45PM -0400, Peter Xu wrote: > On Tue, Apr 02, 2024 at 07:53:20PM -0300, Jason Gunthorpe wrote: > > On Tue, Apr 02, 2024 at 06:43:56PM -0400, Peter Xu wrote: > > > > > I actually tested this without hitting the issue (even though I didn't > > > mention it in the cover letter..). I re-kicked the build test, it turns > > > out my "make alldefconfig" on loongarch will generate a config with both > > > HUGETLB=n && THP=n, while arch/loongarch/configs/loongson3_defconfig has > > > THP=y (which I assume was the one above build used). I didn't further > > > check how "make alldefconfig" generated the config; a bit surprising that > > > it didn't fetch from there. > > > > I suspect it is weird compiler variations.. Maybe something is not > > being inlined. > > > > > (and it also surprises me that this BUILD_BUG can trigger.. I used to try > > > triggering it elsewhere but failed..) > > > > As the pud_leaf() == FALSE should result in the BUILD_BUG never being > > called and the optimizer removing it. > > Good point, for some reason loongarch defined pud_leaf() without defining > pud_pfn(), which does look strange. > > #define pud_leaf(pud) ((pud_val(pud) & _PAGE_HUGE) != 0) > > But I noticed at least MIPS also does it.. Logically I think one arch > should define either none of both. Wow, this is definately an arch issue. You can't define pud_leaf() and not have a pud_pfn(). It makes no sense at all.. I'd say the BUILD_BUG has done it's job and found an issue, fix it by not defining pud_leaf? I don't see any calls to pud_leaf in loongarch at least Jason
Re: [PATCH v1 1/1] powerpc/52xx: Replace of_gpio.h by proper one
On Wed, Mar 13, 2024 at 03:56:45PM +0200, Andy Shevchenko wrote: > of_gpio.h is deprecated and subject to remove. > The driver doesn't use it directly, replace it > with what is really being used. Any comments on this? -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 3/3] usb: typec: displayport: drop driver owner assignment
On Sun, Mar 31, 2024 at 11:17:37AM +0200, Krzysztof Kozlowski wrote:
> Core in typec_altmode_register_driver() already sets the .owner, so
> driver does not need to.
>
> Signed-off-by: Krzysztof Kozlowski
Reviewed-by: Heikki Krogerus
> ---
>
> Changes in v2:
> 1. New patch
> ---
> drivers/usb/typec/altmodes/displayport.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/typec/altmodes/displayport.c
> b/drivers/usb/typec/altmodes/displayport.c
> index 038dc51f429d..596cd4806018 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -802,7 +802,6 @@ static struct typec_altmode_driver dp_altmode_driver = {
> .remove = dp_altmode_remove,
> .driver = {
> .name = "typec_displayport",
> - .owner = THIS_MODULE,
> .dev_groups = displayport_groups,
> },
> };
> --
> 2.34.1
--
heikki
Re: [PATCH v2 2/3] usb: typec: nvidia: drop driver owner assignment
On Sun, Mar 31, 2024 at 11:17:36AM +0200, Krzysztof Kozlowski wrote:
> Core in typec_altmode_register_driver() already sets the .owner, so
> driver does not need to.
>
> Signed-off-by: Krzysztof Kozlowski
Reviewed-by: Heikki Krogerus
> ---
>
> Changes in v2:
> 1. None
> ---
> drivers/usb/typec/altmodes/nvidia.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/usb/typec/altmodes/nvidia.c
> b/drivers/usb/typec/altmodes/nvidia.c
> index c36769736405..fe70b36f078f 100644
> --- a/drivers/usb/typec/altmodes/nvidia.c
> +++ b/drivers/usb/typec/altmodes/nvidia.c
> @@ -35,7 +35,6 @@ static struct typec_altmode_driver nvidia_altmode_driver = {
> .remove = nvidia_altmode_remove,
> .driver = {
> .name = "typec_nvidia",
> - .owner = THIS_MODULE,
> },
> };
> module_typec_altmode_driver(nvidia_altmode_driver);
> --
> 2.34.1
--
heikki
[PATCH v2 2/2] PCI: Create helper to print TLP Header and Prefix Log
Add pcie_print_tlp_log() helper to print TLP Header and Prefix Log.
Print End-End Prefixes only if they are non-zero.
Consolidate the few places which currently print TLP using custom
formatting.
The first attempt used pr_cont() instead of building a string first but
it turns out pr_cont() is not compatible with pci_err() but prints on a
separate line. When I asked about this, Andy Shevchenko suggested
pr_cont() should not be used in the first place (to eventually get rid
of it) so pr_cont() is now replaced with building the string first.
Signed-off-by: Ilpo Järvinen
---
drivers/pci/pci.c | 32
drivers/pci/pcie/aer.c | 10 ++
drivers/pci/pcie/dpc.c | 5 +
include/linux/aer.h| 2 ++
4 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index af230e6e5557..54d4872d14b8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -9,6 +9,7 @@
*/
#include
+#include
#include
#include
#include
@@ -1116,6 +1117,37 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where,
int where2,
}
EXPORT_SYMBOL_GPL(pcie_read_tlp_log);
+/**
+ * pcie_print_tlp_log - Print TLP Header / Prefix Log contents
+ * @dev: PCIe device
+ * @tlp_log: TLP Log structure
+ * @pfx: Internal string prefix (for indentation)
+ *
+ * Prints TLP Header and Prefix Log information held by @tlp_log.
+ */
+void pcie_print_tlp_log(const struct pci_dev *dev,
+ const struct pcie_tlp_log *tlp_log, const char *pfx)
+{
+ char buf[(10 + 1) * (4 + ARRAY_SIZE(tlp_log->prefix)) + 14 + 1];
+ unsigned int i;
+ int len;
+
+ len = scnprintf(buf, sizeof(buf), "%#010x %#010x %#010x %#010x",
+ tlp_log->dw[0], tlp_log->dw[1], tlp_log->dw[2],
+ tlp_log->dw[3]);
+
+ if (tlp_log->prefix[0])
+ len += scnprintf(buf + len, sizeof(buf) - len, " E-E
Prefixes:");
+ for (i = 0; i < ARRAY_SIZE(tlp_log->prefix); i++) {
+ if (!tlp_log->prefix[i])
+ break;
+ len += scnprintf(buf + len, sizeof(buf) - len,
+" %#010x", tlp_log->prefix[i]);
+ }
+
+ pci_err(dev, "%sTLP Header: %s\n", pfx, buf);
+}
+
/**
* pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
* @dev: PCI device to have its BARs restored
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ecc1dea5a208..efb9e728fe94 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -664,12 +664,6 @@ static void pci_rootport_aer_stats_incr(struct pci_dev
*pdev,
}
}
-static void __print_tlp_header(struct pci_dev *dev, struct pcie_tlp_log *t)
-{
- pci_err(dev, " TLP Header: %08x %08x %08x %08x\n",
- t->dw[0], t->dw[1], t->dw[2], t->dw[3]);
-}
-
static void __aer_print_error(struct pci_dev *dev,
struct aer_err_info *info)
{
@@ -724,7 +718,7 @@ void aer_print_error(struct pci_dev *dev, struct
aer_err_info *info)
__aer_print_error(dev, info);
if (info->tlp_header_valid)
- __print_tlp_header(dev, >tlp);
+ pcie_print_tlp_log(dev, >tlp, " ");
out:
if (info->id && info->error_dev_num > 1 && info->id == id)
@@ -796,7 +790,7 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
aer->uncor_severity);
if (tlp_header_valid)
- __print_tlp_header(dev, >header_log);
+ pcie_print_tlp_log(dev, >header_log, " ");
trace_aer_event(dev_name(>dev), (status & ~mask),
aer_severity, tlp_header_valid, >header_log);
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 80b1456f95fe..3f8e3b6c7948 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -229,10 +229,7 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
pcie_read_tlp_log(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
dpc_tlp_log_len(pdev), _log);
- pci_err(pdev, "TLP Header: %#010x %#010x %#010x %#010x\n",
- tlp_log.dw[0], tlp_log.dw[1], tlp_log.dw[2], tlp_log.dw[3]);
- for (i = 0; i < pdev->dpc_rp_log_size - 5; i++)
- pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i,
tlp_log.prefix[i]);
+ pcie_print_tlp_log(pdev, _log, "");
if (pdev->dpc_rp_log_size < 5)
goto clear_status;
diff --git a/include/linux/aer.h b/include/linux/aer.h
index 2484056feb8d..1e8c61deca65 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -41,6 +41,8 @@ struct aer_capability_regs {
int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
unsigned int tlp_len, struct pcie_tlp_log *log);
unsigned int aer_tlp_log_len(struct pci_dev *dev);
+void pcie_print_tlp_log(const
[PATCH v2 1/2] PCI: Add TLP Prefix reading into pcie_read_tlp_log()
pcie_read_tlp_log() handles only 4 TLP Header Log DWORDs but TLP Prefix
Log (PCIe r6.1 secs 7.8.4.12 & 7.9.14.13) may also be present.
Generalize pcie_read_tlp_log() and struct pcie_tlp_log to handle also
TLP Prefix Log. The layout of relevant registers in AER and DPC
Capability is not identical but the offsets of TLP Header Log and TLP
Prefix Log vary so the callers must pass the offsets to
pcie_read_tlp_log().
Convert eetlp_prefix_path into integer called eetlp_prefix_max and
make is available also when CONFIG_PCI_PASID is not configured to
be able to determine the number of E-E Prefixes.
Signed-off-by: Ilpo Järvinen
---
drivers/pci/ats.c | 2 +-
drivers/pci/pci.c | 34 --
drivers/pci/pcie/aer.c| 4 +++-
drivers/pci/pcie/dpc.c| 22 +++---
drivers/pci/probe.c | 14 +-
include/linux/aer.h | 5 -
include/linux/pci.h | 2 +-
include/uapi/linux/pci_regs.h | 2 ++
8 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index c570892b2090..e13433dcfc82 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -377,7 +377,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
if (WARN_ON(pdev->pasid_enabled))
return -EBUSY;
- if (!pdev->eetlp_prefix_path && !pdev->pasid_no_tlp)
+ if (!pdev->eetlp_prefix_max && !pdev->pasid_no_tlp)
return -EINVAL;
if (!pasid)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e5f243dd4288..af230e6e5557 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1066,26 +1066,48 @@ static void pci_enable_acs(struct pci_dev *dev)
pci_disable_acs_redir(dev);
}
+/**
+ * aer_tlp_log_len - Calculates TLP Header/Prefix Log length
+ * @dev: PCIe device
+ *
+ * Return: TLP Header/Prefix Log length
+ */
+unsigned int aer_tlp_log_len(struct pci_dev *dev)
+{
+ return 4 + dev->eetlp_prefix_max;
+}
+
/**
* pcie_read_tlp_log - read TLP Header Log
* @dev: PCIe device
* @where: PCI Config offset of TLP Header Log
+ * @where2: PCI Config offset of TLP Prefix Log
+ * @tlp_len: TLP Log length (in DWORDs)
* @tlp_log: TLP Log structure to fill
*
* Fill @tlp_log from TLP Header Log registers, e.g., AER or DPC.
*
* Return: 0 on success and filled TLP Log structure, <0 on error.
*/
-int pcie_read_tlp_log(struct pci_dev *dev, int where,
- struct pcie_tlp_log *tlp_log)
+int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
+ unsigned int tlp_len, struct pcie_tlp_log *tlp_log)
{
- int i, ret;
+ unsigned int i;
+ int off, ret;
+ u32 *to;
memset(tlp_log, 0, sizeof(*tlp_log));
- for (i = 0; i < 4; i++) {
- ret = pci_read_config_dword(dev, where + i * 4,
- _log->dw[i]);
+ for (i = 0; i < tlp_len; i++) {
+ if (i < 4) {
+ to = _log->dw[i];
+ off = where + i * 4;
+ } else {
+ to = _log->prefix[i - 4];
+ off = where2 + (i - 4) * 4;
+ }
+
+ ret = pci_read_config_dword(dev, off, to);
if (ret)
return pcibios_err_to_errno(ret);
}
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ac6293c24976..ecc1dea5a208 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1245,7 +1245,9 @@ int aer_get_device_error_info(struct pci_dev *dev, struct
aer_err_info *info)
if (info->status & AER_LOG_TLP_MASKS) {
info->tlp_header_valid = 1;
- pcie_read_tlp_log(dev, aer + PCI_ERR_HEADER_LOG,
>tlp);
+ pcie_read_tlp_log(dev, aer + PCI_ERR_HEADER_LOG,
+ aer + PCI_ERR_PREFIX_LOG,
+ aer_tlp_log_len(dev), >tlp);
}
}
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index a668820696dc..80b1456f95fe 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -187,10 +187,19 @@ pci_ers_result_t dpc_reset_link(struct pci_dev *pdev)
return ret;
}
+static unsigned int dpc_tlp_log_len(struct pci_dev *pdev)
+{
+ /* Remove ImpSpec Log register from the count */
+ if (pdev->dpc_rp_log_size >= 5)
+ return pdev->dpc_rp_log_size - 1;
+
+ return pdev->dpc_rp_log_size;
+}
+
static void dpc_process_rp_pio_error(struct pci_dev *pdev)
{
u16 cap = pdev->dpc_cap, dpc_status, first_error;
- u32 status, mask, sev, syserr, exc, log, prefix;
+ u32 status, mask, sev, syserr, exc, log;
struct pcie_tlp_log tlp_log;
int i;
@@ -217,20 +226,19 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
[PATCH v2 0/2] PCI: Consolidate TLP Log reading and printing
This series has the remaining patches of the AER & DPC TLP Log handling consolidation. v2: - Don't add EXPORT()s - Don't include igxbe changes - Don't use pr_cont() as it's incompatible with pci_err() and according to Andy Shevchenko should not be used in the first place Ilpo Järvinen (2): PCI: Add TLP Prefix reading into pcie_read_tlp_log() PCI: Create helper to print TLP Header and Prefix Log drivers/pci/ats.c | 2 +- drivers/pci/pci.c | 66 +++ drivers/pci/pcie/aer.c| 14 +++- drivers/pci/pcie/dpc.c| 23 +++- drivers/pci/probe.c | 14 +--- include/linux/aer.h | 7 +++- include/linux/pci.h | 2 +- include/uapi/linux/pci_regs.h | 2 ++ 8 files changed, 98 insertions(+), 32 deletions(-) -- 2.39.2
[PATCH 00/34] address all -Wunused-const warnings
From: Arnd Bergmann
Compilers traditionally warn for unused 'static' variables, but not
if they are constant. The reason here is a custom for C++ programmers
to define named constants as 'static const' variables in header files
instead of using macros or enums.
In W=1 builds, we get warnings only static const variables in C
files, but not in headers, which is a good compromise, but this still
produces warning output in at least 30 files. These warnings are
almost all harmless, but also trivial to fix, and there is no
good reason to warn only about the non-const variables being unused.
I've gone through all the files that I found using randconfig and
allmodconfig builds and created patches to avoid these warnings,
with the goal of retaining a clean build once the option is enabled
by default.
Unfortunately, there is one fairly large patch ("drivers: remove
incorrect of_match_ptr/ACPI_PTR annotations") that touches
34 individual drivers that all need the same one-line change.
If necessary, I can split it up by driver or by subsystem,
but at least for reviewing I would keep it as one piece for
the moment.
Please merge the individual patches through subsystem trees.
I expect that some of these will have to go through multiple
revisions before they are picked up, so anything that gets
applied early saves me from resending.
Arnd
Arnd Bergmann (31):
powerpc/fsl-soc: hide unused const variable
ubsan: fix unused variable warning in test module
platform: goldfish: remove ACPI_PTR() annotations
i2c: pxa: hide unused icr_bits[] variable
3c515: remove unused 'mtu' variable
tracing: hide unused ftrace_event_id_fops
Input: synaptics: hide unused smbus_pnp_ids[] array
power: rt9455: hide unused rt9455_boost_voltage_values
efi: sysfb: don't build when EFI is disabled
clk: ti: dpll: fix incorrect #ifdef checks
apm-emulation: hide an unused variable
sisfb: hide unused variables
dma/congiguous: avoid warning about unused size_bytes
leds: apu: remove duplicate DMI lookup data
iio: ad5755: hook up of_device_id lookup to platform driver
greybus: arche-ctrl: move device table to its right location
lib: checksum: hide unused expected_csum_ipv6_magic[]
sunrpc: suppress warnings for unused procfs functions
comedi: ni_atmio: avoid warning for unused device_ids[] table
iwlegacy: don't warn for unused variables with DEBUG_FS=n
drm/komeda: don't warn for unused debugfs files
firmware: qcom_scm: mark qcom_scm_qseecom_allowlist as __maybe_unused
crypto: ccp - drop platform ifdef checks
usb: gadget: omap_udc: remove unused variable
isdn: kcapi: don't build unused procfs code
cpufreq: intel_pstate: hide unused intel_pstate_cpu_oob_ids[]
net: xgbe: remove extraneous #ifdef checks
Input: imagis - remove incorrect ifdef checks
sata: mv: drop unnecessary #ifdef checks
ASoC: remove incorrect of_match_ptr/ACPI_PTR annotations
spi: remove incorrect of_match_ptr annotations
drivers: remove incorrect of_match_ptr/ACPI_PTR annotations
kbuild: always enable -Wunused-const-variable
Krzysztof Kozlowski (1):
Input: stmpe-ts - mark OF related data as maybe unused
arch/powerpc/sysdev/fsl_msi.c | 2 +
drivers/ata/sata_mv.c | 64 +--
drivers/char/apm-emulation.c | 5 +-
drivers/char/ipmi/ipmb_dev_int.c | 2 +-
drivers/char/tpm/tpm_ftpm_tee.c | 2 +-
drivers/clk/ti/dpll.c | 10 ++-
drivers/comedi/drivers/ni_atmio.c | 2 +-
drivers/cpufreq/intel_pstate.c| 2 +
drivers/crypto/ccp/sp-platform.c | 14 +---
drivers/dma/img-mdc-dma.c | 2 +-
drivers/firmware/efi/Makefile | 3 +-
drivers/firmware/efi/sysfb_efi.c | 2 -
drivers/firmware/qcom/qcom_scm.c | 2 +-
drivers/fpga/versal-fpga.c| 2 +-
.../gpu/drm/arm/display/komeda/komeda_dev.c | 8 ---
drivers/hid/hid-google-hammer.c | 6 +-
drivers/i2c/busses/i2c-pxa.c | 2 +-
drivers/i2c/muxes/i2c-mux-ltc4306.c | 2 +-
drivers/i2c/muxes/i2c-mux-reg.c | 2 +-
drivers/iio/dac/ad5755.c | 1 +
drivers/input/mouse/synaptics.c | 2 +
drivers/input/touchscreen/imagis.c| 4 +-
drivers/input/touchscreen/stmpe-ts.c | 2 +-
drivers/input/touchscreen/wdt87xx_i2c.c | 2 +-
drivers/isdn/capi/Makefile| 3 +-
drivers/isdn/capi/kcapi.c | 7 +-
drivers/leds/leds-apu.c | 3 +-
drivers/mux/adg792a.c | 2 +-
drivers/net/ethernet/3com/3c515.c | 3 -
drivers/net/ethernet/amd/xgbe/xgbe-platform.c | 8 ---
drivers/net/ethernet/apm/xgene-v2/main.c | 2 +-
drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +-
drivers/net/wireless/intel/iwlegacy/4965-rs.c | 15 +
[PATCH v2 7/7] x86: mm: accelerate pagefault when badaccess
The access_error() of vma already checked under per-VMA lock, if it
is a bad access, directly handle error, no need to retry with mmap_lock
again. In order to release the correct lock, pass the mm_struct into
bad_area_access_error(), if mm is NULL, release vma lock, or release
mmap_lock. Since the page faut is handled under per-VMA lock, count it
as a vma lock event with VMA_LOCK_SUCCESS.
Reviewed-by: Suren Baghdasaryan
Signed-off-by: Kefeng Wang
---
arch/x86/mm/fault.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a4cc20d0036d..67b18adc75dd 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -866,14 +866,17 @@ bad_area_nosemaphore(struct pt_regs *regs, unsigned long
error_code,
static void
__bad_area(struct pt_regs *regs, unsigned long error_code,
- unsigned long address, u32 pkey, int si_code)
+ unsigned long address, struct mm_struct *mm,
+ struct vm_area_struct *vma, u32 pkey, int si_code)
{
- struct mm_struct *mm = current->mm;
/*
* Something tried to access memory that isn't in our memory map..
* Fix it, but check if it's kernel or user first..
*/
- mmap_read_unlock(mm);
+ if (mm)
+ mmap_read_unlock(mm);
+ else
+ vma_end_read(vma);
__bad_area_nosemaphore(regs, error_code, address, pkey, si_code);
}
@@ -897,7 +900,8 @@ static inline bool bad_area_access_from_pkeys(unsigned long
error_code,
static noinline void
bad_area_access_error(struct pt_regs *regs, unsigned long error_code,
- unsigned long address, struct vm_area_struct *vma)
+ unsigned long address, struct mm_struct *mm,
+ struct vm_area_struct *vma)
{
/*
* This OSPKE check is not strictly necessary at runtime.
@@ -927,9 +931,9 @@ bad_area_access_error(struct pt_regs *regs, unsigned long
error_code,
*/
u32 pkey = vma_pkey(vma);
- __bad_area(regs, error_code, address, pkey, SEGV_PKUERR);
+ __bad_area(regs, error_code, address, mm, vma, pkey,
SEGV_PKUERR);
} else {
- __bad_area(regs, error_code, address, 0, SEGV_ACCERR);
+ __bad_area(regs, error_code, address, mm, vma, 0, SEGV_ACCERR);
}
}
@@ -1357,8 +1361,9 @@ void do_user_addr_fault(struct pt_regs *regs,
goto lock_mmap;
if (unlikely(access_error(error_code, vma))) {
- vma_end_read(vma);
- goto lock_mmap;
+ bad_area_access_error(regs, error_code, address, NULL, vma);
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ return;
}
fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK,
regs);
if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
@@ -1394,7 +1399,7 @@ void do_user_addr_fault(struct pt_regs *regs,
* we can handle it..
*/
if (unlikely(access_error(error_code, vma))) {
- bad_area_access_error(regs, error_code, address, vma);
+ bad_area_access_error(regs, error_code, address, mm, vma);
return;
}
--
2.27.0
[PATCH v2 5/7] riscv: mm: accelerate pagefault when badaccess
The access_error() of vma already checked under per-VMA lock, if it
is a bad access, directly handle error, no need to retry with mmap_lock
again. Since the page faut is handled under per-VMA lock, count it as
a vma lock event with VMA_LOCK_SUCCESS.
Reviewed-by: Suren Baghdasaryan
Signed-off-by: Kefeng Wang
---
arch/riscv/mm/fault.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 3ba1d4dde5dd..b3fcf7d67efb 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -292,7 +292,10 @@ void handle_page_fault(struct pt_regs *regs)
if (unlikely(access_error(cause, vma))) {
vma_end_read(vma);
- goto lock_mmap;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ tsk->thread.bad_cause = SEGV_ACCERR;
+ bad_area_nosemaphore(regs, code, addr);
+ return;
}
fault = handle_mm_fault(vma, addr, flags | FAULT_FLAG_VMA_LOCK, regs);
--
2.27.0
[PATCH v2 6/7] s390: mm: accelerate pagefault when badaccess
The vm_flags of vma already checked under per-VMA lock, if it is a
bad access, directly handle error, no need to retry with mmap_lock
again. Since the page faut is handled under per-VMA lock, count it
as a vma lock event with VMA_LOCK_SUCCESS.
Signed-off-by: Kefeng Wang
---
arch/s390/mm/fault.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index c421dd44ffbe..162ca2576fd4 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -325,7 +325,8 @@ static void do_exception(struct pt_regs *regs, int access)
goto lock_mmap;
if (!(vma->vm_flags & access)) {
vma_end_read(vma);
- goto lock_mmap;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ return handle_fault_error_nolock(regs, SEGV_ACCERR);
}
fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK,
regs);
if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
--
2.27.0
[PATCH v2 3/7] arm: mm: accelerate pagefault when VM_FAULT_BADACCESS
The vm_flags of vma already checked under per-VMA lock, if it is a
bad access, directly set fault to VM_FAULT_BADACCESS and handle error,
no need to retry with mmap_lock again. Since the page faut is handled
under per-VMA lock, count it as a vma lock event with VMA_LOCK_SUCCESS.
Reviewed-by: Suren Baghdasaryan
Signed-off-by: Kefeng Wang
---
arch/arm/mm/fault.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 439dc6a26bb9..5c4b417e24f9 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -294,7 +294,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct
pt_regs *regs)
if (!(vma->vm_flags & vm_flags)) {
vma_end_read(vma);
- goto lock_mmap;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ fault = VM_FAULT_BADACCESS;
+ goto bad_area;
}
fault = handle_mm_fault(vma, addr, flags | FAULT_FLAG_VMA_LOCK, regs);
if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
--
2.27.0
[PATCH v2 4/7] powerpc: mm: accelerate pagefault when badaccess
The access_[pkey]_error() of vma already checked under per-VMA lock, if
it is a bad access, directly handle error, no need to retry with mmap_lock
again. In order to release the correct lock, pass the mm_struct into
bad_access_pkey()/bad_access(), if mm is NULL, release vma lock, or
release mmap_lock. Since the page faut is handled under per-VMA lock,
count it as a vma lock event with VMA_LOCK_SUCCESS.
Signed-off-by: Kefeng Wang
---
arch/powerpc/mm/fault.c | 33 -
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 53335ae21a40..215690452495 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -71,23 +71,26 @@ static noinline int bad_area_nosemaphore(struct pt_regs
*regs, unsigned long add
return __bad_area_nosemaphore(regs, address, SEGV_MAPERR);
}
-static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code)
+static int __bad_area(struct pt_regs *regs, unsigned long address, int si_code,
+ struct mm_struct *mm, struct vm_area_struct *vma)
{
- struct mm_struct *mm = current->mm;
/*
* Something tried to access memory that isn't in our memory map..
* Fix it, but check if it's kernel or user first..
*/
- mmap_read_unlock(mm);
+ if (mm)
+ mmap_read_unlock(mm);
+ else
+ vma_end_read(vma);
return __bad_area_nosemaphore(regs, address, si_code);
}
static noinline int bad_access_pkey(struct pt_regs *regs, unsigned long
address,
+ struct mm_struct *mm,
struct vm_area_struct *vma)
{
- struct mm_struct *mm = current->mm;
int pkey;
/*
@@ -109,7 +112,10 @@ static noinline int bad_access_pkey(struct pt_regs *regs,
unsigned long address,
*/
pkey = vma_pkey(vma);
- mmap_read_unlock(mm);
+ if (mm)
+ mmap_read_unlock(mm);
+ else
+ vma_end_read(vma);
/*
* If we are in kernel mode, bail out with a SEGV, this will
@@ -124,9 +130,10 @@ static noinline int bad_access_pkey(struct pt_regs *regs,
unsigned long address,
return 0;
}
-static noinline int bad_access(struct pt_regs *regs, unsigned long address)
+static noinline int bad_access(struct pt_regs *regs, unsigned long address,
+ struct mm_struct *mm, struct vm_area_struct *vma)
{
- return __bad_area(regs, address, SEGV_ACCERR);
+ return __bad_area(regs, address, SEGV_ACCERR, mm, vma);
}
static int do_sigbus(struct pt_regs *regs, unsigned long address,
@@ -479,13 +486,13 @@ static int ___do_page_fault(struct pt_regs *regs,
unsigned long address,
if (unlikely(access_pkey_error(is_write, is_exec,
(error_code & DSISR_KEYFAULT), vma))) {
- vma_end_read(vma);
- goto lock_mmap;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ return bad_access_pkey(regs, address, NULL, vma);
}
if (unlikely(access_error(is_write, is_exec, vma))) {
- vma_end_read(vma);
- goto lock_mmap;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ return bad_access(regs, address, NULL, vma);
}
fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK,
regs);
@@ -521,10 +528,10 @@ static int ___do_page_fault(struct pt_regs *regs,
unsigned long address,
if (unlikely(access_pkey_error(is_write, is_exec,
(error_code & DSISR_KEYFAULT), vma)))
- return bad_access_pkey(regs, address, vma);
+ return bad_access_pkey(regs, address, mm, vma);
if (unlikely(access_error(is_write, is_exec, vma)))
- return bad_access(regs, address);
+ return bad_access(regs, address, mm, vma);
/*
* If for any reason at all we couldn't handle the fault,
--
2.27.0
[PATCH v2 0/7] arch/mm/fault: accelerate pagefault when badaccess
After VMA lock-based page fault handling enabled, if bad access met under per-vma lock, it will fallback to mmap_lock-based handling, so it leads to unnessary mmap lock and vma find again. A test from lmbench shows 34% improve after this changes on arm64, lat_sig -P 1 prot lat_sig 0.29194 -> 0.19198 Only build test on other archs except arm64. v2: - a better changelog, and describe the counting changes, suggested by Suren Baghdasaryan - add RB Kefeng Wang (7): arm64: mm: cleanup __do_page_fault() arm64: mm: accelerate pagefault when VM_FAULT_BADACCESS arm: mm: accelerate pagefault when VM_FAULT_BADACCESS powerpc: mm: accelerate pagefault when badaccess riscv: mm: accelerate pagefault when badaccess s390: mm: accelerate pagefault when badaccess x86: mm: accelerate pagefault when badaccess arch/arm/mm/fault.c | 4 +++- arch/arm64/mm/fault.c | 31 ++- arch/powerpc/mm/fault.c | 33 - arch/riscv/mm/fault.c | 5 - arch/s390/mm/fault.c| 3 ++- arch/x86/mm/fault.c | 23 ++- 6 files changed, 53 insertions(+), 46 deletions(-) -- 2.27.0
[PATCH v2 2/7] arm64: mm: accelerate pagefault when VM_FAULT_BADACCESS
The vm_flags of vma already checked under per-VMA lock, if it is a
bad access, directly set fault to VM_FAULT_BADACCESS and handle error,
no need to retry with mmap_lock again, the latency time reduces 34% in
'lat_sig -P 1 prot lat_sig' from lmbench testcase.
Since the page faut is handled under per-VMA lock, count it as a vma lock
event with VMA_LOCK_SUCCESS.
Reviewed-by: Suren Baghdasaryan
Signed-off-by: Kefeng Wang
---
arch/arm64/mm/fault.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 9bb9f395351a..405f9aa831bd 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -572,7 +572,9 @@ static int __kprobes do_page_fault(unsigned long far,
unsigned long esr,
if (!(vma->vm_flags & vm_flags)) {
vma_end_read(vma);
- goto lock_mmap;
+ fault = VM_FAULT_BADACCESS;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ goto done;
}
fault = handle_mm_fault(vma, addr, mm_flags | FAULT_FLAG_VMA_LOCK,
regs);
if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
--
2.27.0
[PATCH v2 1/7] arm64: mm: cleanup __do_page_fault()
The __do_page_fault() only calls handle_mm_fault() after vm_flags
checked, and it is only called by do_page_fault(), let's squash
it into do_page_fault() to cleanup code.
Reviewed-by: Suren Baghdasaryan
Signed-off-by: Kefeng Wang
---
arch/arm64/mm/fault.c | 27 +++
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 8251e2fea9c7..9bb9f395351a 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -486,25 +486,6 @@ static void do_bad_area(unsigned long far, unsigned long
esr,
}
}
-#define VM_FAULT_BADMAP((__force vm_fault_t)0x01)
-#define VM_FAULT_BADACCESS ((__force vm_fault_t)0x02)
-
-static vm_fault_t __do_page_fault(struct mm_struct *mm,
- struct vm_area_struct *vma, unsigned long
addr,
- unsigned int mm_flags, unsigned long vm_flags,
- struct pt_regs *regs)
-{
- /*
-* Ok, we have a good vm_area for this memory access, so we can handle
-* it.
-* Check that the permissions on the VMA allow for the fault which
-* occurred.
-*/
- if (!(vma->vm_flags & vm_flags))
- return VM_FAULT_BADACCESS;
- return handle_mm_fault(vma, addr, mm_flags, regs);
-}
-
static bool is_el0_instruction_abort(unsigned long esr)
{
return ESR_ELx_EC(esr) == ESR_ELx_EC_IABT_LOW;
@@ -519,6 +500,9 @@ static bool is_write_abort(unsigned long esr)
return (esr & ESR_ELx_WNR) && !(esr & ESR_ELx_CM);
}
+#define VM_FAULT_BADMAP((__force vm_fault_t)0x01)
+#define VM_FAULT_BADACCESS ((__force vm_fault_t)0x02)
+
static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
struct pt_regs *regs)
{
@@ -617,7 +601,10 @@ static int __kprobes do_page_fault(unsigned long far,
unsigned long esr,
goto done;
}
- fault = __do_page_fault(mm, vma, addr, mm_flags, vm_flags, regs);
+ if (!(vma->vm_flags & vm_flags))
+ fault = VM_FAULT_BADACCESS;
+ else
+ fault = handle_mm_fault(vma, addr, mm_flags, regs);
/* Quick path to respond to signals */
if (fault_signal_pending(fault, regs)) {
--
2.27.0
[PATCH 3/3] powerpc/mm: Update the memory limit based on direct mapping restrictions
memory limit value specified by the user are further updated such that
the value is 16MB aligned. This is because hash translation mode use
16MB as direct mapping page size. Make sure we update the global
variable 'memory_limit' with the 16MB aligned value such that all kernel
components will see the new aligned value of the memory limit.
Signed-off-by: Aneesh Kumar K.V (IBM)
---
arch/powerpc/kernel/prom.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7451bedad1f4..b8f764453eaa 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -779,7 +779,6 @@ static inline void save_fscr_to_task(void) {}
void __init early_init_devtree(void *params)
{
- phys_addr_t limit;
DBG(" -> early_init_devtree(%px)\n", params);
@@ -850,8 +849,8 @@ void __init early_init_devtree(void *params)
memory_limit = 0;
/* Align down to 16 MB which is large page size with hash page
translation */
- limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M);
- memblock_enforce_memory_limit(limit);
+ memory_limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(),
SZ_16M);
+ memblock_enforce_memory_limit(memory_limit);
#if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES)
if (!early_radix_enabled())
--
2.44.0
[PATCH 2/3] powerpc/fadump: Don't update the user-specified memory limit
If the user specifies the memory limit, the kernel should honor it such
that all allocation and reservations are made within the memory limit
specified. fadump was breaking that rule. Remove the code which updates
the memory limit such that fadump reservations are done within the
limit specified.
Cc: Mahesh Salgaonkar
Signed-off-by: Aneesh Kumar K.V (IBM)
---
arch/powerpc/kernel/fadump.c | 16
1 file changed, 16 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index d14eda1e8589..4e768d93c6d4 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -573,22 +573,6 @@ int __init fadump_reserve_mem(void)
}
}
- /*
-* Calculate the memory boundary.
-* If memory_limit is less than actual memory boundary then reserve
-* the memory for fadump beyond the memory_limit and adjust the
-* memory_limit accordingly, so that the running kernel can run with
-* specified memory_limit.
-*/
- if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
- size = get_fadump_area_size();
- if ((memory_limit + size) < memblock_end_of_DRAM())
- memory_limit += size;
- else
- memory_limit = memblock_end_of_DRAM();
- printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
- " dump, now %#016llx\n", memory_limit);
- }
if (memory_limit)
mem_boundary = memory_limit;
else
--
2.44.0
[PATCH 1/3] powerpc/mm: Align memory_limit value specified using mem= kernel parameter
The value specified for the memory limit is used to set a restriction on memory usage. It is important to ensure that this restriction is within the linear map kernel address space range. The hash page table translation uses a 16MB page size to map the kernel linear map address space. htab_bolt_mapping() function aligns down the size of the range while mapping kernel linear address space. Since the memblock limit is enforced very early during boot, before we can detect the type of memory translation (radix vs hash), we align the memory limit value specified as a kernel parameter to 16MB. This alignment value will work for both hash and radix translations. Signed-off-by: Aneesh Kumar K.V (IBM) --- arch/powerpc/kernel/prom.c | 7 +-- arch/powerpc/kernel/prom_init.c | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index cd8d8883de90..7451bedad1f4 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -846,8 +846,11 @@ void __init early_init_devtree(void *params) reserve_crashkernel(); early_reserve_mem(); - /* Ensure that total memory size is page-aligned. */ - limit = ALIGN(memory_limit ?: memblock_phys_mem_size(), PAGE_SIZE); + if (memory_limit > memblock_phys_mem_size()) + memory_limit = 0; + + /* Align down to 16 MB which is large page size with hash page translation */ + limit = ALIGN_DOWN(memory_limit ?: memblock_phys_mem_size(), SZ_16M); memblock_enforce_memory_limit(limit); #if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_4K_PAGES) diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index 0ef358285337..fbb68fc28ed3 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -817,8 +817,8 @@ static void __init early_cmdline_parse(void) opt += 4; prom_memory_limit = prom_memparse(opt, (const char **)); #ifdef CONFIG_PPC64 - /* Align to 16 MB == size of ppc64 large page */ - prom_memory_limit = ALIGN(prom_memory_limit, 0x100); + /* Align down to 16 MB which is large page size with hash page translation */ + prom_memory_limit = ALIGN_DOWN(prom_memory_limit, SZ_16M); #endif } base-commit: 3e92c1e6cd876754b64d1998ec0a01800ed954a6 -- 2.44.0
Re: [PATCH 01/34] powerpc/fsl-soc: hide unused const variable
Le 03/04/2024 à 10:06, Arnd Bergmann a écrit :
> From: Arnd Bergmann
>
> vmpic_msi_feature is only used conditionally, which triggers a rare
> -Werror=unused-const-variable= warning with gcc:
>
> arch/powerpc/sysdev/fsl_msi.c:567:37: error: 'vmpic_msi_feature' defined but
> not used [-Werror=unused-const-variable=]
>567 | static const struct fsl_msi_feature vmpic_msi_feature =
>
> Hide this one in the same #ifdef as the reference so we can turn on
> the warning by default.
>
> Fixes: 305bcf26128e ("powerpc/fsl-soc: use CONFIG_EPAPR_PARAVIRT for hcalls")
> Signed-off-by: Arnd Bergmann
Reviewed-by: Christophe Leroy
> ---
> arch/powerpc/sysdev/fsl_msi.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> index 8e6c84df4ca1..e205135ae1fe 100644
> --- a/arch/powerpc/sysdev/fsl_msi.c
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -564,10 +564,12 @@ static const struct fsl_msi_feature ipic_msi_feature = {
> .msiir_offset = 0x38,
> };
>
> +#ifdef CONFIG_EPAPR_PARAVIRT
> static const struct fsl_msi_feature vmpic_msi_feature = {
> .fsl_pic_ip = FSL_PIC_IP_VMPIC,
> .msiir_offset = 0,
> };
> +#endif
>
> static const struct of_device_id fsl_of_msi_ids[] = {
> {
[PATCH 01/34] powerpc/fsl-soc: hide unused const variable
From: Arnd Bergmann
vmpic_msi_feature is only used conditionally, which triggers a rare
-Werror=unused-const-variable= warning with gcc:
arch/powerpc/sysdev/fsl_msi.c:567:37: error: 'vmpic_msi_feature' defined but
not used [-Werror=unused-const-variable=]
567 | static const struct fsl_msi_feature vmpic_msi_feature =
Hide this one in the same #ifdef as the reference so we can turn on
the warning by default.
Fixes: 305bcf26128e ("powerpc/fsl-soc: use CONFIG_EPAPR_PARAVIRT for hcalls")
Signed-off-by: Arnd Bergmann
---
arch/powerpc/sysdev/fsl_msi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 8e6c84df4ca1..e205135ae1fe 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -564,10 +564,12 @@ static const struct fsl_msi_feature ipic_msi_feature = {
.msiir_offset = 0x38,
};
+#ifdef CONFIG_EPAPR_PARAVIRT
static const struct fsl_msi_feature vmpic_msi_feature = {
.fsl_pic_ip = FSL_PIC_IP_VMPIC,
.msiir_offset = 0,
};
+#endif
static const struct of_device_id fsl_of_msi_ids[] = {
{
--
2.39.2
Re: [PATCH 7/7] x86: mm: accelerate pagefault when badaccess
On 2024/4/3 13:59, Suren Baghdasaryan wrote:
On Tue, Apr 2, 2024 at 12:53 AM Kefeng Wang wrote:
The vm_flags of vma already checked under per-VMA lock, if it is a
bad access, directly handle error and return, there is no need to
lock_mm_and_find_vma() and check vm_flags again.
Signed-off-by: Kefeng Wang
Looks safe to me.
Using (mm != NULL) to indicate that we are holding mmap_lock is not
ideal but I guess that works.
Yes, I will add this part it into change too,
The access_error() of vma already checked under per-VMA lock, if it
is a bad access, directly handle error, no need to retry with mmap_lock
again. In order to release the correct lock, pass the mm_struct into
bad_area_access_error(), if mm is NULL, release vma lock, or release
mmap_lock. Since the page faut is handled under per-VMA lock, count it
as a vma lock event with VMA_LOCK_SUCCESS.
Thanks.
Reviewed-by: Suren Baghdasaryan
---
arch/x86/mm/fault.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a4cc20d0036d..67b18adc75dd 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -866,14 +866,17 @@ bad_area_nosemaphore(struct pt_regs *regs, unsigned long
error_code,
static void
__bad_area(struct pt_regs *regs, unsigned long error_code,
- unsigned long address, u32 pkey, int si_code)
+ unsigned long address, struct mm_struct *mm,
+ struct vm_area_struct *vma, u32 pkey, int si_code)
{
- struct mm_struct *mm = current->mm;
/*
* Something tried to access memory that isn't in our memory map..
* Fix it, but check if it's kernel or user first..
*/
- mmap_read_unlock(mm);
+ if (mm)
+ mmap_read_unlock(mm);
+ else
+ vma_end_read(vma);
__bad_area_nosemaphore(regs, error_code, address, pkey, si_code);
}
@@ -897,7 +900,8 @@ static inline bool bad_area_access_from_pkeys(unsigned long
error_code,
static noinline void
bad_area_access_error(struct pt_regs *regs, unsigned long error_code,
- unsigned long address, struct vm_area_struct *vma)
+ unsigned long address, struct mm_struct *mm,
+ struct vm_area_struct *vma)
{
/*
* This OSPKE check is not strictly necessary at runtime.
@@ -927,9 +931,9 @@ bad_area_access_error(struct pt_regs *regs, unsigned long
error_code,
*/
u32 pkey = vma_pkey(vma);
- __bad_area(regs, error_code, address, pkey, SEGV_PKUERR);
+ __bad_area(regs, error_code, address, mm, vma, pkey,
SEGV_PKUERR);
} else {
- __bad_area(regs, error_code, address, 0, SEGV_ACCERR);
+ __bad_area(regs, error_code, address, mm, vma, 0, SEGV_ACCERR);
}
}
@@ -1357,8 +1361,9 @@ void do_user_addr_fault(struct pt_regs *regs,
goto lock_mmap;
if (unlikely(access_error(error_code, vma))) {
- vma_end_read(vma);
- goto lock_mmap;
+ bad_area_access_error(regs, error_code, address, NULL, vma);
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
+ return;
}
fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK,
regs);
if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
@@ -1394,7 +1399,7 @@ void do_user_addr_fault(struct pt_regs *regs,
* we can handle it..
*/
if (unlikely(access_error(error_code, vma))) {
- bad_area_access_error(regs, error_code, address, vma);
+ bad_area_access_error(regs, error_code, address, mm, vma);
return;
}
--
2.27.0
[PATCH v8 6/6] docs: trusted-encrypted: add DCP as new trust source
Update the documentation for trusted and encrypted KEYS with DCP as new trust source: - Describe security properties of DCP trust source - Describe key usage - Document blob format Co-developed-by: Richard Weinberger Signed-off-by: Richard Weinberger Co-developed-by: David Oberhollenzer Signed-off-by: David Oberhollenzer Signed-off-by: David Gstir --- .../security/keys/trusted-encrypted.rst | 53 +++ security/keys/trusted-keys/trusted_dcp.c | 19 +++ 2 files changed, 72 insertions(+) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index e989b9802f92..f4d7e162d5e4 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -42,6 +42,14 @@ safe. randomly generated and fused into each SoC at manufacturing time. Otherwise, a common fixed test key is used instead. + (4) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs) + + Rooted to a one-time programmable key (OTP) that is generally burnt + in the on-chip fuses and is accessible to the DCP encryption engine only. + DCP provides two keys that can be used as root of trust: the OTP key + and the UNIQUE key. Default is to use the UNIQUE key, but selecting + the OTP key can be done via a module parameter (dcp_use_otp_key). + * Execution isolation (1) TPM @@ -57,6 +65,12 @@ safe. Fixed set of operations running in isolated execution environment. + (4) DCP + + Fixed set of cryptographic operations running in isolated execution + environment. Only basic blob key encryption is executed there. + The actual key sealing/unsealing is done on main processor/kernel space. + * Optional binding to platform integrity state (1) TPM @@ -79,6 +93,11 @@ safe. Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs for platform integrity. + (4) DCP + + Relies on Secure/Trusted boot process (called HAB by vendor) for + platform integrity. + * Interfaces and APIs (1) TPM @@ -94,6 +113,11 @@ safe. Interface is specific to silicon vendor. + (4) DCP + + Vendor-specific API that is implemented as part of the DCP crypto driver in + ``drivers/crypto/mxs-dcp.c``. + * Threat model The strength and appropriateness of a particular trust source for a given @@ -129,6 +153,13 @@ selected trust source: CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and ensure the device is probed. + * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs) + + The DCP hardware device itself does not provide a dedicated RNG interface, + so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do have + a dedicated hardware RNG that is independent from DCP which can be enabled + to back the kernel RNG. + Users may override this by specifying ``trusted.rng=kernel`` on the kernel command-line to override the used RNG with the kernel's random number pool. @@ -231,6 +262,19 @@ Usage:: CAAM-specific format. The key length for new keys is always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits). +Trusted Keys usage: DCP +--- + +Usage:: + +keyctl add trusted name "new keylen" ring +keyctl add trusted name "load hex_blob" ring +keyctl print keyid + +"keyctl print" returns an ASCII hex copy of the sealed key, which is in format +specific to this DCP key-blob implementation. The key length for new keys is +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits). + Encrypted Keys usage @@ -426,3 +470,12 @@ string length. privkey is the binary representation of TPM2B_PUBLIC excluding the initial TPM2B header which can be reconstructed from the ASN.1 octed string length. + +DCP Blob Format +--- + +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c + :doc: dcp blob format + +.. kernel-doc:: security/keys/trusted-keys/trusted_dcp.c + :identifiers: struct dcp_blob_fmt diff --git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c index 16c44aafeab3..b5f81a05be36 100644 --- a/security/keys/trusted-keys/trusted_dcp.c +++ b/security/keys/trusted-keys/trusted_dcp.c @@ -19,6 +19,25 @@ #define DCP_BLOB_VERSION 1 #define DCP_BLOB_AUTHLEN 16 +/** + * DOC: dcp blob format + * + * The Data Co-Processor (DCP) provides hardware-bound AES keys using its + * AES encryption engine only. It does not provide direct key sealing/unsealing. + * To make DCP hardware encryption keys usable as trust source, we define + * our own custom format that uses a hardware-bound key to secure the sealing + * key stored in the key blob. + * + * Whenever a new trusted key using DCP is generated, we generate a random 128-bit + * blob
[PATCH v8 5/6] docs: document DCP-backed trusted keys kernel params
Document the kernel parameters trusted.dcp_use_otp_key and trusted.dcp_skip_zk_test for DCP-backed trusted keys. Co-developed-by: Richard Weinberger Signed-off-by: Richard Weinberger Co-developed-by: David Oberhollenzer Signed-off-by: David Oberhollenzer Signed-off-by: David Gstir Reviewed-by: Jarkko Sakkinen --- Documentation/admin-guide/kernel-parameters.txt | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 24c02c704049..3a59abf06039 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -6698,6 +6698,7 @@ - "tpm" - "tee" - "caam" + - "dcp" If not specified then it defaults to iterating through the trust source list starting with TPM and assigns the first trust source as a backend which is initialized @@ -6713,6 +6714,18 @@ If not specified, "default" is used. In this case, the RNG's choice is left to each individual trust source. + trusted.dcp_use_otp_key + This is intended to be used in combination with + trusted.source=dcp and will select the DCP OTP key + instead of the DCP UNIQUE key blob encryption. + + trusted.dcp_skip_zk_test + This is intended to be used in combination with + trusted.source=dcp and will disable the check if the + blob key is all zeros. This is helpful for situations where + having this key zero'ed is acceptable. E.g. in testing + scenarios. + tsc=Disable clocksource stability checks for TSC. Format: [x86] reliable: mark tsc clocksource as reliable, this -- 2.35.3
[PATCH v8 4/6] MAINTAINERS: add entry for DCP-based trusted keys
This covers trusted keys backed by NXP's DCP (Data Co-Processor) chip found in smaller i.MX SoCs. Signed-off-by: David Gstir Acked-by: Jarkko Sakkinen --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 976a5cea1577..ca7f42ca9338 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12019,6 +12019,15 @@ S: Maintained F: include/keys/trusted_caam.h F: security/keys/trusted-keys/trusted_caam.c +KEYS-TRUSTED-DCP +M: David Gstir +R: sigma star Kernel Team +L: linux-integr...@vger.kernel.org +L: keyri...@vger.kernel.org +S: Supported +F: include/keys/trusted_dcp.h +F: security/keys/trusted-keys/trusted_dcp.c + KEYS-TRUSTED-TEE M: Sumit Garg L: linux-integr...@vger.kernel.org -- 2.35.3
[PATCH v8 3/6] KEYS: trusted: Introduce NXP DCP-backed trusted keys
DCP (Data Co-Processor) is the little brother of NXP's CAAM IP.
Beside of accelerated crypto operations, it also offers support for
hardware-bound keys. Using this feature it is possible to implement a blob
mechanism similar to what CAAM offers. Unlike on CAAM, constructing and
parsing the blob has to happen in software (i.e. the kernel).
The software-based blob format used by DCP trusted keys encrypts
the payload using AES-128-GCM with a freshly generated random key and nonce.
The random key itself is AES-128-ECB encrypted using the DCP unique
or OTP key.
The DCP trusted key blob format is:
/*
* struct dcp_blob_fmt - DCP BLOB format.
*
* @fmt_version: Format version, currently being %1
* @blob_key: Random AES 128 key which is used to encrypt @payload,
*@blob_key itself is encrypted with OTP or UNIQUE device key in
*AES-128-ECB mode by DCP.
* @nonce: Random nonce used for @payload encryption.
* @payload_len: Length of the plain text @payload.
* @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
* GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
*
* The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
* AES_BLOCK_SIZE.
*/
struct dcp_blob_fmt {
__u8 fmt_version;
__u8 blob_key[AES_KEYSIZE_128];
__u8 nonce[AES_KEYSIZE_128];
__le32 payload_len;
__u8 payload[];
} __packed;
By default the unique key is used. It is also possible to use the
OTP key. While the unique key should be unique it is not documented how
this key is derived. Therefore selection the OTP key is supported as
well via the use_otp_key module parameter.
Co-developed-by: Richard Weinberger
Signed-off-by: Richard Weinberger
Co-developed-by: David Oberhollenzer
Signed-off-by: David Oberhollenzer
Signed-off-by: David Gstir
Reviewed-by: Jarkko Sakkinen
---
include/keys/trusted_dcp.h| 11 +
security/keys/trusted-keys/Kconfig| 8 +
security/keys/trusted-keys/Makefile | 2 +
security/keys/trusted-keys/trusted_core.c | 6 +-
security/keys/trusted-keys/trusted_dcp.c | 313 ++
5 files changed, 339 insertions(+), 1 deletion(-)
create mode 100644 include/keys/trusted_dcp.h
create mode 100644 security/keys/trusted-keys/trusted_dcp.c
diff --git a/include/keys/trusted_dcp.h b/include/keys/trusted_dcp.h
new file mode 100644
index ..9aaa42075b40
--- /dev/null
+++ b/include/keys/trusted_dcp.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ */
+
+#ifndef TRUSTED_DCP_H
+#define TRUSTED_DCP_H
+
+extern struct trusted_key_ops dcp_trusted_key_ops;
+
+#endif
diff --git a/security/keys/trusted-keys/Kconfig
b/security/keys/trusted-keys/Kconfig
index 553dc117f385..1fb8aa001995 100644
--- a/security/keys/trusted-keys/Kconfig
+++ b/security/keys/trusted-keys/Kconfig
@@ -39,6 +39,14 @@ config TRUSTED_KEYS_CAAM
Enable use of NXP's Cryptographic Accelerator and Assurance Module
(CAAM) as trusted key backend.
+config TRUSTED_KEYS_DCP
+ bool "DCP-based trusted keys"
+ depends on CRYPTO_DEV_MXS_DCP >= TRUSTED_KEYS
+ default y
+ select HAVE_TRUSTED_KEYS
+ help
+ Enable use of NXP's DCP (Data Co-Processor) as trusted key backend.
+
if !HAVE_TRUSTED_KEYS
comment "No trust source selected!"
endif
diff --git a/security/keys/trusted-keys/Makefile
b/security/keys/trusted-keys/Makefile
index 735aa0bc08ef..f0f3b27f688b 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -14,3 +14,5 @@ trusted-$(CONFIG_TRUSTED_KEYS_TPM) += tpm2key.asn1.o
trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
trusted-$(CONFIG_TRUSTED_KEYS_CAAM) += trusted_caam.o
+
+trusted-$(CONFIG_TRUSTED_KEYS_DCP) += trusted_dcp.o
diff --git a/security/keys/trusted-keys/trusted_core.c
b/security/keys/trusted-keys/trusted_core.c
index fee1ab2c734d..5113aeae5628 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -30,7 +31,7 @@ MODULE_PARM_DESC(rng, "Select trusted key RNG");
static char *trusted_key_source;
module_param_named(source, trusted_key_source, charp, 0);
-MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");
+MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee, caam or dcp)");
static const struct trusted_key_source trusted_key_sources[] = {
#if defined(CONFIG_TRUSTED_KEYS_TPM)
@@ -42,6 +43,9 @@ static const struct trusted_key_source trusted_key_sources[]
= {
#if defined(CONFIG_TRUSTED_KEYS_CAAM)
{ "caam", _key_caam_ops },
#endif
+#if defined(CONFIG_TRUSTED_KEYS_DCP)
+ { "dcp", _trusted_key_ops },
+#endif
};
DEFINE_STATIC_CALL_NULL(trusted_key_seal, *trusted_key_sources[0].ops->seal);
diff --git
[PATCH v8 2/6] KEYS: trusted: improve scalability of trust source config
Enabling trusted keys requires at least one trust source implementation (currently TPM, TEE or CAAM) to be enabled. Currently, this is done by checking each trust source's config option individually. This does not scale when more trust sources like the one for DCP are added, because the condition will get long and hard to read. Add config HAVE_TRUSTED_KEYS which is set to true by each trust source once its enabled and adapt the check for having at least one active trust source to use this option. Whenever a new trust source is added, it now needs to select HAVE_TRUSTED_KEYS. Signed-off-by: David Gstir Tested-by: Jarkko Sakkinen # for TRUSTED_KEYS_TPM Reviewed-by: Jarkko Sakkinen --- security/keys/trusted-keys/Kconfig | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig index dbfdd8536468..553dc117f385 100644 --- a/security/keys/trusted-keys/Kconfig +++ b/security/keys/trusted-keys/Kconfig @@ -1,3 +1,6 @@ +config HAVE_TRUSTED_KEYS + bool + config TRUSTED_KEYS_TPM bool "TPM-based trusted keys" depends on TCG_TPM >= TRUSTED_KEYS @@ -9,6 +12,7 @@ config TRUSTED_KEYS_TPM select ASN1_ENCODER select OID_REGISTRY select ASN1 + select HAVE_TRUSTED_KEYS help Enable use of the Trusted Platform Module (TPM) as trusted key backend. Trusted keys are random number symmetric keys, @@ -20,6 +24,7 @@ config TRUSTED_KEYS_TEE bool "TEE-based trusted keys" depends on TEE >= TRUSTED_KEYS default y + select HAVE_TRUSTED_KEYS help Enable use of the Trusted Execution Environment (TEE) as trusted key backend. @@ -29,10 +34,11 @@ config TRUSTED_KEYS_CAAM depends on CRYPTO_DEV_FSL_CAAM_JR >= TRUSTED_KEYS select CRYPTO_DEV_FSL_CAAM_BLOB_GEN default y + select HAVE_TRUSTED_KEYS help Enable use of NXP's Cryptographic Accelerator and Assurance Module (CAAM) as trusted key backend. -if !TRUSTED_KEYS_TPM && !TRUSTED_KEYS_TEE && !TRUSTED_KEYS_CAAM -comment "No trust source selected!" +if !HAVE_TRUSTED_KEYS + comment "No trust source selected!" endif -- 2.35.3
[PATCH v8 1/6] crypto: mxs-dcp: Add support for hardware-bound keys
DCP (Data Co-Processor) is able to derive private keys for a fused
random seed, which can be referenced by handle but not accessed by
the CPU. Similarly, DCP is able to store arbitrary keys in four
dedicated key slots located in its secure memory area (internal SRAM).
These keys can be used to perform AES encryption.
Expose these derived keys and key slots through the crypto API via their
handle. The main purpose is to add DCP-backed trusted keys. Other
use cases are possible too (see similar existing paes implementations),
but these should carefully be evaluated as e.g. enabling AF_ALG will
give userspace full access to use keys. In scenarios with untrustworthy
userspace, this will enable en-/decryption oracles.
Co-developed-by: Richard Weinberger
Signed-off-by: Richard Weinberger
Co-developed-by: David Oberhollenzer
Signed-off-by: David Oberhollenzer
Signed-off-by: David Gstir
Acked-by: Herbert Xu
Reviewed-by: Jarkko Sakkinen
---
drivers/crypto/mxs-dcp.c | 104 ++-
include/soc/fsl/dcp.h| 20
2 files changed, 113 insertions(+), 11 deletions(-)
create mode 100644 include/soc/fsl/dcp.h
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 2b3ebe0db3a6..057d73c370b7 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
@@ -101,6 +102,7 @@ struct dcp_async_ctx {
struct crypto_skcipher *fallback;
unsigned intkey_len;
uint8_t key[AES_KEYSIZE_128];
+ boolkey_referenced;
};
struct dcp_aes_req_ctx {
@@ -155,6 +157,7 @@ static struct dcp *global_sdcp;
#define MXS_DCP_CONTROL0_HASH_TERM (1 << 13)
#define MXS_DCP_CONTROL0_HASH_INIT (1 << 12)
#define MXS_DCP_CONTROL0_PAYLOAD_KEY (1 << 11)
+#define MXS_DCP_CONTROL0_OTP_KEY (1 << 10)
#define MXS_DCP_CONTROL0_CIPHER_ENCRYPT(1 << 8)
#define MXS_DCP_CONTROL0_CIPHER_INIT (1 << 9)
#define MXS_DCP_CONTROL0_ENABLE_HASH (1 << 6)
@@ -168,6 +171,8 @@ static struct dcp *global_sdcp;
#define MXS_DCP_CONTROL1_CIPHER_MODE_ECB (0 << 4)
#define MXS_DCP_CONTROL1_CIPHER_SELECT_AES128 (0 << 0)
+#define MXS_DCP_CONTROL1_KEY_SELECT_SHIFT 8
+
static int mxs_dcp_start_dma(struct dcp_async_ctx *actx)
{
int dma_err;
@@ -224,13 +229,16 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
struct dcp *sdcp = global_sdcp;
struct dcp_dma_desc *desc = >coh->desc[actx->chan];
struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
+ bool key_referenced = actx->key_referenced;
int ret;
- key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
- 2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
- ret = dma_mapping_error(sdcp->dev, key_phys);
- if (ret)
- return ret;
+ if (!key_referenced) {
+ key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
+ 2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
+ ret = dma_mapping_error(sdcp->dev, key_phys);
+ if (ret)
+ return ret;
+ }
src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf,
DCP_BUF_SZ, DMA_TO_DEVICE);
@@ -255,8 +263,12 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
MXS_DCP_CONTROL0_INTERRUPT |
MXS_DCP_CONTROL0_ENABLE_CIPHER;
- /* Payload contains the key. */
- desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
+ if (key_referenced)
+ /* Set OTP key bit to select the key via KEY_SELECT. */
+ desc->control0 |= MXS_DCP_CONTROL0_OTP_KEY;
+ else
+ /* Payload contains the key. */
+ desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
if (rctx->enc)
desc->control0 |= MXS_DCP_CONTROL0_CIPHER_ENCRYPT;
@@ -270,6 +282,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
else
desc->control1 |= MXS_DCP_CONTROL1_CIPHER_MODE_CBC;
+ if (key_referenced)
+ desc->control1 |= sdcp->coh->aes_key[0] <<
MXS_DCP_CONTROL1_KEY_SELECT_SHIFT;
+
desc->next_cmd_addr = 0;
desc->source = src_phys;
desc->destination = dst_phys;
@@ -284,9 +299,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
err_dst:
dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE);
err_src:
- dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
-DMA_TO_DEVICE);
-
+ if (!key_referenced)
+ dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
+DMA_TO_DEVICE);
return ret;
}
@@ -453,7 +468,7 @@ static int
[PATCH v8 0/6] DCP as trusted keys backend
This is a revival of the previous patch set submitted by Richard Weinberger:
https://lore.kernel.org/linux-integrity/20210614201620.30451-1-rich...@nod.at/
After having been thoroughly reviewed by Jarkko, it would be great if this
could go into 6.10. :-)
v7 is here:
https://lore.kernel.org/keyrings/20240327082454.13729-1-da...@sigma-star.at/
v7 -> v8:
- Add Reviewed-by from Jarkko Sakkinen for patches #2 and #5
- Use kernel-doc for DCP blob format documentation instead of copy-pasting as
suggested by Jarkko Sakkinen
- Fix wording in docs for trusted.dcp_skip_zk_test kernel param
v6 -> v7:
- Add Reviewed-by from Jarkko Sakkinen for patches #1 and #3
- Improved commit messages
- Changed log level for non-trusted/secure mode check from error to warning
v5 -> v6:
- Cleaned up coding style and commit messages to make the whole series more
coherent as suggested by Jarkko Sakkinen
- Added Acked-By from Jarkko Sakkinen to patch #4 - thanks!
- Rebased against next-20240307
v4 -> v5:
- Make Kconfig for trust source check scalable as suggested by Jarkko Sakkinen
- Add Acked-By from Herbert Xu to patch #1 - thanks!
v3 -> v4:
- Split changes on MAINTAINERS and documentation into dedicated patches
- Use more concise wording in commit messages as suggested by Jarkko Sakkinen
v2 -> v3:
- Addressed review comments from Jarkko Sakkinen
v1 -> v2:
- Revive and rebase to latest version
- Include review comments from Ahmad Fatoum
The Data Co-Processor (DCP) is an IP core built into many NXP SoCs such
as i.mx6ull.
Similar to the CAAM engine used in more powerful SoCs, DCP can AES-
encrypt/decrypt user data using a unique, never-disclosed,
device-specific key. Unlike CAAM though, it cannot directly wrap and
unwrap blobs in hardware. As DCP offers only the bare minimum feature
set and a blob mechanism needs aid from software. A blob in this case
is a piece of sensitive data (e.g. a key) that is encrypted and
authenticated using the device-specific key so that unwrapping can only
be done on the hardware where the blob was wrapped.
This patch series adds a DCP based, trusted-key backend and is similar
in spirit to the one by Ahmad Fatoum [0] that does the same for CAAM.
It is of interest for similar use cases as the CAAM patch set, but for
lower end devices, where CAAM is not available.
Because constructing and parsing the blob has to happen in software,
we needed to decide on a blob format and chose the following:
struct dcp_blob_fmt {
__u8 fmt_version;
__u8 blob_key[AES_KEYSIZE_128];
__u8 nonce[AES_KEYSIZE_128];
__le32 payload_len;
__u8 payload[];
} __packed;
The `fmt_version` is currently 1.
The encrypted key is stored in the payload area. It is AES-128-GCM
encrypted using `blob_key` and `nonce`, GCM auth tag is attached at
the end of the payload (`payload_len` does not include the size of
the auth tag).
The `blob_key` itself is encrypted in AES-128-ECB mode by DCP using
the OTP or UNIQUE device key. A new `blob_key` and `nonce` are generated
randomly, when sealing/exporting the DCP blob.
This patchset was tested with dm-crypt on an i.MX6ULL board.
[0]
https://lore.kernel.org/keyrings/20220513145705.2080323-1-a.fat...@pengutronix.de/
David Gstir (6):
crypto: mxs-dcp: Add support for hardware-bound keys
KEYS: trusted: improve scalability of trust source config
KEYS: trusted: Introduce NXP DCP-backed trusted keys
MAINTAINERS: add entry for DCP-based trusted keys
docs: document DCP-backed trusted keys kernel params
docs: trusted-encrypted: add DCP as new trust source
.../admin-guide/kernel-parameters.txt | 13 +
.../security/keys/trusted-encrypted.rst | 53 +++
MAINTAINERS | 9 +
drivers/crypto/mxs-dcp.c | 104 +-
include/keys/trusted_dcp.h| 11 +
include/soc/fsl/dcp.h | 20 ++
security/keys/trusted-keys/Kconfig| 18 +-
security/keys/trusted-keys/Makefile | 2 +
security/keys/trusted-keys/trusted_core.c | 6 +-
security/keys/trusted-keys/trusted_dcp.c | 332 ++
10 files changed, 554 insertions(+), 14 deletions(-)
create mode 100644 include/keys/trusted_dcp.h
create mode 100644 include/soc/fsl/dcp.h
create mode 100644 security/keys/trusted-keys/trusted_dcp.c
--
2.35.3
Re: [PATCH 2/7] arm64: mm: accelerate pagefault when VM_FAULT_BADACCESS
On 2024/4/3 13:30, Suren Baghdasaryan wrote:
On Tue, Apr 2, 2024 at 10:19 PM Suren Baghdasaryan wrote:
On Tue, Apr 2, 2024 at 12:53 AM Kefeng Wang wrote:
The vm_flags of vma already checked under per-VMA lock, if it is a
bad access, directly set fault to VM_FAULT_BADACCESS and handle error,
no need to lock_mm_and_find_vma() and check vm_flags again, the latency
time reduce 34% in lmbench 'lat_sig -P 1 prot lat_sig'.
The change makes sense to me. Per-VMA lock is enough to keep
vma->vm_flags stable, so no need to retry with mmap_lock.
Signed-off-by: Kefeng Wang
Reviewed-by: Suren Baghdasaryan
---
arch/arm64/mm/fault.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 9bb9f395351a..405f9aa831bd 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -572,7 +572,9 @@ static int __kprobes do_page_fault(unsigned long far,
unsigned long esr,
if (!(vma->vm_flags & vm_flags)) {
vma_end_read(vma);
- goto lock_mmap;
+ fault = VM_FAULT_BADACCESS;
+ count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
nit: VMA_LOCK_SUCCESS accounting here seems correct to me but
unrelated to the main change. Either splitting into a separate patch
or mentioning this additional fixup in the changelog would be helpful.
The above nit applies to all the patches after this one, so I won't
comment on each one separately. If you decide to split or adjust the
changelog please do that for each patch.
I will update the change log for each patch, thank for your review and
suggestion.
+ goto done;
}
fault = handle_mm_fault(vma, addr, mm_flags | FAULT_FLAG_VMA_LOCK,
regs);
if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
--
2.27.0
Re: [PATCH 7/7] x86: mm: accelerate pagefault when badaccess
On Tue, Apr 2, 2024 at 12:53 AM Kefeng Wang wrote:
>
> The vm_flags of vma already checked under per-VMA lock, if it is a
> bad access, directly handle error and return, there is no need to
> lock_mm_and_find_vma() and check vm_flags again.
>
> Signed-off-by: Kefeng Wang
Looks safe to me.
Using (mm != NULL) to indicate that we are holding mmap_lock is not
ideal but I guess that works.
Reviewed-by: Suren Baghdasaryan
> ---
> arch/x86/mm/fault.c | 23 ++-
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index a4cc20d0036d..67b18adc75dd 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -866,14 +866,17 @@ bad_area_nosemaphore(struct pt_regs *regs, unsigned
> long error_code,
>
> static void
> __bad_area(struct pt_regs *regs, unsigned long error_code,
> - unsigned long address, u32 pkey, int si_code)
> + unsigned long address, struct mm_struct *mm,
> + struct vm_area_struct *vma, u32 pkey, int si_code)
> {
> - struct mm_struct *mm = current->mm;
> /*
> * Something tried to access memory that isn't in our memory map..
> * Fix it, but check if it's kernel or user first..
> */
> - mmap_read_unlock(mm);
> + if (mm)
> + mmap_read_unlock(mm);
> + else
> + vma_end_read(vma);
>
> __bad_area_nosemaphore(regs, error_code, address, pkey, si_code);
> }
> @@ -897,7 +900,8 @@ static inline bool bad_area_access_from_pkeys(unsigned
> long error_code,
>
> static noinline void
> bad_area_access_error(struct pt_regs *regs, unsigned long error_code,
> - unsigned long address, struct vm_area_struct *vma)
> + unsigned long address, struct mm_struct *mm,
> + struct vm_area_struct *vma)
> {
> /*
> * This OSPKE check is not strictly necessary at runtime.
> @@ -927,9 +931,9 @@ bad_area_access_error(struct pt_regs *regs, unsigned long
> error_code,
> */
> u32 pkey = vma_pkey(vma);
>
> - __bad_area(regs, error_code, address, pkey, SEGV_PKUERR);
> + __bad_area(regs, error_code, address, mm, vma, pkey,
> SEGV_PKUERR);
> } else {
> - __bad_area(regs, error_code, address, 0, SEGV_ACCERR);
> + __bad_area(regs, error_code, address, mm, vma, 0,
> SEGV_ACCERR);
> }
> }
>
> @@ -1357,8 +1361,9 @@ void do_user_addr_fault(struct pt_regs *regs,
> goto lock_mmap;
>
> if (unlikely(access_error(error_code, vma))) {
> - vma_end_read(vma);
> - goto lock_mmap;
> + bad_area_access_error(regs, error_code, address, NULL, vma);
> + count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
> + return;
> }
> fault = handle_mm_fault(vma, address, flags | FAULT_FLAG_VMA_LOCK,
> regs);
> if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)))
> @@ -1394,7 +1399,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> * we can handle it..
> */
> if (unlikely(access_error(error_code, vma))) {
> - bad_area_access_error(regs, error_code, address, vma);
> + bad_area_access_error(regs, error_code, address, mm, vma);
> return;
> }
>
> --
> 2.27.0
>
