Testing __init functions
15 views
Skip to first unread message
Martin Fernandez
Mar 8, 2022, 1:56:31 PM3/8/22
to kuni...@googlegroups.com, Kees Cook, Daniel Gutson
I'm writing some tests for arch/x86/kernel/e820.c, in particular for
some __init functions. When compiling a get the following modpost
warning:
WARNING: modpost: vmlinux.o(.data+0x26800): Section mismatch in
reference from the variable __UNIQUE_ID_array286 to the variable
.init.data:e820_test_suite
The variable __UNIQUE_ID_array286 references
the variable __initdata e820_test_suite
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
where e820_test_suite is the test suite that I'm creating.
My hypothesis is that KUnit is declaring some symbols which are not
marked with __init and the linker doesn't like that. Is that correct?
Is there any workaround?
I'm attaching the complete file at the end of this mail. Bear in mind that it
contains some code of the patch that I'm currently working on.
Thanks.
#include <kunit/test.h>
#include <asm/e820/api.h>
#include <asm/setup.h>
#define KUNIT_EXPECT_E820_ENTRY_EQ(test, entry, _addr, _size, _type, \
_crypto_capable) \
do { \
KUNIT_EXPECT_EQ((test), (entry).addr, (_addr)); \
KUNIT_EXPECT_EQ((test), (entry).size, (_size)); \
KUNIT_EXPECT_EQ((test), (entry).type, (_type)); \
KUNIT_EXPECT_EQ((test), (entry).crypto_capable, \
(_crypto_capable)); \
} while (0)
struct e820_table test_table __initdata;
static void __init test_e820_range_add(struct kunit *test)
{
u32 full;
full = ARRAY_SIZE(test_table.entries);
/* Add last entry. */
test_table.nr_entries = full - 1;
__e820__range_add(&test_table, 0, 15, 0, 0);
KUNIT_EXPECT_EQ(test, test_table.nr_entries, full);
/* Skip new entry when full. */
__e820__range_add(&test_table, 0, 15, 0, 0);
KUNIT_EXPECT_EQ(test, test_table.nr_entries, full);
}
static void __init test_e820_range_update(struct kunit *test)
{
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size * 2, entry_size,
E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__range_update(&test_table, 0, entry_size * 2,
E820_TYPE_RAM, E820_TYPE_RESERVED);
/* The first 2 regions were updated */
KUNIT_EXPECT_EQ(test, updated_size, entry_size * 2);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size,
E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size,
entry_size, E820_TYPE_RESERVED,
E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_ACPI,
E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__range_update(&test_table, 0, entry_size * 3,
E820_TYPE_RESERVED, E820_TYPE_RAM);
/* Only the first 2 regions were updated */
KUNIT_EXPECT_EQ(test, updated_size, entry_size * 2);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size,
E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size,
entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_ACPI,
E820_NOT_CRYPTO_CAPABLE);
}
static void __init test_e820_range_remove(struct kunit *test)
{
u64 entry_size = 15;
u64 removed_size = 0;
struct e820_entry_updater updater = {
.should_update = remover__should_update,
.update = remover__update,
.new = remover__new
};
struct e820_remover_data data = {
.check_type = true,
.old_type = E820_TYPE_RAM
};
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size * 2, entry_size,
E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE);
/*
* Need to use __e820__handle_range_update because
* e820__range_remove doesn't ask for the table
*/
removed_size = __e820__handle_range_update(&test_table,
0, entry_size * 2,
&updater, &data);
/* The first two regions were removed */
KUNIT_EXPECT_EQ(test, removed_size, entry_size * 2);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 0, 0, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 0, 0, 0);
removed_size = __e820__handle_range_update(&test_table,
0, entry_size * 3,
&updater, &data);
/* Nothing was removed */
KUNIT_EXPECT_EQ(test, removed_size, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 0, 0, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 0, 0, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_ACPI,
E820_NOT_CRYPTO_CAPABLE);
}
static void __init test_e820_range_crypto_update(struct kunit *test)
{
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size * 2, entry_size,
E820_TYPE_RAM, E820_CRYPTO_CAPABLE);
updated_size = __e820__range_update_crypto(&test_table, 0, entry_size * 3,
E820_CRYPTO_CAPABLE);
/* Only the region in the middle was updated */
KUNIT_EXPECT_EQ(test, updated_size, entry_size);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size,
E820_TYPE_RAM, E820_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size,
entry_size, E820_TYPE_RAM,
E820_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_RAM,
E820_CRYPTO_CAPABLE);
}
static void __init test_e820_handle_range_update_intersection(struct
kunit *test)
{
struct e820_entry_updater updater = {
.should_update = type_updater__should_update,
.update = type_updater__update,
.new = type_updater__new
};
struct e820_type_updater_data data = {
.old_type = E820_TYPE_RAM,
.new_type = E820_TYPE_RESERVED
};
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__handle_range_update(&test_table,
0, entry_size - 2,
&updater, &data);
KUNIT_EXPECT_EQ(test, updated_size, entry_size - 2);
/* There is a new entry */
KUNIT_EXPECT_EQ(test, test_table.nr_entries, 2);
/* The original entry now is moved */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], entry_size - 2,
2, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
/* The new entry has the correct values */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 13,
E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE);
}
static void __init test_e820_handle_range_update_inside(struct kunit *test)
{
struct e820_entry_updater updater = {
.should_update = type_updater__should_update,
.update = type_updater__update,
.new = type_updater__new
};
struct e820_type_updater_data data = {
.old_type = E820_TYPE_RAM,
.new_type = E820_TYPE_RESERVED
};
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__handle_range_update(&test_table,
5, entry_size - 10,
&updater, &data);
KUNIT_EXPECT_EQ(test, updated_size, entry_size - 10);
/* There are two new entrie */
KUNIT_EXPECT_EQ(test, test_table.nr_entries, 3);
/* The original entry now shrinked */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 5,
E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
/* The new entries have the correct values */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 5,
entry_size - 10, E820_TYPE_RESERVED,
E820_NOT_CRYPTO_CAPABLE);
/* Left over of the original region */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size - 5,
5, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
}
static struct kunit_case e820_test_cases[] __initdata = {
KUNIT_CASE(test_e820_range_add),
KUNIT_CASE(test_e820_range_update),
KUNIT_CASE(test_e820_range_remove),
KUNIT_CASE(test_e820_range_crypto_update),
KUNIT_CASE(test_e820_handle_range_update_intersection),
KUNIT_CASE(test_e820_handle_range_update_inside),
{}
};
static struct kunit_suite e820_test_suite __initdata = {
.name = "e820",
.test_cases = e820_test_cases,
};
kunit_test_suite(e820_test_suite);
some __init functions. When compiling a get the following modpost
warning:
WARNING: modpost: vmlinux.o(.data+0x26800): Section mismatch in
reference from the variable __UNIQUE_ID_array286 to the variable
.init.data:e820_test_suite
The variable __UNIQUE_ID_array286 references
the variable __initdata e820_test_suite
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
where e820_test_suite is the test suite that I'm creating.
My hypothesis is that KUnit is declaring some symbols which are not
marked with __init and the linker doesn't like that. Is that correct?
Is there any workaround?
I'm attaching the complete file at the end of this mail. Bear in mind that it
contains some code of the patch that I'm currently working on.
Thanks.
#include <kunit/test.h>
#include <asm/e820/api.h>
#include <asm/setup.h>
#define KUNIT_EXPECT_E820_ENTRY_EQ(test, entry, _addr, _size, _type, \
_crypto_capable) \
do { \
KUNIT_EXPECT_EQ((test), (entry).addr, (_addr)); \
KUNIT_EXPECT_EQ((test), (entry).size, (_size)); \
KUNIT_EXPECT_EQ((test), (entry).type, (_type)); \
KUNIT_EXPECT_EQ((test), (entry).crypto_capable, \
(_crypto_capable)); \
} while (0)
struct e820_table test_table __initdata;
static void __init test_e820_range_add(struct kunit *test)
{
u32 full;
full = ARRAY_SIZE(test_table.entries);
/* Add last entry. */
test_table.nr_entries = full - 1;
__e820__range_add(&test_table, 0, 15, 0, 0);
KUNIT_EXPECT_EQ(test, test_table.nr_entries, full);
/* Skip new entry when full. */
__e820__range_add(&test_table, 0, 15, 0, 0);
KUNIT_EXPECT_EQ(test, test_table.nr_entries, full);
}
static void __init test_e820_range_update(struct kunit *test)
{
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size * 2, entry_size,
E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__range_update(&test_table, 0, entry_size * 2,
E820_TYPE_RAM, E820_TYPE_RESERVED);
/* The first 2 regions were updated */
KUNIT_EXPECT_EQ(test, updated_size, entry_size * 2);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size,
E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size,
entry_size, E820_TYPE_RESERVED,
E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_ACPI,
E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__range_update(&test_table, 0, entry_size * 3,
E820_TYPE_RESERVED, E820_TYPE_RAM);
/* Only the first 2 regions were updated */
KUNIT_EXPECT_EQ(test, updated_size, entry_size * 2);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size,
E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size,
entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_ACPI,
E820_NOT_CRYPTO_CAPABLE);
}
static void __init test_e820_range_remove(struct kunit *test)
{
u64 entry_size = 15;
u64 removed_size = 0;
struct e820_entry_updater updater = {
.should_update = remover__should_update,
.update = remover__update,
.new = remover__new
};
struct e820_remover_data data = {
.check_type = true,
.old_type = E820_TYPE_RAM
};
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size * 2, entry_size,
E820_TYPE_ACPI, E820_NOT_CRYPTO_CAPABLE);
/*
* Need to use __e820__handle_range_update because
* e820__range_remove doesn't ask for the table
*/
removed_size = __e820__handle_range_update(&test_table,
0, entry_size * 2,
&updater, &data);
/* The first two regions were removed */
KUNIT_EXPECT_EQ(test, removed_size, entry_size * 2);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 0, 0, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 0, 0, 0);
removed_size = __e820__handle_range_update(&test_table,
0, entry_size * 3,
&updater, &data);
/* Nothing was removed */
KUNIT_EXPECT_EQ(test, removed_size, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 0, 0, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 0, 0, 0);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_ACPI,
E820_NOT_CRYPTO_CAPABLE);
}
static void __init test_e820_range_crypto_update(struct kunit *test)
{
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
__e820__range_add(&test_table, entry_size * 2, entry_size,
E820_TYPE_RAM, E820_CRYPTO_CAPABLE);
updated_size = __e820__range_update_crypto(&test_table, 0, entry_size * 3,
E820_CRYPTO_CAPABLE);
/* Only the region in the middle was updated */
KUNIT_EXPECT_EQ(test, updated_size, entry_size);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, entry_size,
E820_TYPE_RAM, E820_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], entry_size,
entry_size, E820_TYPE_RAM,
E820_CRYPTO_CAPABLE);
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size * 2,
entry_size, E820_TYPE_RAM,
E820_CRYPTO_CAPABLE);
}
static void __init test_e820_handle_range_update_intersection(struct
kunit *test)
{
struct e820_entry_updater updater = {
.should_update = type_updater__should_update,
.update = type_updater__update,
.new = type_updater__new
};
struct e820_type_updater_data data = {
.old_type = E820_TYPE_RAM,
.new_type = E820_TYPE_RESERVED
};
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__handle_range_update(&test_table,
0, entry_size - 2,
&updater, &data);
KUNIT_EXPECT_EQ(test, updated_size, entry_size - 2);
/* There is a new entry */
KUNIT_EXPECT_EQ(test, test_table.nr_entries, 2);
/* The original entry now is moved */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], entry_size - 2,
2, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
/* The new entry has the correct values */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 0, 13,
E820_TYPE_RESERVED, E820_NOT_CRYPTO_CAPABLE);
}
static void __init test_e820_handle_range_update_inside(struct kunit *test)
{
struct e820_entry_updater updater = {
.should_update = type_updater__should_update,
.update = type_updater__update,
.new = type_updater__new
};
struct e820_type_updater_data data = {
.old_type = E820_TYPE_RAM,
.new_type = E820_TYPE_RESERVED
};
u64 entry_size = 15;
u64 updated_size = 0;
/* Initialize table */
test_table.nr_entries = 0;
__e820__range_add(&test_table, 0, entry_size, E820_TYPE_RAM,
E820_NOT_CRYPTO_CAPABLE);
updated_size = __e820__handle_range_update(&test_table,
5, entry_size - 10,
&updater, &data);
KUNIT_EXPECT_EQ(test, updated_size, entry_size - 10);
/* There are two new entrie */
KUNIT_EXPECT_EQ(test, test_table.nr_entries, 3);
/* The original entry now shrinked */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[0], 0, 5,
E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
/* The new entries have the correct values */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[1], 5,
entry_size - 10, E820_TYPE_RESERVED,
E820_NOT_CRYPTO_CAPABLE);
/* Left over of the original region */
KUNIT_EXPECT_E820_ENTRY_EQ(test, test_table.entries[2], entry_size - 5,
5, E820_TYPE_RAM, E820_NOT_CRYPTO_CAPABLE);
}
static struct kunit_case e820_test_cases[] __initdata = {
KUNIT_CASE(test_e820_range_add),
KUNIT_CASE(test_e820_range_update),
KUNIT_CASE(test_e820_range_remove),
KUNIT_CASE(test_e820_range_crypto_update),
KUNIT_CASE(test_e820_handle_range_update_intersection),
KUNIT_CASE(test_e820_handle_range_update_inside),
{}
};
static struct kunit_suite e820_test_suite __initdata = {
.name = "e820",
.test_cases = e820_test_cases,
};
kunit_test_suite(e820_test_suite);
Daniel Latypov
Mar 8, 2022, 3:58:35 PM3/8/22
to Martin Fernandez, kuni...@googlegroups.com, Kees Cook, Daniel Gutson
On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
Development <kuni...@googlegroups.com> wrote:
>
> I'm writing some tests for arch/x86/kernel/e820.c, in particular for
> some __init functions. When compiling a get the following modpost
> warning:
>
> WARNING: modpost: vmlinux.o(.data+0x26800): Section mismatch in
> reference from the variable __UNIQUE_ID_array286 to the variable
> .init.data:e820_test_suite
> The variable __UNIQUE_ID_array286 references
> the variable __initdata e820_test_suite
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>
> where e820_test_suite is the test suite that I'm creating.
>
> My hypothesis is that KUnit is declaring some symbols which are not
> marked with __init and the linker doesn't like that. Is that correct?
That is correct.
Development <kuni...@googlegroups.com> wrote:
>
> I'm writing some tests for arch/x86/kernel/e820.c, in particular for
> some __init functions. When compiling a get the following modpost
> warning:
>
> WARNING: modpost: vmlinux.o(.data+0x26800): Section mismatch in
> reference from the variable __UNIQUE_ID_array286 to the variable
> .init.data:e820_test_suite
> The variable __UNIQUE_ID_array286 references
> the variable __initdata e820_test_suite
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>
> where e820_test_suite is the test suite that I'm creating.
>
> My hypothesis is that KUnit is declaring some symbols which are not
> marked with __init and the linker doesn't like that. Is that correct?
It's here: https://elixir.bootlin.com/linux/v5.17-rc7/source/include/kunit/test.h#L351
> Is there any workaround?
Nothing clean at this time.
Locally, you can update the macro to put `__init` on the array to
verify that helps (it should).
I think we'd need a new macro for this that also wraps
__kunit_test_suites(), but applies __init.
Brendan, thoughts?
Brendan Higgins
Mar 9, 2022, 3:18:09 AM3/9/22
to Daniel Latypov, Martin Fernandez, kuni...@googlegroups.com, Kees Cook, Daniel Gutson
On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
I agree - well mostly: ".kunit_test_suites" is already defined to be
in the INIT_DATA section, so it is necessarily safe to use it with
functions and data marked __init; however, I don't know that it's OK
to specify the __section tag twice.
So what we probably need to do is to make modpost respect that KUnit
can safely ignore what section data and code live in.
All that being said, I think I would like to make a new
__kunit_test_init_suites() (or something like that) which does the
same thing as the regular flavor but doesn't cause modpost to complain
so that if we ever allow tests to be run dynamically after the kernel
is booted, we can safely ignore tests that depend on __init stuff.
in the INIT_DATA section, so it is necessarily safe to use it with
functions and data marked __init; however, I don't know that it's OK
to specify the __section tag twice.
So what we probably need to do is to make modpost respect that KUnit
can safely ignore what section data and code live in.
All that being said, I think I would like to make a new
__kunit_test_init_suites() (or something like that) which does the
same thing as the regular flavor but doesn't cause modpost to complain
so that if we ever allow tests to be run dynamically after the kernel
is booted, we can safely ignore tests that depend on __init stuff.
Brendan Higgins
Mar 9, 2022, 3:52:56 AM3/9/22
to Daniel Latypov, Martin Fernandez, kuni...@googlegroups.com, Kees Cook, Daniel Gutson
that introduces the new __kunit_test_init_suites() which just names
the array and suites suffixed with "_probe" or something - that should
make modpost happy for now and will give us the flexibility to do
something a bit smarter down the road. I can try this out tomorrow if
no one has any objections.
Brendan Higgins
Mar 9, 2022, 3:27:31 PM3/9/22
to Daniel Latypov, Martin Fernandez, kuni...@googlegroups.com, Kees Cook, Daniel Gutson
On Wed, Mar 9, 2022 at 3:52 AM Brendan Higgins
your code or otherwise share the patches?
It seems the code you posted here depends on some symbols which are
not in Linus' tree (for example E820_NOT_CRYPTO_CAPABLE).
I just want to be able to build your test to make sure I fix your problem.
Cheers
<brendan...@google.com> wrote:
>
> On Wed, Mar 9, 2022 at 3:17 AM Brendan Higgins
> <brendan...@google.com> wrote:
> >
> > On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
> > <kuni...@googlegroups.com> wrote:
> > >
> > > On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
> > > Development <kuni...@googlegroups.com> wrote:
> > > >
> > > > I'm writing some tests for arch/x86/kernel/e820.c, in particular for
> > > > some __init functions. When compiling a get the following modpost
> > > > warning:
Martin, can you share a link to a tree with a buildable version of
>
> On Wed, Mar 9, 2022 at 3:17 AM Brendan Higgins
> <brendan...@google.com> wrote:
> >
> > On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
> > <kuni...@googlegroups.com> wrote:
> > >
> > > On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
> > > Development <kuni...@googlegroups.com> wrote:
> > > >
> > > > I'm writing some tests for arch/x86/kernel/e820.c, in particular for
> > > > some __init functions. When compiling a get the following modpost
> > > > warning:
your code or otherwise share the patches?
It seems the code you posted here depends on some symbols which are
not in Linus' tree (for example E820_NOT_CRYPTO_CAPABLE).
I just want to be able to build your test to make sure I fix your problem.
Cheers
Martin Fernandez
Mar 9, 2022, 5:07:43 PM3/9/22
to Brendan Higgins, Daniel Latypov, kuni...@googlegroups.com, Kees Cook, Daniel Gutson
On 3/9/22, Brendan Higgins <brendan...@google.com> wrote:
> On Wed, Mar 9, 2022 at 3:52 AM Brendan Higgins
> <brendan...@google.com> wrote:
>>
>> On Wed, Mar 9, 2022 at 3:17 AM Brendan Higgins
>> <brendan...@google.com> wrote:
>> >
>> > On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
>> > <kuni...@googlegroups.com> wrote:
>> > >
>> > > On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
>> > > Development <kuni...@googlegroups.com> wrote:
>> > > >
>> > > > I'm writing some tests for arch/x86/kernel/e820.c, in particular
>> > > > for
>> > > > some __init functions. When compiling a get the following modpost
>> > > > warning:
>
> Martin, can you share a link to a tree with a buildable version of
> your code or otherwise share the patches?
>
> It seems the code you posted here depends on some symbols which are
> not in Linus' tree (for example E820_NOT_CRYPTO_CAPABLE).
>
> I just want to be able to build your test to make sure I fix your problem.
Sure!
> On Wed, Mar 9, 2022 at 3:52 AM Brendan Higgins
> <brendan...@google.com> wrote:
>>
>> On Wed, Mar 9, 2022 at 3:17 AM Brendan Higgins
>> <brendan...@google.com> wrote:
>> >
>> > On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
>> > <kuni...@googlegroups.com> wrote:
>> > >
>> > > On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
>> > > Development <kuni...@googlegroups.com> wrote:
>> > > >
>> > > > I'm writing some tests for arch/x86/kernel/e820.c, in particular
>> > > > for
>> > > > some __init functions. When compiling a get the following modpost
>> > > > warning:
>
> Martin, can you share a link to a tree with a buildable version of
> your code or otherwise share the patches?
>
> It seems the code you posted here depends on some symbols which are
> not in Linus' tree (for example E820_NOT_CRYPTO_CAPABLE).
>
> I just want to be able to build your test to make sure I fix your problem.
This patch below it's enough for cause the modpost warning and can be
applied to linus's tree. I prefered to do it this way because I need
more testing for my patch I don't want you to deal with that :)
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index d3a6f74a94bd..6449e6f00f07 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -225,6 +225,11 @@ config PUNIT_ATOM_DEBUG
The current power state can be read from
/sys/kernel/debug/punit_atom/dev_power_state
+config E820_KUNIT_TEST
+ tristate "Tests for E820" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y
+ default KUNIT_ALL_TESTS
+
choice
prompt "Choose kernel unwinder"
default UNWINDER_ORC if X86_64
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index bc0657f0deed..13a10f04d4fe 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1329,3 +1329,8 @@ void __init e820__memblock_setup(void)
memblock_dump_all();
}
+
+#ifdef CONFIG_E820_KUNIT_TEST
+/* Let e820_test have access the static functions in this file */
+#include "e820_test.c"
+#endif
diff --git a/arch/x86/kernel/e820_test.c b/arch/x86/kernel/e820_test.c
new file mode 100644
index 000000000000..48050abbab71
--- /dev/null
+++ b/arch/x86/kernel/e820_test.c
@@ -0,0 +1,39 @@
+#include <kunit/test.h>
+
+#include <asm/e820/api.h>
+#include <asm/setup.h>
+
+#define KUNIT_EXPECT_E820_ENTRY_EQ(test, entry, _addr, _size, _type)
\
+ do { \
+ KUNIT_EXPECT_EQ((test), (entry).addr, (_addr)); \
+ KUNIT_EXPECT_EQ((test), (entry).size, (_size)); \
+ KUNIT_EXPECT_EQ((test), (entry).type, (_type)); \
+ } while (0)
+
+struct e820_table test_table __initdata;
+
+static void __init test_e820_range_add(struct kunit *test)
+{
+ u32 full;
+
+ full = ARRAY_SIZE(test_table.entries);
+ /* Add last entry. */
+ test_table.nr_entries = full - 1;
+ __e820__range_add(&test_table, 0, 15, 0);
+ KUNIT_EXPECT_EQ(test, test_table.nr_entries, full);
+ /* Skip new entry when full. */
+ __e820__range_add(&test_table, 0, 15, 0);
+ KUNIT_EXPECT_EQ(test, test_table.nr_entries, full);
+}
+
+static struct kunit_case e820_test_cases[] __initdata = {
+ KUNIT_CASE(test_e820_range_add),
+ {}
+};
+
+static struct kunit_suite e820_test_suite __initdata = {
+ .name = "e820",
+ .test_cases = e820_test_cases,
+};
+
+kunit_test_suite(e820_test_suite);
Brendan Higgins
Mar 10, 2022, 4:13:00 PM3/10/22
to Martin Fernandez, Daniel Latypov, kuni...@googlegroups.com, Kees Cook, Daniel Gutson
On Wed, Mar 09, 2022 at 07:07:41PM -0300, Martin Fernandez wrote:
> On 3/9/22, Brendan Higgins <brendan...@google.com> wrote:
> > On Wed, Mar 9, 2022 at 3:52 AM Brendan Higgins
> > <brendan...@google.com> wrote:
> >>
> >> On Wed, Mar 9, 2022 at 3:17 AM Brendan Higgins
> >> <brendan...@google.com> wrote:
> >> >
> >> > On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
> >> > <kuni...@googlegroups.com> wrote:
> >> > >
> >> > > On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
> >> > > Development <kuni...@googlegroups.com> wrote:
> >> > > >
> >> > > > I'm writing some tests for arch/x86/kernel/e820.c, in particular
> >> > > > for
> >> > > > some __init functions. When compiling a get the following modpost
> >> > > > warning:
> >
> > Martin, can you share a link to a tree with a buildable version of
> > your code or otherwise share the patches?
> >
> > It seems the code you posted here depends on some symbols which are
> > not in Linus' tree (for example E820_NOT_CRYPTO_CAPABLE).
> >
> > I just want to be able to build your test to make sure I fix your problem.
>
> Sure!
>
> This patch below it's enough for cause the modpost warning and can be
> applied to linus's tree. I prefered to do it this way because I need
> more testing for my patch I don't want you to deal with that :)
Thanks! Works perfectly. I put together a patch that I believe should
> On 3/9/22, Brendan Higgins <brendan...@google.com> wrote:
> > On Wed, Mar 9, 2022 at 3:52 AM Brendan Higgins
> > <brendan...@google.com> wrote:
> >>
> >> On Wed, Mar 9, 2022 at 3:17 AM Brendan Higgins
> >> <brendan...@google.com> wrote:
> >> >
> >> > On Tue, Mar 8, 2022 at 3:58 PM 'Daniel Latypov' via KUnit Development
> >> > <kuni...@googlegroups.com> wrote:
> >> > >
> >> > > On Tue, Mar 8, 2022 at 12:56 PM 'Martin Fernandez' via KUnit
> >> > > Development <kuni...@googlegroups.com> wrote:
> >> > > >
> >> > > > I'm writing some tests for arch/x86/kernel/e820.c, in particular
> >> > > > for
> >> > > > some __init functions. When compiling a get the following modpost
> >> > > > warning:
> >
> > Martin, can you share a link to a tree with a buildable version of
> > your code or otherwise share the patches?
> >
> > It seems the code you posted here depends on some symbols which are
> > not in Linus' tree (for example E820_NOT_CRYPTO_CAPABLE).
> >
> > I just want to be able to build your test to make sure I fix your problem.
>
> Sure!
>
> This patch below it's enough for cause the modpost warning and can be
> applied to linus's tree. I prefered to do it this way because I need
> more testing for my patch I don't want you to deal with that :)
address your issue:
https://lore.kernel.org/linux-kselftest/20220310210210.2124...@google.com/
You will then need to apply the following diff to your patch:
diff --git a/arch/x86/kernel/e820_test.c b/arch/x86/kernel/e820_test.c
index cb5719a507a5..5ae4ea2bb20a 100644
--- a/arch/x86/kernel/e820_test.c
+++ b/arch/x86/kernel/e820_test.c
@@ -36,4 +36,4 @@ static struct kunit_suite e820_test_suite __initdata = {
.test_cases = e820_test_cases,
};
-kunit_test_suite(e820_test_suite);
+kunit_test_init_suite(e820_test_suite);
You can also see the patch applied with your patch on our public Gerrit
instance (along with the above diff):
https://kunit-review.googlesource.com/c/linux/+/5209
If you wouldn't mind, please double check that my fix in fact works for
you and give me a Tested-by on the LKML thread.
Thanks!
Reply all
Reply to author
Forward
0 new messages