[PATCH v2 2/2] book3s64/radix : Align section vmemmap start address to PAGE_SIZE
A vmemmap altmap is a device-provided region used to provide
backing storage for struct pages. For each namespace, the altmap
should belong to that same namespace. If the namespaces are
created unaligned, there is a chance that the section vmemmap
start address could also be unaligned. If the section vmemmap
start address is unaligned, the altmap page allocated from the
current namespace might be used by the previous namespace also.
During the free operation, since the altmap is shared between two
namespaces, the previous namespace may detect that the page does
not belong to its altmap and incorrectly assume that the page is a
normal page. It then attempts to free the normal page, which leads
to a kernel crash.
Kernel attempted to read user page (18) - exploit attempt? (uid: 0)
BUG: Kernel NULL pointer dereference on read at 0x0018
Faulting instruction address: 0xc0530c7c
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
CPU: 32 PID: 2104 Comm: ndctl Kdump: loaded Tainted: GW
NIP: c0530c7c LR: c0530e00 CTR: 7ffe
REGS: c00015e57040 TRAP: 0300 Tainted: GW
MSR: 8280b033 CR: 84482404
CFAR: c0530dfc DAR: 0018 DSISR: 4000 IRQMASK: 0
GPR00: c0530e00 c00015e572e0 c2c5cb00 c00c000101008040
GPR04: 0007 0001 001f
GPR08: 0005 0018 2000
GPR12: c01d2fb0 c060de6b0080 c060dbf90020
GPR16: c00c000101008000 0001 c00125b20f00
GPR20: 0001 c00c000101007fff
GPR24: 0001
GPR28: 04040201 0001 c00c000101008040
NIP [c0530c7c] get_pfnblock_flags_mask+0x7c/0xd0
LR [c0530e00] free_unref_page_prepare+0x130/0x4f0
Call Trace:
free_unref_page+0x50/0x1e0
free_reserved_page+0x40/0x68
free_vmemmap_pages+0x98/0xe0
remove_pte_table+0x164/0x1e8
remove_pmd_table+0x204/0x2c8
remove_pud_table+0x1c4/0x288
remove_pagetable+0x1c8/0x310
vmemmap_free+0x24/0x50
section_deactivate+0x28c/0x2a0
__remove_pages+0x84/0x110
arch_remove_memory+0x38/0x60
memunmap_pages+0x18c/0x3d0
devm_action_release+0x30/0x50
release_nodes+0x68/0x140
devres_release_group+0x100/0x190
dax_pmem_compat_release+0x44/0x80 [dax_pmem_compat]
device_for_each_child+0x8c/0x100
[dax_pmem_compat_remove+0x2c/0x50 [dax_pmem_compat]
nvdimm_bus_remove+0x78/0x140 [libnvdimm]
device_remove+0x70/0xd0
Another issue is that if there is no altmap, a PMD-sized vmemmap
page will be allocated from RAM, regardless of the alignment of
the section start address. If the section start address is not
aligned to the PMD size, a VM_BUG_ON will be triggered when
setting the PMD-sized page to page table.
In this patch, we are aligning the section vmemmap start address
to PAGE_SIZE. After alignment, the start address will not be
part of the current namespace, and a normal page will be allocated
for the vmemmap mapping of the current section. For the remaining
sections, altmaps will be allocated. During the free operation,
the normal page will be correctly freed.
In the same way, a PMD_SIZE vmemmap page will be allocated only if
the section start address is PMD_SIZE-aligned; otherwise, it will
fall back to a PAGE-sized vmemmap allocation.
Without this patch
==
NS1 start NS2 start
_
| NS1 |NS2 |
-
| Altmap| Altmap | .|Altmap| Altmap | ...
| NS1 | NS1 | | NS2 | NS2 |
In the above scenario, NS1 and NS2 are two namespaces. The vmemmap
for NS1 comes from Altmap NS1, which belongs to NS1, and the
vmemmap for NS2 comes from Altmap NS2, which belongs to NS2.
The vmemmap start for NS2 is not aligned, so Altmap NS2 is shared
by both NS1 and NS2. During the free operation in NS1, Altmap NS2
is not part of NS1's altmap, causing it to attempt to free an
invalid page.
With this patch
===
NS1 start NS2 start
_
| NS1 |NS2 |
-
| Altmap| Altmap | .| Normal | Altmap | Altmap |...
| NS1 | NS1 | | Page | NS2 | NS2 |
If the vmemmap start for NS2 is not aligned then we are allocating
a normal page. NS1 and NS2 vmemmap will be freed correctly.
Fixes: 368a0590d954 ("powerpc/book3s64/vmemmap: switch radix to use a different
vmemmap handling function")
Co-developed-by: Ritesh Harjani (IBM)
Signed-off-by: Ritesh Harjani (IBM)
Signed-off-by: Donet Tom
---
v1 -> v2 : Added a fix for the VM_BUG_ON being triggered whe
[PATCH v2 1/2] book3s64/radix: Fix compile errors when CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP=n
From: "Ritesh Harjani (IBM)"
Fix compile errors when CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP=n
Signed-off-by: Ritesh Harjani (IBM)
Signed-off-by: Donet Tom
---
arch/powerpc/mm/book3s64/radix_pgtable.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 311e2112d782..bd6916419472 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -976,7 +976,7 @@ int __meminit radix__vmemmap_create_mapping(unsigned long
start,
return 0;
}
-
+#ifdef CONFIG_ARCH_WANT_OPTIMIZE_DAX_VMEMMAP
bool vmemmap_can_optimize(struct vmem_altmap *altmap, struct dev_pagemap
*pgmap)
{
if (radix_enabled())
@@ -984,6 +984,7 @@ bool vmemmap_can_optimize(struct vmem_altmap *altmap,
struct dev_pagemap *pgmap)
return false;
}
+#endif
int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
unsigned long addr, unsigned long next)
--
2.47.1
Re: [PATCH v5 3/3] printf: implicate test line in failure messages
On Fri 2025-02-21 15:34:32, Tamir Duberstein wrote: > This improves the failure output by pointing to the failing line at the > top level of the test, e.g.: > # test_number: EXPECTATION FAILED at lib/printf_kunit.c:103 > lib/printf_kunit.c:167: vsnprintf(buf, 256, "%#-12x", ...) wrote > '0x1234abcd ', expected '0x1234abce ' > # test_number: EXPECTATION FAILED at lib/printf_kunit.c:142 > lib/printf_kunit.c:167: kvasprintf(..., "%#-12x", ...) returned '0x1234abcd > ', expected '0x1234abce ' > > Signed-off-by: Tamir Duberstein Just for record. I like this improvement. But I am going to wait for the next version of the patchset which is going to add back the trailing '\n'. Best Regards, Petr
Re: crng init: was: Re: [PATCH v5 2/3] printf: break kunit into test cases
On Fri, Mar 7, 2025 at 11:18 AM Petr Mladek wrote:
>
> On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
> > Move all tests into `printf_test_cases`. This gives us nicer output in
> > the event of a failure.
> >
> > Combine `plain_format` and `plain_hash` into `hash_pointer` since
> > they're testing the same scenario.
> >
> > --- a/lib/tests/printf_kunit.c
> > +++ b/lib/tests/printf_kunit.c
> > @@ -178,7 +179,7 @@ test_number(void)
> > }
> >
> > static void
> > -test_string(void)
> > +test_string(struct kunit *kunittest)
> > {
> > test("", "%s%.0s", "", "123");
> > test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456");
> > @@ -215,29 +216,6 @@ test_string(void)
> > #define ZEROS "" /* hex 32 zero bits */
> > #define ONES "" /* hex 32 one bits */
> >
> > -static int
> > -plain_format(void)
> > -{
> > - char buf[PLAIN_BUF_SIZE];
> > - int nchars;
> > -
> > - nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
> > -
> > - if (nchars != PTR_WIDTH)
> > - return -1;
> > -
> > - if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> > - kunit_warn(kunittest, "crng possibly not yet initialized.
> > plain 'p' buffer contains \"%s\"",
> > - PTR_VAL_NO_CRNG);
> > - return 0;
> > - }
> > -
> > - if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
> > - return -1;
> > -
> > - return 0;
> > -}
> > -
> > #else
> >
> > #define PTR_WIDTH 8
> > @@ -247,89 +225,44 @@ plain_format(void)
> > #define ZEROS ""
> > #define ONES ""
> >
> > -static int
> > -plain_format(void)
> > -{
> > - /* Format is implicitly tested for 32 bit machines by plain_hash() */
> > - return 0;
> > -}
> > -
> > #endif /* BITS_PER_LONG == 64 */
> >
> > -static int
> > -plain_hash_to_buffer(const void *p, char *buf, size_t len)
> > +static void
> > +plain_hash_to_buffer(struct kunit *kunittest, const void *p, char *buf,
> > size_t len)
> > {
> > - int nchars;
> > -
> > - nchars = snprintf(buf, len, "%p", p);
> > -
> > - if (nchars != PTR_WIDTH)
> > - return -1;
> > + KUNIT_ASSERT_EQ(kunittest, snprintf(buf, len, "%p", p), PTR_WIDTH);
> >
> > if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> > kunit_warn(kunittest, "crng possibly not yet initialized.
> > plain 'p' buffer contains \"%s\"",
> > PTR_VAL_NO_CRNG);
> > - return 0;
>
> I have simulated the not-yet-initialized crng and got:
>
> [ 80.109760] printf_kunit: module verification failed: signature and/or
> required key missing - tainting kernel
> [ 80.114218] KTAP version 1
> [ 80.114743] 1..1
> [ 80.116124] KTAP version 1
> [ 80.116752] # Subtest: printf
> [ 80.117239] # module: printf_kunit
> [ 80.117256] 1..28
> [ 80.120924] ok 1 test_basic
> [ 80.121495] ok 2 test_number
> [ 80.122741] ok 3 test_string
> [ 80.123498] # hash_pointer: crng possibly not yet initialized. plain
> 'p' buffer contains "(ptrval)"
> [ 80.124044] # hash_pointer: EXPECTATION FAILED at
> lib/tests/printf_kunit.c:256
>Expected buf == "", but
>buf ==
><28><5f><5f><5f><5f><70><74><72>
>"" ==
><30><30><30><30><30><30><30><30>
> [ 80.125888] not ok 4 hash_pointer
> [ 80.129831] ok 5 null_pointer
> [ 80.130253] ok 6 error_pointer
> [ 80.131221] # invalid_pointer: crng possibly not yet initialized.
> plain 'p' buffer contains "(ptrval)"
> [ 80.132168] ok 7 invalid_pointer
> [ 80.135149] ok 8 symbol_ptr
> [ 80.136016] ok 9 kernel_ptr
> [ 80.136868] ok 10 struct_resource
> [ 80.137768] ok 11 struct_range
> [ 80.138613] ok 12 addr
> [ 80.139370] ok 13 escaped_str
> [ 80.140054] ok 14 hex_string
> [ 80.140601] ok 15 mac
> [ 80.141162] ok 16 ip4
> [ 80.141670] ok 17 ip6
> [ 80.142221] ok 18 uuid
> [ 80.143090] ok 19 dentry
> [ 80.143963] ok 20 struct_va_format
> [ 80.144523] ok 21 time_and_date
> [ 80.145043] ok 22 struct_clk
> [ 80.145589] ok 23 bitmap
> [ 80.146087] ok 24 netdev_features
> [ 80.146572] ok 25 flags
> [ 80.146980] # errptr: crng possibly not yet initialized. plain 'p'
> buffer contains "(ptrval)"
> [ 80.147412] ok 26 errptr
> [ 80.148548] ok 27 fwnode_pointer
> [ 80.149086] ok 28 fourcc_pointer
> [ 80.149090] # printf: ran 448 tests
> [ 80.149099] # printf: pass:27 fail:1 skip:0 total:28
> [ 80.149102] # Totals: pass:27 fail:1 skip:0 total:28
> [ 80.149106] not ok 1 printf
>
> => One test failed even though vspritf() worked as expected.
>
>The "EXPECTATION FAILED" message was a bit tricky because
>it printed "<28><5f><5f><5f><5f><70><74><72>" instead of
> "(___
Re: [PATCH v5 3/3] printf: implicate test line in failure messages
On Fri, Mar 7, 2025 at 11:23 AM Petr Mladek wrote: > > On Fri 2025-02-21 15:34:32, Tamir Duberstein wrote: > > This improves the failure output by pointing to the failing line at the > > top level of the test, e.g.: > > # test_number: EXPECTATION FAILED at lib/printf_kunit.c:103 > > lib/printf_kunit.c:167: vsnprintf(buf, 256, "%#-12x", ...) wrote > > '0x1234abcd ', expected '0x1234abce ' > > # test_number: EXPECTATION FAILED at lib/printf_kunit.c:142 > > lib/printf_kunit.c:167: kvasprintf(..., "%#-12x", ...) returned > > '0x1234abcd ', expected '0x1234abce ' > > > > Signed-off-by: Tamir Duberstein > > Just for record. I like this improvement. But I am going to wait for the > next version of the patchset which is going to add back the trailing '\n'. 👍
RE: [PATCH v3 net-next 01/13] net: enetc: add initial netc-lib driver to support NTMP
> > On Tue, 4 Mar 2025 15:21:49 +0800 Wei Fang wrote: > > > +config NXP_NETC_LIB > > > + tristate "NETC Library" > > > > Remove the string after "tristate", the user should not be prompted > > to make a choice for this, since the consumers "select" this config > > directly. > > > > Okay, I will remove it. > > > > + help > > > + This module provides common functionalities for both ENETC and NETC > > > + Switch, such as NETC Table Management Protocol (NTMP) 2.0, > common > > tc > > > + flower and debugfs interfaces and so on. > > > + > > > + If compiled as module (M), the module name is nxp-netc-lib. > > > > Not sure if the help makes sense for an invisible symbol either. > > Yes, I think it can also be removed. Thanks. > > > > > config FSL_ENETC > > > tristate "ENETC PF driver" > > > depends on PCI_MSI > > > @@ -40,6 +50,7 @@ config NXP_ENETC4 > > > select FSL_ENETC_CORE > > > select FSL_ENETC_MDIO > > > select NXP_ENETC_PF_COMMON > > > + select NXP_NETC_LIB > > > select PHYLINK > > > select DIMLIB > > > help > > > > > +#pragma pack(1) > > > > please don't blindly pack all structs, only if they are misaligned > > or will otherwise have holes. > > Because these structures are in hardware buffer format and need > to be aligned, so for convenience, I simply used pack(1). You are right, > I should use pack() for structures with holes. Thanks. > > > > > +#if IS_ENABLED(CONFIG_NXP_NETC_LIB) > > > > why the ifdef, all callers select the config option > > hm..., there are some interfaces of netc-lib are used in common .c files > in downstream, so I used "ifdef" in downstream. Now for the upstream, > I'm going to separate them from the common .c files. So yes, we can > remove it now. Sorry, I misread the header file. The ifdef in ntmp.h is needed because the interfaces in this header file will be used by the enetc-core and enetc-vf drivers. For the ENETC v1 (LS1028A platform), it will not select NXP_NETC_LIB.
RE: [PATCH v3 net-next 01/13] net: enetc: add initial netc-lib driver to support NTMP
> On Sat, 8 Mar 2025 02:05:35 + Wei Fang wrote: > > > > On Tue, 4 Mar 2025 15:21:49 +0800 Wei Fang wrote: > > > hm..., there are some interfaces of netc-lib are used in common .c > > > files in downstream, so I used "ifdef" in downstream. Now for the > > > upstream, I'm going to separate them from the common .c files. So > > > yes, we can remove it now. > > > > Sorry, I misread the header file. The ifdef in ntmp.h is needed > > because the interfaces in this header file will be used by the > > enetc-core and enetc-vf drivers. For the ENETC v1 (LS1028A platform), > > it will not select NXP_NETC_LIB. > > Meaning FSL_ENETC ? And the calls are in FSL_ENETC_CORE ? Yes, ENETC v1 uses FSL_ENETC, and some of the functions in ntmp.h are called in FSL_ENETC_CORE, enetc-core is common driver which provides interfaces to ENETC (v1 and v4) PF and VF drivers.
[PATCH v6 3/3] printf: implicate test line in failure messages
This improves the failure output by pointing to the failing line at the
top level of the test, e.g.:
# test_number: EXPECTATION FAILED at lib/printf_kunit.c:103
lib/printf_kunit.c:167: vsnprintf(buf, 256, "%#-12x", ...) wrote '0x1234abcd
', expected '0x1234abce '
# test_number: EXPECTATION FAILED at lib/printf_kunit.c:142
lib/printf_kunit.c:167: kvasprintf(..., "%#-12x", ...) returned '0x1234abcd
', expected '0x1234abce '
Signed-off-by: Tamir Duberstein
---
lib/tests/printf_kunit.c | 60 ++--
1 file changed, 33 insertions(+), 27 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index dd373cb9036a..2c9f6170bacd 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -38,9 +38,9 @@ static unsigned int total_tests;
static char *test_buffer;
static char *alloced_buffer;
-static void __printf(5, 0)
-do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen,
- const char *fmt, va_list ap)
+static void __printf(7, 0)
+do_test(struct kunit *kunittest, const char *file, const int line, int
bufsize, const char *expect,
+ int elen, const char *fmt, va_list ap)
{
va_list aq;
int ret, written;
@@ -53,20 +53,24 @@ do_test(struct kunit *kunittest, int bufsize, const char
*expect, int elen,
va_end(aq);
if (ret != elen) {
- KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, \"%s\", ...) returned
%d, expected %d\n",
- bufsize, fmt, ret, elen);
+ KUNIT_FAIL(kunittest,
+ "%s:%d: vsnprintf(buf, %d, \"%s\", ...) returned %d,
expected %d\n",
+ file, line, bufsize, fmt, ret, elen);
return;
}
if (memchr_inv(alloced_buffer, FILL_CHAR, PAD_SIZE)) {
- KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, \"%s\", ...) wrote
before buffer\n",
- bufsize, fmt);
+ KUNIT_FAIL(kunittest,
+ "%s:%d: vsnprintf(buf, %d, \"%s\", ...) wrote before
buffer\n",
+ file, line, bufsize, fmt);
return;
}
if (!bufsize) {
if (memchr_inv(test_buffer, FILL_CHAR, BUF_SIZE + PAD_SIZE)) {
- KUNIT_FAIL(kunittest, "vsnprintf(buf, 0, \"%s\", ...)
wrote to buffer\n", fmt);
+ KUNIT_FAIL(kunittest,
+ "%s:%d: vsnprintf(buf, 0, \"%s\", ...) wrote
to buffer\n",
+ file, line, fmt);
}
return;
}
@@ -74,34 +78,36 @@ do_test(struct kunit *kunittest, int bufsize, const char
*expect, int elen,
written = min(bufsize-1, elen);
if (test_buffer[written]) {
KUNIT_FAIL(kunittest,
- "vsnprintf(buf, %d, \"%s\", ...) did not
nul-terminate buffer\n",
- bufsize, fmt);
+ "%s:%d: vsnprintf(buf, %d, \"%s\", ...) did not
nul-terminate buffer\n",
+ file, line, bufsize, fmt);
return;
}
if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written
+ 1))) {
KUNIT_FAIL(kunittest,
- "vsnprintf(buf, %d, \"%s\", ...) wrote beyond the
nul-terminator\n",
- bufsize, fmt);
+ "%s:%d: vsnprintf(buf, %d, \"%s\", ...) wrote beyond
the nul-terminator\n",
+ file, line, bufsize, fmt);
return;
}
if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE -
bufsize)) {
- KUNIT_FAIL(kunittest, "vsnprintf(buf, %d, \"%s\", ...) wrote
beyond buffer\n",
- bufsize, fmt);
+ KUNIT_FAIL(kunittest,
+ "%s:%d: vsnprintf(buf, %d, \"%s\", ...) wrote beyond
buffer\n",
+ file, line, bufsize, fmt);
return;
}
if (memcmp(test_buffer, expect, written)) {
KUNIT_FAIL(kunittest,
- "vsnprintf(buf, %d, \"%s\", ...) wrote '%s',
expected '%.*s'\n",
- bufsize, fmt, test_buffer, written, expect);
+ "%s:%d: vsnprintf(buf, %d, \"%s\", ...) wrote '%s',
expected '%.*s'\n",
+ file, line, bufsize, fmt, test_buffer, written,
expect);
return;
}
}
-static void __printf(4, 5)
-__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt,
...)
+static void __printf(6, 7)
+__test(struct kunit *kunittest, const char *file, const int line, const char
*expect, int elen,
+ const char *fmt, ...)
{
va_list ap;
int rand;
@@ -109,8 +115,8 @@ __test(struct kunit *kunittest, const char *expect, int
elen, con
Re: [PATCH] book3s64/radix : Align section vmemmap start address to PAGE_SIZE
Donet Tom writes:
> A vmemmap altmap is a device-provided region used to provide
> backing storage for struct pages. For each namespace, the altmap
> should belong to that same namespace. If the namespaces are
> created unaligned, there is a chance that the section vmemmap
> start address could also be unaligned. If the section vmemmap
> start address is unaligned, the altmap page allocated from the
> current namespace might be used by the previous namespace also.
> During the free operation, since the altmap is shared between two
> namespaces, the previous namespace may detect that the page does
> not belong to its altmap and incorrectly assume that the page is a
> normal page. It then attempts to free the normal page, which leads
> to a kernel crash.
>
> In this patch, we are aligning the section vmemmap start address
> to PAGE_SIZE. After alignment, the start address will not be
> part of the current namespace, and a normal page will be allocated
> for the vmemmap mapping of the current section. For the remaining
> sections, altmaps will be allocated. During the free operation,
> the normal page will be correctly freed.
>
> Without this patch
> ==
> NS1 start NS2 start
> _
> | NS1 |NS2 |
> -
> | Altmap| Altmap | .|Altmap| Altmap | ...
> | NS1 | NS1 | | NS2 | NS2 |
>
> In the above scenario, NS1 and NS2 are two namespaces. The vmemmap
> for NS1 comes from Altmap NS1, which belongs to NS1, and the
> vmemmap for NS2 comes from Altmap NS2, which belongs to NS2.
>
> The vmemmap start for NS2 is not aligned, so Altmap NS2 is shared
> by both NS1 and NS2. During the free operation in NS1, Altmap NS2
> is not part of NS1's altmap, causing it to attempt to free an
> invalid page.
>
> With this patch
> ===
> NS1 start NS2 start
> _
> | NS1 |NS2 |
> -
> | Altmap| Altmap | .| Normal | Altmap | Altmap |...
> | NS1 | NS1 | | Page | NS2 | NS2 |
>
> If the vmemmap start for NS2 is not aligned then we are allocating
> a normal page. NS1 and NS2 vmemmap will be freed correctly.
>
> Fixes: 368a0590d954("powerpc/book3s64/vmemmap: switch radix to use a
> different vmemmap handling function")
> Co-developed-by: Ritesh Harjani (IBM)
> Signed-off-by: Ritesh Harjani (IBM)
> Signed-off-by: Donet Tom
> ---
> arch/powerpc/mm/book3s64/radix_pgtable.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
> b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index 311e2112d782..b22d5f6147d2 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -1120,6 +1120,8 @@ int __meminit radix__vmemmap_populate(unsigned long
> start, unsigned long end, in
> pmd_t *pmd;
> pte_t *pte;
>
/*
* Make sure we align the start vmemmap addr so that we calculate
the correct start_pfn in altmap boundary check to decided whether
we should use altmap or RAM based backing memory allocation. Also
the address need to be aligned for set_pte operation.
If the start addr is already PMD_SIZE aligned we will try to use
a pmd mapping. We don't want to be too aggressive here beacause
that will cause more allocations in RAM. So only if the namespace
vmemmap start addr is PMD_SIZE aligned we will use PMD mapping.
*/
May be with some comments as above?
> + start = ALIGN_DOWN(start, PAGE_SIZE);
> +
> for (addr = start; addr < end; addr = next) {
> next = pmd_addr_end(addr, end);
>
> --
> 2.43.5
Re: [PATCH v5 1/3] printf: convert self-test to KUnit
On Thu 2025-03-06 09:41:44, Tamir Duberstein wrote:
> On Thu, Mar 6, 2025 at 9:25 AM Tamir Duberstein wrote:
> >
> > On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek wrote:
> > >
> > > On Fri 2025-02-21 15:34:30, Tamir Duberstein wrote:
> > > > Convert the printf() self-test to a KUnit test.
> > > >
> > > > [...]
> > > >
> >
> > > 2. What was the motivation to remove the trailing '\n', please?
> > >
> > >It actually makes a difference from the printk() POV. Messages without
> > >the trailing '\n' are _not_ flushed to the console until another
> > >message is added. The reason is that they might still be appended
> > >by pr_cont(). And printk() emits only complete lines to the
> > >console.
> > >
> > >In general, messages should include the trailing '\n' unless the
> > >code wants to append something later or the trailing '\n' is
> > >added by another layer of the code. It does not seem to be this case.
> > >
> > >
> > > > bufsize, fmt, ret, elen);
> > > > - return 1;
> > > > + return;
> > > > }
> > >
> > > [...]
> >
> > I noticed in my testing that the trailing \n didn't change the test
> > output, but I didn't know the details you shared about the trailing
> > \n. I'll restore them, unless we jump straight to the KUNIT macros per
> > the discussion above.
> Ah, I forgot that `tc_fail` already delegates to KUNIT_FAIL. This was
> the reason I removed the trailing newlines -- there is a mix of
> present and absent trailing newlines in KUNIT_* macros, and it's not
> clear to me what the correct thing is. For instance, the examples in
> Documentation/dev-tools/kunit/{start,usage}.rst omit the trailing newlines.
Honestly, I am not able to find how the KUNIT_FAIL() actually prints
the message. I can't find how assert_format() is defined.
Anyway, it seems that for example, kunit_warn() prints the messages
as is in kunit_log(). It does not add the trailing '\n' on its own.
Also I do not see any empty lines when I add back the trailing '\n'
to KUNIT_FAIL() message. This suggests that even KUNIT_FAIL()
prints the messages as is and does not add any extra trailing '\n'.
In my opinion, using the trailing '\n' is the right thing to do
from the printk() POV. Please, add it back.
Best Regards,
Petr
Re: [PATCH v5 1/3] printf: convert self-test to KUnit
On Thu 2025-03-06 09:25:43, Tamir Duberstein wrote:
> On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek wrote:
> >
> > On Fri 2025-02-21 15:34:30, Tamir Duberstein wrote:
> > > Convert the printf() self-test to a KUnit test.
> > >
> > > In the interest of keeping the patch reasonably-sized this doesn't
> > > refactor the tests into proper parameterized tests - it's all one big
> > > test case.
> > >
> > > --- a/lib/test_printf.c
> > > +++ b/lib/tests/printf_kunit.c
> > > @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen,
> > > va_end(aq);
> > >
> > > if (ret != elen) {
> > > - pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d,
> > > expected %d\n",
> > > + tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d,
> > > expected %d",
> >
> > 1. It looks a bit strange that the 1st patch replaces pr_warn() with
> >tc_fail() which hides KUNIT_FAIL().
> >
> >And the 2nd patch replaces tc_fail() with KUNIT_FAIL().
> >
> >It looks like a non-necessary churn.
> >
> >It would be better to avoid the temporary "tc_fail" and swith to
> >KUNIT_FAIL() already in this patch.
> >
> >I did not find any comment about this in the earier versions of the
> >patchset.
> >
> >Is it just a result of the evolution of the patchset or
> >is there any motivation for this?
>
> The motivation was to keep the width of the macro the same in this
> first patch for ease of review, particularly in the 7 instances where
> the invocation wraps to a second line. If you prefer I go straight to
> KUNIT_FAIL, I can make that change.
I see. It might have been useful when the patch removed the trailing '\n'.
But you are going to add it back. So there won't be any hidden change.
So I would prefer to go straight to KUNIT_FAIL().
> > > @@ -842,13 +836,15 @@ test_pointer(void)
> > > fourcc_pointer();
> > > }
> > >
> > > -static void __init selftest(void)
> > > +static void printf_test(struct kunit *test)
> > > {
> > > alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
> > > if (!alloced_buffer)
> > > return;
> >
> > I would use here:
> >
> > KUNIT_ASSERT_NOT_NULL(test, alloced_buffer);
> >
> > And move the same change for the other kmalloc() location from
> > the 2nd patch.
>
> I didn't do that here because I was trying to keep this patch as small
> as possible, and I wrote that in the commit message.
>
> As for using KUNIT_ASSERT_NOT_NULL here, that would have to change
> back to an error return in the 2nd patch because this code moves into
> `suite_init`, which is called with `struct kunit_suite` rather than
> `struct kunit_test`, and KUnit assertion macros do not work with the
> former (and for good reason, because failures in suite setup cannot be
> attributed to a particular test case).
I see. KUNIT_ASSERT_NOT_NULL() can't be used in the .suite_exit() callback.
> So I'd prefer to leave this as is.
I agree to leave this as is.
Best Regards,
Petr
Re: [PATCH v5 1/3] printf: convert self-test to KUnit
On Fri, Mar 7, 2025 at 11:01 AM Petr Mladek wrote:
>
> On Thu 2025-03-06 09:25:43, Tamir Duberstein wrote:
> > On Thu, Mar 6, 2025 at 7:25 AM Petr Mladek wrote:
> > >
> > > On Fri 2025-02-21 15:34:30, Tamir Duberstein wrote:
> > > > Convert the printf() self-test to a KUnit test.
> > > >
> > > > In the interest of keeping the patch reasonably-sized this doesn't
> > > > refactor the tests into proper parameterized tests - it's all one big
> > > > test case.
> > > >
> > > > --- a/lib/test_printf.c
> > > > +++ b/lib/tests/printf_kunit.c
> > > > @@ -57,52 +58,50 @@ do_test(int bufsize, const char *expect, int elen,
> > > > va_end(aq);
> > > >
> > > > if (ret != elen) {
> > > > - pr_warn("vsnprintf(buf, %d, \"%s\", ...) returned %d,
> > > > expected %d\n",
> > > > + tc_fail("vsnprintf(buf, %d, \"%s\", ...) returned %d,
> > > > expected %d",
> > >
> > > 1. It looks a bit strange that the 1st patch replaces pr_warn() with
> > >tc_fail() which hides KUNIT_FAIL().
> > >
> > >And the 2nd patch replaces tc_fail() with KUNIT_FAIL().
> > >
> > >It looks like a non-necessary churn.
> > >
> > >It would be better to avoid the temporary "tc_fail" and swith to
> > >KUNIT_FAIL() already in this patch.
> > >
> > >I did not find any comment about this in the earier versions of the
> > >patchset.
> > >
> > >Is it just a result of the evolution of the patchset or
> > >is there any motivation for this?
> >
> > The motivation was to keep the width of the macro the same in this
> > first patch for ease of review, particularly in the 7 instances where
> > the invocation wraps to a second line. If you prefer I go straight to
> > KUNIT_FAIL, I can make that change.
>
> I see. It might have been useful when the patch removed the trailing '\n'.
> But you are going to add it back. So there won't be any hidden change.
> So I would prefer to go straight to KUNIT_FAIL().
👍 I've restored all the newlines and added a few previously missing ones.
> > > > @@ -842,13 +836,15 @@ test_pointer(void)
> > > > fourcc_pointer();
> > > > }
> > > >
> > > > -static void __init selftest(void)
> > > > +static void printf_test(struct kunit *test)
> > > > {
> > > > alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
> > > > if (!alloced_buffer)
> > > > return;
> > >
> > > I would use here:
> > >
> > > KUNIT_ASSERT_NOT_NULL(test, alloced_buffer);
> > >
> > > And move the same change for the other kmalloc() location from
> > > the 2nd patch.
> >
> > I didn't do that here because I was trying to keep this patch as small
> > as possible, and I wrote that in the commit message.
> >
> > As for using KUNIT_ASSERT_NOT_NULL here, that would have to change
> > back to an error return in the 2nd patch because this code moves into
> > `suite_init`, which is called with `struct kunit_suite` rather than
> > `struct kunit_test`, and KUnit assertion macros do not work with the
> > former (and for good reason, because failures in suite setup cannot be
> > attributed to a particular test case).
>
> I see. KUNIT_ASSERT_NOT_NULL() can't be used in the .suite_exit() callback.
>
> > So I'd prefer to leave this as is.
>
> I agree to leave this as is.
👍
Re: [PATCH 07/13] s390: make setup_zero_pages() use memblock
On Thu, Mar 06, 2025 at 08:51:17PM +0200, Mike Rapoport wrote:
> From: "Mike Rapoport (Microsoft)"
>
> Allocating the zero pages from memblock is simpler because the memory is
> already reserved.
>
> This will also help with pulling out memblock_free_all() to the generic
> code and reducing code duplication in arch::mem_init().
>
> Signed-off-by: Mike Rapoport (Microsoft)
> ---
> arch/s390/mm/init.c | 14 +++---
> 1 file changed, 3 insertions(+), 11 deletions(-)
Acked-by: Heiko Carstens
> - empty_zero_page = __get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
> + empty_zero_page = (unsigned long)memblock_alloc(PAGE_SIZE << order,
> order);
> if (!empty_zero_page)
> panic("Out of memory in setup_zero_pages");
This could have been converted to memblock_alloc_or_panic(), but I
guess this can also be done at a later point in time.
[kvm-unit-tests PATCH] Makefile: Use CFLAGS in cc-option
When cross compiling with clang we need to specify the target in CFLAGS and cc-option will fail to recognize target-specific options without it. Add CFLAGS to the CC invocation in cc-option. Signed-off-by: Andrew Jones --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 78352fced9d4..9dc5d2234e2a 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ DESTDIR := $(PREFIX)/share/kvm-unit-tests/ # cc-option # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0) -cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \ +cc-option = $(shell if $(CC) $(CFLAGS) -Werror $(1) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) libcflat := lib/libcflat.a -- 2.48.1
Re: [kvm-unit-tests PATCH] Makefile: Use CFLAGS in cc-option
On 07/03/2025 09.39, Andrew Jones wrote: When cross compiling with clang we need to specify the target in CFLAGS and cc-option will fail to recognize target-specific options without it. Add CFLAGS to the CC invocation in cc-option. Signed-off-by: Andrew Jones --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 78352fced9d4..9dc5d2234e2a 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ DESTDIR := $(PREFIX)/share/kvm-unit-tests/ # cc-option # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0) -cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \ +cc-option = $(shell if $(CC) $(CFLAGS) -Werror $(1) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) libcflat := lib/libcflat.a Reviewed-by: Thomas Huth
Re: [kvm-unit-tests PATCH] Makefile: Use CFLAGS in cc-option
On Fri, Mar 07, 2025 at 09:42:03AM +0100, Thomas Huth wrote: > On 07/03/2025 09.39, Andrew Jones wrote: > > When cross compiling with clang we need to specify the target in > > CFLAGS and cc-option will fail to recognize target-specific options > > without it. Add CFLAGS to the CC invocation in cc-option. > > > > Signed-off-by: Andrew Jones > > --- > > Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index 78352fced9d4..9dc5d2234e2a 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -21,7 +21,7 @@ DESTDIR := $(PREFIX)/share/kvm-unit-tests/ > > # cc-option > > # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, > > -malign-functions=0) > > -cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \ > > +cc-option = $(shell if $(CC) $(CFLAGS) -Werror $(1) -S -o /dev/null -xc > > /dev/null \ > > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) > > libcflat := lib/libcflat.a > > Reviewed-by: Thomas Huth Thanks, but I just found out that I was too hasty with this patch. I broke x86, /builds/jones-drew/kvm-unit-tests/x86/Makefile.common:105: *** Recursive variable 'CFLAGS' references itself (eventually). Stop. I'll try to sort that out and send a v2. Thanks, drew
[kvm-unit-tests PATCH v2] Makefile: Use CFLAGS in cc-option
When cross compiling with clang we need to specify the target in CFLAGS and cc-option will fail to recognize target-specific options without it. Add CFLAGS to the CC invocation in cc-option. The introduction of the realmode_bits variable is necessary to avoid make failing to build x86 due to CFLAGS referencing itself. Signed-off-by: Andrew Jones --- v2: - Fixed x86 builds with the realmode_bits variable Makefile| 2 +- x86/Makefile.common | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 78352fced9d4..9dc5d2234e2a 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ DESTDIR := $(PREFIX)/share/kvm-unit-tests/ # cc-option # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0) -cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \ +cc-option = $(shell if $(CC) $(CFLAGS) -Werror $(1) -S -o /dev/null -xc /dev/null \ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;) libcflat := lib/libcflat.a diff --git a/x86/Makefile.common b/x86/Makefile.common index 0b7f35c8de85..e97464912e28 100644 --- a/x86/Makefile.common +++ b/x86/Makefile.common @@ -98,6 +98,7 @@ tests-common = $(TEST_DIR)/vmexit.$(exe) $(TEST_DIR)/tsc.$(exe) \ ifneq ($(CONFIG_EFI),y) tests-common += $(TEST_DIR)/realmode.$(exe) \ $(TEST_DIR)/la57.$(exe) +realmode_bits := $(if $(call cc-option,-m16,""),16,32) endif test_cases: $(tests-common) $(tests) @@ -108,7 +109,7 @@ $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o $(LD) -m elf_i386 -nostdlib -o $@ \ -T $(SRCDIR)/$(TEST_DIR)/realmode.lds $^ -$(TEST_DIR)/realmode.o: bits = $(if $(call cc-option,-m16,""),16,32) +$(TEST_DIR)/realmode.o: bits = $(realmode_bits) $(TEST_DIR)/access_test.$(bin): $(TEST_DIR)/access.o -- 2.48.1
[PATCH] powerpc/boot: Fix build with gcc 15
Similar to x86 the ppc boot code does not build with GCC 15.
Copy the fix from
commit ee2ab467bddf ("x86/boot: Use '-std=gnu11' to fix build with GCC 15")
Signed-off-by: Michal Suchanek
---
arch/powerpc/boot/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 1ff6ad4f6cd2..e6b35699c049 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -33,6 +33,7 @@ else
endif
ifdef CONFIG_PPC64_BOOT_WRAPPER
+BOOTTARGETFLAGS+= -std=gnu11
BOOTTARGETFLAGS+= -m64
BOOTTARGETFLAGS+= -mabi=elfv2
ifdef CONFIG_PPC64_ELF_ABI_V2
--
2.47.1
Re: PowerPC: sleftests/powerpc fails to compile linux-next
On 07/03/25 12:32 pm, Madhavan Srinivasan wrote:
On 3/6/25 10:30 PM, Venkat Rao Bagalkote wrote:
Greetings!!
I see selftests/powerpc fails to compile on next-20250306.
This error has been introduced in next-20250218. Make is successful on
next-20250217.
Attached is the .config used.
If you fix this, please add below tag.
Reported-by: Venkat Rao Bagalkote
Errors:
make -C powerpc/
make: Entering directory
'/root/venkat/linux-next/tools/testing/selftests/powerpc'
Makefile:60: warning: overriding recipe for target 'emit_tests'
../lib.mk:182: warning: ignoring old recipe for target 'emit_tests'
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/alignment;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C alignment all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/benchmarks;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C benchmarks all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/cache_shape;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C cache_shape all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/copyloops;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C copyloops all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/dexcr;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C dexcr all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/dscr;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C dscr all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/mm; mkdir
-p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C mm all
CC pkey_exec_prot
In file included from pkey_exec_prot.c:18:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h: In
function ‘pkeys_unsupported’:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
error: ‘PKEY_UNRESTRICTED’ undeclared (first use in this function)
96 | pkey = sys_pkey_alloc(0, PKEY_UNRESTRICTED);
| ^
Commit 6d61527d931ba ('mm/pkey: Add PKEY_UNRESTRICTED macro') added a macro
PKEY_UNRESTRICTED to handle implicit literal
value of 0x0 (which is "unrestricted"). belore patch add the same to powerpc/mm
selftest.
Can you try with this patch to check whether it fixes the build break for you
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h
b/tools/testing/selftests/powerpc/include/pkeys.h
index c6d4063dd4f6..d6deb6ffa1b9 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -24,6 +24,9 @@
#undef PKEY_DISABLE_EXECUTE
#define PKEY_DISABLE_EXECUTE 0x4
+#undef PKEY_UNRESTRICTED
+#define PKEY_UNRESTRICTED 0x0
+
/* Older versions of libc do not define this */
#ifndef SEGV_PKUERR
#define SEGV_PKUERR4
Maddy
Tested with the above patch and it fixes the issue.
Please add below tag.
Tested-by: Venkat Rao Bagalkote
Regards,
Venkat.
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
note: each undeclared identifier is reported only once for each function it
appears in
pkey_exec_prot.c: In function ‘segv_handler’:
pkey_exec_prot.c:75:53: error: ‘PKEY_UNRESTRICTED’ undeclared (first use in
this function)
75 | pkey_set_rights(fault_pkey, PKEY_UNRESTRICTED);
| ^
make[1]: *** [../../lib.mk:222:
/root/venkat/linux-next/tools/testing/selftests/powerpc/mm/pkey_exec_prot]
Error 1
CC pkey_siginfo
In file included from pkey_siginfo.c:22:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h: In
function ‘pkeys_unsupported’:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
error: ‘PKEY_UNRESTRICTED’ undeclared (first use in this function)
96 | pkey = sys_pkey_alloc(0, PKEY_UNRESTRICTED);
| ^
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
note: each undeclared identifier is reported only once for each function it
appears in
pkey_siginfo.c: In function ‘segv_handler’:
pkey_siginfo.c:86:39: error: ‘PKEY_UNRESTRICTED’ undeclared (first use in this
function)
86 | pkey_set_rights(pkey, PKEY_UNRESTRICTED);
| ^
make[1]: *** [../../lib.mk:222:
/root/venkat/linux-next/tools/testing/selftests/powerpc/mm/pkey_siginfo] Error 1
make[1]: Target 'all' not remade because of errors.
make: *** [Makefile:40: mm] Error 2
make: Leaving directory
'/root/venkat/linux-next/tools/testing/selftests/powerpc'
Regards,
Venkat.
Re: PowerPC: sleftests/powerpc fails to compile linux-next
On 07/03/25 12:32 pm, Madhavan Srinivasan wrote:
On 3/6/25 10:30 PM, Venkat Rao Bagalkote wrote:
Greetings!!
I see selftests/powerpc fails to compile on next-20250306.
This error has been introduced in next-20250218. Make is successful on
next-20250217.
Attached is the .config used.
If you fix this, please add below tag.
Reported-by: Venkat Rao Bagalkote
Errors:
make -C powerpc/
make: Entering directory
'/root/venkat/linux-next/tools/testing/selftests/powerpc'
Makefile:60: warning: overriding recipe for target 'emit_tests'
../lib.mk:182: warning: ignoring old recipe for target 'emit_tests'
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/alignment;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C alignment all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/benchmarks;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C benchmarks all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/cache_shape;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C cache_shape all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/copyloops;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C copyloops all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/dexcr;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C dexcr all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/dscr;
mkdir -p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C dscr all
make[1]: Nothing to be done for 'all'.
BUILD_TARGET=/root/venkat/linux-next/tools/testing/selftests/powerpc/mm; mkdir
-p $BUILD_TARGET; make OUTPUT=$BUILD_TARGET -k -C mm all
CC pkey_exec_prot
In file included from pkey_exec_prot.c:18:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h: In
function ‘pkeys_unsupported’:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
error: ‘PKEY_UNRESTRICTED’ undeclared (first use in this function)
96 | pkey = sys_pkey_alloc(0, PKEY_UNRESTRICTED);
| ^
Commit 6d61527d931ba ('mm/pkey: Add PKEY_UNRESTRICTED macro') added a macro
PKEY_UNRESTRICTED to handle implicit literal
value of 0x0 (which is "unrestricted"). belore patch add the same to powerpc/mm
selftest.
Can you try with this patch to check whether it fixes the build break for you
diff --git a/tools/testing/selftests/powerpc/include/pkeys.h
b/tools/testing/selftests/powerpc/include/pkeys.h
index c6d4063dd4f6..d6deb6ffa1b9 100644
--- a/tools/testing/selftests/powerpc/include/pkeys.h
+++ b/tools/testing/selftests/powerpc/include/pkeys.h
@@ -24,6 +24,9 @@
#undef PKEY_DISABLE_EXECUTE
#define PKEY_DISABLE_EXECUTE 0x4
+#undef PKEY_UNRESTRICTED
+#define PKEY_UNRESTRICTED 0x0
+
/* Older versions of libc do not define this */
#ifndef SEGV_PKUERR
#define SEGV_PKUERR4
Maddy
/Responding to all/
/
/
Tested with the above patch and it fixes the issue.
Please add below tag.
Tested-by: Venkat Rao Bagalkote
Regards,
Venkat.
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
note: each undeclared identifier is reported only once for each function it
appears in
pkey_exec_prot.c: In function ‘segv_handler’:
pkey_exec_prot.c:75:53: error: ‘PKEY_UNRESTRICTED’ undeclared (first use in
this function)
75 | pkey_set_rights(fault_pkey, PKEY_UNRESTRICTED);
| ^
make[1]: *** [../../lib.mk:222:
/root/venkat/linux-next/tools/testing/selftests/powerpc/mm/pkey_exec_prot]
Error 1
CC pkey_siginfo
In file included from pkey_siginfo.c:22:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h: In
function ‘pkeys_unsupported’:
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
error: ‘PKEY_UNRESTRICTED’ undeclared (first use in this function)
96 | pkey = sys_pkey_alloc(0, PKEY_UNRESTRICTED);
| ^
/root/venkat/linux-next/tools/testing/selftests/powerpc/include/pkeys.h:96:34:
note: each undeclared identifier is reported only once for each function it
appears in
pkey_siginfo.c: In function ‘segv_handler’:
pkey_siginfo.c:86:39: error: ‘PKEY_UNRESTRICTED’ undeclared (first use in this
function)
86 | pkey_set_rights(pkey, PKEY_UNRESTRICTED);
| ^
make[1]: *** [../../lib.mk:222:
/root/venkat/linux-next/tools/testing/selftests/powerpc/mm/pkey_siginfo] Error 1
make[1]: Target 'all' not remade because of errors.
make: *** [Makefile:40: mm] Error 2
make: Leaving directory
'/root/venkat/linux-next/tools/testing/selftests/powerpc'
R
RE: [PATCH v3 net-next 01/13] net: enetc: add initial netc-lib driver to support NTMP
> On Tue, 4 Mar 2025 15:21:49 +0800 Wei Fang wrote: > > +config NXP_NETC_LIB > > + tristate "NETC Library" > > Remove the string after "tristate", the user should not be prompted > to make a choice for this, since the consumers "select" this config > directly. > Okay, I will remove it. > > + help > > + This module provides common functionalities for both ENETC and NETC > > + Switch, such as NETC Table Management Protocol (NTMP) 2.0, common > tc > > + flower and debugfs interfaces and so on. > > + > > + If compiled as module (M), the module name is nxp-netc-lib. > > Not sure if the help makes sense for an invisible symbol either. Yes, I think it can also be removed. Thanks. > > > config FSL_ENETC > > tristate "ENETC PF driver" > > depends on PCI_MSI > > @@ -40,6 +50,7 @@ config NXP_ENETC4 > > select FSL_ENETC_CORE > > select FSL_ENETC_MDIO > > select NXP_ENETC_PF_COMMON > > + select NXP_NETC_LIB > > select PHYLINK > > select DIMLIB > > help > > > +#pragma pack(1) > > please don't blindly pack all structs, only if they are misaligned > or will otherwise have holes. Because these structures are in hardware buffer format and need to be aligned, so for convenience, I simply used pack(1). You are right, I should use pack() for structures with holes. Thanks. > > > +#if IS_ENABLED(CONFIG_NXP_NETC_LIB) > > why the ifdef, all callers select the config option hm..., there are some interfaces of netc-lib are used in common .c files in downstream, so I used "ifdef" in downstream. Now for the upstream, I'm going to separate them from the common .c files. So yes, we can remove it now.
Re: [PATCH v3 net-next 01/13] net: enetc: add initial netc-lib driver to support NTMP
On Sat, 8 Mar 2025 02:05:35 + Wei Fang wrote: > > > On Tue, 4 Mar 2025 15:21:49 +0800 Wei Fang wrote: > > hm..., there are some interfaces of netc-lib are used in common .c files > > in downstream, so I used "ifdef" in downstream. Now for the upstream, > > I'm going to separate them from the common .c files. So yes, we can > > remove it now. > > Sorry, I misread the header file. The ifdef in ntmp.h is needed because > the interfaces in this header file will be used by the enetc-core and > enetc-vf drivers. For the ENETC v1 (LS1028A platform), it will not select > NXP_NETC_LIB. Meaning FSL_ENETC ? And the calls are in FSL_ENETC_CORE ?
Re: [PATCH v6 0/3] printf: convert self-test to KUnit
On Fri, Mar 07, 2025 at 05:08:55PM -0500, Tamir Duberstein wrote: > This is one of just 3 remaining "Test Module" kselftests (the others > being bitmap and scanf), the rest having been converted to KUnit. > > I tested this using: > > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf > > I have also sent out a series converting scanf[0]. > > Link: > https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee...@gmail.com/T/#u > [0] > > Signed-off-by: Tamir Duberstein > --- > Changes in v6: > - Use __printf correctly on `__test`. (Petr Mladek) > - Rebase on linux-next. Thanks for doing this! If Petr, Rasmus, Andy, and/or others Ack this I can carry it in my "lib/ kunit tests move to lib/tests/" tree, as that's where all the infrastructure in lib/tests/ exists. -Kees -- Kees Cook
[PATCH v2] powerpc/kexec: fix physical address calculation in clear_utlb_entry()
In relocate_32.S, function clear_utlb_entry() goes into real mode. To
do so, it has to calculate the physical address based on the virtual
address. To get the virtual address it uses 'bl' which is problematic
(see commit c974809a26a1 ("powerpc/vdso: Avoid link stack corruption
in __get_datapage()")). In addition, the calculation is done on a
wrong address because 'bl' loads LR with the address of the following
instruction, not the address of the target. So when the target is not
the instruction following the 'bl' instruction, it may lead to
unexpected behaviour.
Fix it by re-writing the code so that is goes via another path which
is based 'bcl 20,31,.+4' which is the right instruction to use for that.
Fixes: 683430200315 ("powerpc/47x: Kernel support for KEXEC")
Signed-off-by: Christophe Leroy
---
v2: Fixed patch description and added Fixes: tag.
[Previous patch subject was "powerpc: Fix address calculation in
clear_utlb_entry()"]
---
arch/powerpc/kexec/relocate_32.S | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kexec/relocate_32.S b/arch/powerpc/kexec/relocate_32.S
index 104c9911f406..dd86e338307d 100644
--- a/arch/powerpc/kexec/relocate_32.S
+++ b/arch/powerpc/kexec/relocate_32.S
@@ -348,16 +348,13 @@ write_utlb:
rlwinm r10, r24, 0, 22, 27
cmpwi r10, PPC47x_TLB0_4K
- bne 0f
li r10, 0x1000 /* r10 = 4k */
- ANNOTATE_INTRA_FUNCTION_CALL
- bl 1f
+ beq 0f
-0:
/* Defaults to 256M */
lis r10, 0x1000
- bcl 20,31,$+4
+0: bcl 20,31,$+4
1: mflrr4
addir4, r4, (2f-1b) /* virtual address of 2f */
--
2.47.0
crng init: was: Re: [PATCH v5 2/3] printf: break kunit into test cases
On Fri 2025-02-21 15:34:31, Tamir Duberstein wrote:
> Move all tests into `printf_test_cases`. This gives us nicer output in
> the event of a failure.
>
> Combine `plain_format` and `plain_hash` into `hash_pointer` since
> they're testing the same scenario.
>
> --- a/lib/tests/printf_kunit.c
> +++ b/lib/tests/printf_kunit.c
> @@ -178,7 +179,7 @@ test_number(void)
> }
>
> static void
> -test_string(void)
> +test_string(struct kunit *kunittest)
> {
> test("", "%s%.0s", "", "123");
> test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456");
> @@ -215,29 +216,6 @@ test_string(void)
> #define ZEROS "" /* hex 32 zero bits */
> #define ONES "" /* hex 32 one bits */
>
> -static int
> -plain_format(void)
> -{
> - char buf[PLAIN_BUF_SIZE];
> - int nchars;
> -
> - nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
> -
> - if (nchars != PTR_WIDTH)
> - return -1;
> -
> - if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> - kunit_warn(kunittest, "crng possibly not yet initialized. plain
> 'p' buffer contains \"%s\"",
> - PTR_VAL_NO_CRNG);
> - return 0;
> - }
> -
> - if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
> - return -1;
> -
> - return 0;
> -}
> -
> #else
>
> #define PTR_WIDTH 8
> @@ -247,89 +225,44 @@ plain_format(void)
> #define ZEROS ""
> #define ONES ""
>
> -static int
> -plain_format(void)
> -{
> - /* Format is implicitly tested for 32 bit machines by plain_hash() */
> - return 0;
> -}
> -
> #endif /* BITS_PER_LONG == 64 */
>
> -static int
> -plain_hash_to_buffer(const void *p, char *buf, size_t len)
> +static void
> +plain_hash_to_buffer(struct kunit *kunittest, const void *p, char *buf,
> size_t len)
> {
> - int nchars;
> -
> - nchars = snprintf(buf, len, "%p", p);
> -
> - if (nchars != PTR_WIDTH)
> - return -1;
> + KUNIT_ASSERT_EQ(kunittest, snprintf(buf, len, "%p", p), PTR_WIDTH);
>
> if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
> kunit_warn(kunittest, "crng possibly not yet initialized. plain
> 'p' buffer contains \"%s\"",
> PTR_VAL_NO_CRNG);
> - return 0;
I have simulated the not-yet-initialized crng and got:
[ 80.109760] printf_kunit: module verification failed: signature and/or
required key missing - tainting kernel
[ 80.114218] KTAP version 1
[ 80.114743] 1..1
[ 80.116124] KTAP version 1
[ 80.116752] # Subtest: printf
[ 80.117239] # module: printf_kunit
[ 80.117256] 1..28
[ 80.120924] ok 1 test_basic
[ 80.121495] ok 2 test_number
[ 80.122741] ok 3 test_string
[ 80.123498] # hash_pointer: crng possibly not yet initialized. plain 'p'
buffer contains "(ptrval)"
[ 80.124044] # hash_pointer: EXPECTATION FAILED at
lib/tests/printf_kunit.c:256
Expected buf == "", but
buf ==
<28><5f><5f><5f><5f><70><74><72>
"" ==
<30><30><30><30><30><30><30><30>
[ 80.125888] not ok 4 hash_pointer
[ 80.129831] ok 5 null_pointer
[ 80.130253] ok 6 error_pointer
[ 80.131221] # invalid_pointer: crng possibly not yet initialized. plain
'p' buffer contains "(ptrval)"
[ 80.132168] ok 7 invalid_pointer
[ 80.135149] ok 8 symbol_ptr
[ 80.136016] ok 9 kernel_ptr
[ 80.136868] ok 10 struct_resource
[ 80.137768] ok 11 struct_range
[ 80.138613] ok 12 addr
[ 80.139370] ok 13 escaped_str
[ 80.140054] ok 14 hex_string
[ 80.140601] ok 15 mac
[ 80.141162] ok 16 ip4
[ 80.141670] ok 17 ip6
[ 80.142221] ok 18 uuid
[ 80.143090] ok 19 dentry
[ 80.143963] ok 20 struct_va_format
[ 80.144523] ok 21 time_and_date
[ 80.145043] ok 22 struct_clk
[ 80.145589] ok 23 bitmap
[ 80.146087] ok 24 netdev_features
[ 80.146572] ok 25 flags
[ 80.146980] # errptr: crng possibly not yet initialized. plain 'p'
buffer contains "(ptrval)"
[ 80.147412] ok 26 errptr
[ 80.148548] ok 27 fwnode_pointer
[ 80.149086] ok 28 fourcc_pointer
[ 80.149090] # printf: ran 448 tests
[ 80.149099] # printf: pass:27 fail:1 skip:0 total:28
[ 80.149102] # Totals: pass:27 fail:1 skip:0 total:28
[ 80.149106] not ok 1 printf
=> One test failed even though vspritf() worked as expected.
The "EXPECTATION FAILED" message was a bit tricky because
it printed "<28><5f><5f><5f><5f><70><74><72>" instead of "(ptrval)".
Two tests succeeded even after a warning message which would make people
to investigate it.
I suggest to rather skip the test in this case. Something like:
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
kunit_skip(kunittest,
"crng possibly no
Re: [PATCH 1/2] tools/perf: Add field to annotation options to determine disassembler used
On Tue, 04 Mar 2025 21:11:13 +0530, Athira Rajeev wrote: > When doing "perf annotate", perf tool provides option to > use specific disassembler like llvm/objdump/capstone. The > order picked is to use llvm first and if that fails fallback > to objdump ie to use PERF_DISASM_LLVM, PERF_DISASM_CAPSTONE > and PERF_DISASM_OBJDUMP > > In powerpc, when using "data type" sort keys, first preferred > approach is to read the raw instruction from the DSO. In objdump > is specified in "--objdump" option, it picks the symbol disassemble > using objdump. Currently disasm_line__parse_powerpc() function > uses length of the "line" to determine if objdump is used. > But there are few cases, where if objdump doesn't recognise the > instruction, the disassembled string will be empty. > > [...] Applied to perf-tools-next, thanks! Best regards, Namhyung
[PATCH v6 1/3] printf: convert self-test to KUnit
Convert the printf() self-test to a KUnit test.
In the interest of keeping the patch reasonably-sized this doesn't
refactor the tests into proper parameterized tests - it's all one big
test case.
Signed-off-by: Tamir Duberstein
---
Documentation/core-api/printk-formats.rst | 4 +-
Documentation/dev-tools/kselftest.rst | 2 +-
MAINTAINERS | 2 +-
lib/Kconfig.debug | 12 +-
lib/Makefile| 1 -
lib/tests/Makefile | 1 +
lib/{test_printf.c => tests/printf_kunit.c} | 207
tools/testing/selftests/kselftest/module.sh | 2 +-
tools/testing/selftests/lib/Makefile| 2 +-
tools/testing/selftests/lib/config | 1 -
tools/testing/selftests/lib/printf.sh | 4 -
11 files changed, 132 insertions(+), 106 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst
b/Documentation/core-api/printk-formats.rst
index e0473da9..4bdc394e86af 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -661,7 +661,7 @@ Do *not* use it from C.
Thanks
==
-If you add other %p extensions, please extend with
-one or more test cases, if at all feasible.
+If you add other %p extensions, please extend
+with one or more test cases, if at all feasible.
Thank you for your cooperation and attention.
diff --git a/Documentation/dev-tools/kselftest.rst
b/Documentation/dev-tools/kselftest.rst
index fdb1df86783a..18c2da67fae4 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -347,7 +347,7 @@ kselftest. We use kselftests for lib/ as an example.
1. Create the test module
2. Create the test script that will run (load/unload) the module
- e.g. ``tools/testing/selftests/lib/printf.sh``
+ e.g. ``tools/testing/selftests/lib/bitmap.sh``
3. Add line to config file e.g. ``tools/testing/selftests/lib/config``
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e9e0e52f92e..1633b62f48c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25599,8 +25599,8 @@ R: Sergey Senozhatsky
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git
F: Documentation/core-api/printk-formats.rst
-F: lib/test_printf.c
F: lib/test_scanf.c
+F: lib/tests/printf_kunit.c
F: lib/vsprintf.c
VT1211 HARDWARE MONITOR DRIVER
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ebb5b190e9f9..3e594d3105f8 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2436,6 +2436,15 @@ config ASYNC_RAID6_TEST
config TEST_HEXDUMP
tristate "Test functions located in the hexdump module at runtime"
+config PRINTF_KUNIT_TEST
+ tristate "KUnit test printf() family of functions at runtime" if
!KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Enable this option to test the printf functions at runtime.
+
+ If unsure, say N.
+
config STRING_KUNIT_TEST
tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
depends on KUNIT
@@ -2449,9 +2458,6 @@ config STRING_HELPERS_KUNIT_TEST
config TEST_KSTRTOX
tristate "Test kstrto*() family of functions at runtime"
-config TEST_PRINTF
- tristate "Test printf() family of functions at runtime"
-
config TEST_SCANF
tristate "Test scanf() family of functions at runtime"
diff --git a/lib/Makefile b/lib/Makefile
index 7bab71e59019..5531a2e727d1 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -77,7 +77,6 @@ obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
-obj-$(CONFIG_TEST_PRINTF) += test_printf.o
obj-$(CONFIG_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index 8961fbcff7a4..183c6a838a5d 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
CFLAGS_overflow_kunit.o = $(call cc-disable-warning,
tautological-constant-out-of-range-compare)
obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
+obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o
obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
obj-$(CONFIG_TEST_SORT) += test_sort.o
diff --git a/lib/test_printf.c b/lib/tests/printf_kunit.c
similarity index 85%
rename from lib/test_printf.c
rename to lib/tests/printf_kunit.c
index 59dbe4f9a4cb..1f4096b015c6 100644
--- a/lib/test_printf.c
+++ b/lib/tests/printf_kunit.c
@@ -3,9 +3,7 @@
* Test cases for printf facility.
*/
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include
+#include
#include
#include
#include
@@ -25,8 +2
[PATCH v6 2/3] printf: break kunit into test cases
Move all tests into `printf_test_cases`. This gives us nicer output in
the event of a failure.
Combine `plain_format` and `plain_hash` into `hash_pointer` since
they're testing the same scenario.
Signed-off-by: Tamir Duberstein
---
lib/tests/printf_kunit.c | 295 ---
1 file changed, 102 insertions(+), 193 deletions(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index 1f4096b015c6..dd373cb9036a 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -38,10 +38,8 @@ static unsigned int total_tests;
static char *test_buffer;
static char *alloced_buffer;
-static struct kunit *kunittest;
-
-static void __printf(4, 0)
-do_test(int bufsize, const char *expect, int elen,
+static void __printf(5, 0)
+do_test(struct kunit *kunittest, int bufsize, const char *expect, int elen,
const char *fmt, va_list ap)
{
va_list aq;
@@ -102,8 +100,8 @@ do_test(int bufsize, const char *expect, int elen,
}
}
-static void __printf(3, 4)
-__test(const char *expect, int elen, const char *fmt, ...)
+static void __printf(4, 5)
+__test(struct kunit *kunittest, const char *expect, int elen, const char *fmt,
...)
{
va_list ap;
int rand;
@@ -124,11 +122,11 @@ __test(const char *expect, int elen, const char *fmt, ...)
* enough and 0), and then we also test that kvasprintf would
* be able to print it as expected.
*/
- do_test(BUF_SIZE, expect, elen, fmt, ap);
+ do_test(kunittest, BUF_SIZE, expect, elen, fmt, ap);
rand = get_random_u32_inclusive(1, elen + 1);
/* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
- do_test(rand, expect, elen, fmt, ap);
- do_test(0, expect, elen, fmt, ap);
+ do_test(kunittest, rand, expect, elen, fmt, ap);
+ do_test(kunittest, 0, expect, elen, fmt, ap);
p = kvasprintf(GFP_KERNEL, fmt, ap);
if (p) {
@@ -144,10 +142,10 @@ __test(const char *expect, int elen, const char *fmt, ...)
}
#define test(expect, fmt, ...) \
- __test(expect, strlen(expect), fmt, ##__VA_ARGS__)
+ __test(kunittest, expect, strlen(expect), fmt, ##__VA_ARGS__)
static void
-test_basic(void)
+test_basic(struct kunit *kunittest)
{
/* Work around annoying "warning: zero-length gnu_printf format
string". */
char nul = '\0';
@@ -155,11 +153,11 @@ test_basic(void)
test("", &nul);
test("100%", "100%%");
test("xxx%yyy", "xxx%cyyy", '%');
- __test("xxx\0yyy", 7, "xxx%cyyy", '\0');
+ __test(kunittest, "xxx\0yyy", 7, "xxx%cyyy", '\0');
}
static void
-test_number(void)
+test_number(struct kunit *kunittest)
{
test("0x1234abcd ", "%#-12x", 0x1234abcd);
test(" 0x1234abcd", "%#12x", 0x1234abcd);
@@ -181,7 +179,7 @@ test_number(void)
}
static void
-test_string(void)
+test_string(struct kunit *kunittest)
{
test("", "%s%.0s", "", "123");
test("ABCD|abc|123", "%s|%.3s|%.*s", "ABCD", "abcdef", 3, "123456");
@@ -218,30 +216,6 @@ test_string(void)
#define ZEROS "" /* hex 32 zero bits */
#define ONES ""/* hex 32 one bits */
-static int
-plain_format(void)
-{
- char buf[PLAIN_BUF_SIZE];
- int nchars;
-
- nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
-
- if (nchars != PTR_WIDTH)
- return -1;
-
- if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
- kunit_warn(kunittest,
- "crng possibly not yet initialized. plain 'p' buffer
contains \"%s\"\n",
- PTR_VAL_NO_CRNG);
- return 0;
- }
-
- if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
- return -1;
-
- return 0;
-}
-
#else
#define PTR_WIDTH 8
@@ -251,90 +225,47 @@ plain_format(void)
#define ZEROS ""
#define ONES ""
-static int
-plain_format(void)
-{
- /* Format is implicitly tested for 32 bit machines by plain_hash() */
- return 0;
-}
-
#endif /* BITS_PER_LONG == 64 */
-static int
-plain_hash_to_buffer(const void *p, char *buf, size_t len)
+static void
+plain_hash_to_buffer(struct kunit *kunittest, const void *p, char *buf, size_t
len)
{
- int nchars;
-
- nchars = snprintf(buf, len, "%p", p);
-
- if (nchars != PTR_WIDTH)
- return -1;
+ KUNIT_ASSERT_EQ(kunittest, snprintf(buf, len, "%p", p), PTR_WIDTH);
if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
- kunit_warn(kunittest,
+ kunit_skip(kunittest,
"crng possibly not yet initialized. plain 'p' buffer
contains \"%s\"\n",
PTR_VAL_NO_CRNG);
- return 0;
}
-
- return 0;
}
-static int
-plain_hash(void)
-{
- char buf[PLAIN_BUF_SIZE];
- int ret;
-
- ret = plain_hash_to_buffer(PTR, buf, PLAIN_BUF_SIZE);
-
[PATCH v6 0/3] printf: convert self-test to KUnit
This is one of just 3 remaining "Test Module" kselftests (the others
being bitmap and scanf), the rest having been converted to KUnit.
I tested this using:
$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf
I have also sent out a series converting scanf[0].
Link:
https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee...@gmail.com/T/#u
[0]
Signed-off-by: Tamir Duberstein
---
Changes in v6:
- Use __printf correctly on `__test`. (Petr Mladek)
- Rebase on linux-next.
- Remove leftover references to `printf.sh`.
- Update comment in `hash_pointer`. (Petr Mladek)
- Avoid overrun in `KUNIT_EXPECT_MEMNEQ`. (Petr Mladek)
- Restore trailing newlines on printk strings and add some missing ones.
(Petr Mladek)
- Use `kunit_skip` on not-yet-initialized crng. (Petr Mladek)
- Link to v5:
https://lore.kernel.org/r/20250221-printf-kunit-convert-v5-0-5db840301...@gmail.com
Changes in v5:
- Update `do_test` `__printf` annotation (Rasmus Villemoes).
- Link to v4:
https://lore.kernel.org/r/20250214-printf-kunit-convert-v4-0-c254572f1...@gmail.com
Changes in v4:
- Add patch "implicate test line in failure messages".
- Rebase on linux-next, move scanf_kunit.c into lib/tests/.
- Link to v3:
https://lore.kernel.org/r/20250210-printf-kunit-convert-v3-0-ee6ac5500...@gmail.com
Changes in v3:
- Remove extraneous trailing newlines from failure messages.
- Replace `pr_warn` with `kunit_warn`.
- Drop arch changes.
- Remove KUnit boilerplate from CONFIG_PRINTF_KUNIT_TEST help text.
- Restore `total_tests` counting.
- Remove tc_fail macro in last patch.
- Link to v2:
https://lore.kernel.org/r/20250207-printf-kunit-convert-v2-0-057b23860...@gmail.com
Changes in v2:
- Incorporate code review from prior work[0] by Arpitha Raghunandan.
- Link to v1:
https://lore.kernel.org/r/20250204-printf-kunit-convert-v1-0-ecf1b846a...@gmail.com
Link:
https://lore.kernel.org/lkml/20200817043028.76502-1-98.a...@gmail.com/t/#u [0]
---
Tamir Duberstein (3):
printf: convert self-test to KUnit
printf: break kunit into test cases
printf: implicate test line in failure messages
Documentation/core-api/printk-formats.rst | 4 +-
Documentation/dev-tools/kselftest.rst | 2 +-
MAINTAINERS | 2 +-
lib/Kconfig.debug | 12 +-
lib/Makefile| 1 -
lib/tests/Makefile | 1 +
lib/{test_printf.c => tests/printf_kunit.c} | 442
tools/testing/selftests/kselftest/module.sh | 2 +-
tools/testing/selftests/lib/Makefile| 2 +-
tools/testing/selftests/lib/config | 1 -
tools/testing/selftests/lib/printf.sh | 4 -
11 files changed, 207 insertions(+), 266 deletions(-)
---
base-commit: 7ec162622e66a4ff886f8f28712ea1b13069e1aa
change-id: 20250131-printf-kunit-convert-fd4012aa2ec6
Best regards,
--
Tamir Duberstein
