clk: Add kunit tests for fixed rate and parent data

We'd like to apply overlays to the root node in KUnit so we can test platform devices created as children of the root node.

On some architectures (powerpc), the root node isn't marked with OF_POPULATED_BUS. If an overlay tries to modify the root node on these platforms it will fail, while on other platforms, such as ARM, it will succeed. This is because the root node is marked with OF_POPULATED_BUS by of_platform_default_populate_init() calling of_platform_default_populate() with NULL as the first argument.

Loosen the requirement here so that platform devices can be created for nodes created as children of the root node via DT overlays even if the platform bus wasn't populated for the root node.

Reviewed-by Rob Herring (Arm) <[email protected]>

Cc Saravana Kannan <[email protected]>

Signed-off-by Geert Uytterhoeven <[email protected]>

[[email protected]: Folded in condition fix]

Signed-off-by Stephen Boyd <[email protected]>

---

Add test managed wrappers for of_overlay_apply() that automatically removes the overlay when the test is finished. This API is intended for use by KUnit tests that test code which relies on 'struct device_node's and of_*() APIs.

KUnit tests will call of_overlay_apply_kunit() to load an overlay that's been built into the kernel image. When the test is complete, the overlay will be removed.

This has a few benefits:

1) It keeps the tests hermetic because the overlay is removed when the test is complete. Tests won't even be aware that an overlay was loaded in another test.

2) The overlay code can live right next to the unit test that loads it. The overlay and the unit test can be compiled into one kernel module if desired.

3) We can test different device tree configurations by loading different overlays. The overlays can be written for a specific test, and there can be many of them loaded per-test without needing to jam all possible combinations into one DTB.

4) It also allows KUnit to test device tree dependent code on any architecture, not just UML. This allows KUnit tests to test architecture specific device tree code.

There are some potential pitfalls though. Test authors need to be careful to not overwrite properties in the live tree. The easiest way to do this is to add and remove nodes with a 'kunit-' prefix, almost guaranteeing that the same node won't be present in the tree loaded at boot.

Suggested-by Rob Herring <[email protected]>

Cc Rob Herring <[email protected]>

Cc Saravana Kannan <[email protected]>

Reviewed-by Rob Herring (Arm) <[email protected]>

Reviewed-by David Gow <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Add the vendor prefix "test" to reserve a vendor prefix for bindings that are purely for testing device tree code. This allows test code to write bindings that can be checked by the schema validator.

Reviewed-by Rob Herring <[email protected]>

Reviewed-by David Gow <[email protected]>

Cc Krzysztof Kozlowski <[email protected]>

Cc Conor Dooley <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Test the KUnit test managed overlay APIs. Confirm that platform devices are created and destroyed properly. This provides us confidence that the test managed APIs work correctly and can be relied upon to provide tests with fake platform devices and device nodes via overlays compiled into the kernel image.

Cc Rob Herring <[email protected]>

Cc Saravana Kannan <[email protected]>

Cc Daniel Latypov <[email protected]>

Cc Brendan Higgins <[email protected]>

Reviewed-by David Gow <[email protected]>

Cc Rae Moar <[email protected]>

Reviewed-by Rob Herring (Arm) <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Introduce KUnit resource wrappers around platform_driver_register(), platform_device_alloc(), and platform_device_add() so that test authors can register platform drivers/devices from their tests and have the drivers/devices automatically be unregistered when the test is done.

This makes test setup code simpler when a platform driver or platform device is needed. Add a few test cases at the same time to make sure the APIs work as intended.

Cc Brendan Higgins <[email protected]>

Reviewed-by David Gow <[email protected]>

Cc Rae Moar <[email protected]>

Reviewed-by Greg Kroah-Hartman <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Introduce KUnit resource wrappers around platform_driver_register(), platform_device_alloc(), and platform_device_add() so that test authors can register platform drivers/devices from their tests and have the drivers/devices automatically be unregistered when the test is done.

This makes test setup code simpler when a platform driver or platform device is needed. Add a few test cases at the same time to make sure the APIs work as intended.

Cc Brendan Higgins <[email protected]>

Reviewed-by David Gow <[email protected]>

Cc Rae Moar <[email protected]>

Reviewed-by Greg Kroah-Hartman <[email protected]>

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

Unit tests are more ergonomic and simpler to understand if they don't have to hoist a bunch of code into the test harness init and exit functions. Add some test managed wrappers for the clk APIs so that clk unit tests can write more code in the actual test and less code in the harness.

Only add APIs that are used for now. More wrappers can be added in the future as necessary.

Cc Brendan Higgins <[email protected]>

Cc David Gow <[email protected]>

Cc Rae Moar <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Test that the fixed rate basic type clk works as intended.

Cc Brendan Higgins <[email protected]>

Cc David Gow <[email protected]>

Cc Rae Moar <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Cc Christian Marangi <[email protected]>

Cc Brendan Higgins <[email protected]>

Reviewed-by David Gow <[email protected]>

Cc Rae Moar <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Thanks,
Guenter

---

[   20.464809]     KTAP version 1
[   20.464865]     # Subtest: clk_register_clk_parent_data_of
[   20.464936]     # module: clk_test
[   20.464979]     1..1
[   20.465098]         KTAP version 1
[   20.465208]         # Subtest: clk_register_clk_parent_data_of_test
[   20.468964] OF: overlay: find target, node: /fragment@0, path '/' not found
[   20.469558] OF: overlay: init_overlay_changeset() failed, ret = -22
[   20.470177]     # clk_register_clk_parent_data_of_test: ASSERTION FAILED at drivers/clk/clk_test.c:2760
[   20.470177]     Expected 0 == ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }), but
[   20.470177]         ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }) == -22 (0xffffffffffffffea)
[   20.471793]         not ok 1 clk_parent_data_of_index_test
[   20.474095] OF: overlay: find target, node: /fragment@0, path '/' not found
[   20.474373] OF: overlay: init_overlay_changeset() failed, ret = -22
[   20.474737]     # clk_register_clk_parent_data_of_test: ASSERTION FAILED at drivers/clk/clk_test.c:2760
[   20.474737]     Expected 0 == ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }), but
[   20.474737]         ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }) == -22 (0xffffffffffffffea)
[   20.477677]         not ok 2 clk_parent_data_of_fwname_test
[   20.479773] OF: overlay: find target, node: /fragment@0, path '/' not found
[   20.479941] OF: overlay: init_overlay_changeset() failed, ret = -22
[   20.480160]     # clk_register_clk_parent_data_of_test: ASSERTION FAILED at drivers/clk/clk_test.c:2760
[   20.480160]     Expected 0 == ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }), but
[   20.480160]         ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }) == -22 (0xffffffffffffffea)
[   20.481513]         not ok 3 clk_parent_data_of_name_test
[   20.483711] OF: overlay: find target, node: /fragment@0, path '/' not found
[   20.483878] OF: overlay: init_overlay_changeset() failed, ret = -22
[   20.484100]     # clk_register_clk_parent_data_of_test: ASSERTION FAILED at drivers/clk/clk_test.c:2760
[   20.484100]     Expected 0 == ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }), but
[   20.484100]         ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }) == -22 (0xffffffffffffffea)
[   20.485444]         not ok 4 clk_parent_data_of_fwname_name_test
[   20.487432] OF: overlay: find target, node: /fragment@0, path '/' not found
[   20.487600] OF: overlay: init_overlay_changeset() failed, ret = -22
[   20.487841]     # clk_register_clk_parent_data_of_test: ASSERTION FAILED at drivers/clk/clk_test.c:2760
[   20.487841]     Expected 0 == ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }), but
[   20.487841]         ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }) == -22 (0xffffffffffffffea)
[   20.489207]         not ok 5 clk_parent_data_of_index_name_priority_test
[   20.490998] OF: overlay: find target, node: /fragment@0, path '/' not found
[   20.491504] OF: overlay: init_overlay_changeset() failed, ret = -22
[   20.491725]     # clk_register_clk_parent_data_of_test: ASSERTION FAILED at drivers/clk/clk_test.c:2760
[   20.491725]     Expected 0 == ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }), but
[   20.491725]         ({ extern uint8_t __dtbo_kunit_clk_parent_data_test_begin[]; extern uint8_t __dtbo_kunit_clk_parent_data_test_end[]; __of_overlay_apply_kunit((test), __dtbo_kunit_clk_parent_data_test_begin, __dtbo_kunit_clk_parent_data_test_end); }) == -22 (0xffffffffffffffea)
[   20.493053]         not ok 6 clk_parent_data_of_index_fwname_name_priority_test
[   20.493583]     # clk_register_clk_parent_data_of_test: pass:0 fail:6 skip:0 total:6
[   20.493701]     not ok 1 clk_register_clk_parent_data_of_test
[   20.493822] # clk_register_clk_parent_data_of: pass:0 fail:1 skip:0 total:1
[   20.493920] # Totals: pass:0 fail:6 skip:0 total:6
[   20.494032] not ok 49 clk_register_clk_parent_data_of

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

Copying devicetree maintainers.

Quoted 14 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Quoted 8 lines

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

From arch/arm64/kernel/setup.c:setup_arch():

	if (acpi_disabled)
		unflatten_device_tree();		// initializes of_root

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Quoted 38 lines · Guenter Roeck

Copying devicetree maintainers.

Quoted 14 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Quoted 8 lines

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

 From arch/arm64/kernel/setup.c:setup_arch():

    if (acpi_disabled)
        unflatten_device_tree();        // initializes of_root

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

Quoted 7 lines

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Would it work if these tests check if acpi_disabled and skip if it isn't disabled? It might be low overhead condition to check from these tests.

acpi_disabled is exported:

arch/arm64/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/loongarch/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/riscv/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/x86/kernel/acpi/boot.c:EXPORT_SYMBOL(acpi_disabled);

Quoted 50 lines · Shuah Khan

Quoted 38 lines · Guenter Roeck

Copying devicetree maintainers.

Quoted 14 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Quoted 8 lines

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

 From arch/arm64/kernel/setup.c:setup_arch():

    if (acpi_disabled)
        unflatten_device_tree();        // initializes of_root

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

Quoted 7 lines

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Would it work if these tests check if acpi_disabled and skip if it isn't disabled? It might be low overhead condition to check from these tests.

Quoted 7 lines

acpi_disabled is exported:

arch/arm64/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/loongarch/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/riscv/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/x86/kernel/acpi/boot.c:EXPORT_SYMBOL(acpi_disabled);

I don't think that would work. Looking through the use of acpi_init, I don't think that of_root is always NULL when acpi_init is false; that just happens to be the case on arm64 when booting through efi. However, even arm64 has the following code.

        if (acpi_disabled)
                psci_dt_init();
        else
                psci_acpi_init();

While psci_dt_init() doesn't set of_root, it does try to do a devicetree match. So there must be some other condition where acpi_disabled is set but of_root is set anyway. I just have not found that code path.

Quoted 64 lines · Guenter Roeck

Quoted 50 lines · Shuah Khan

Quoted 38 lines · Guenter Roeck

Copying devicetree maintainers.

Quoted 14 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Quoted 8 lines

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

 From arch/arm64/kernel/setup.c:setup_arch():

    if (acpi_disabled)
        unflatten_device_tree();        // initializes of_root

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

Quoted 7 lines

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Would it work if these tests check if acpi_disabled and skip if it isn't disabled? It might be low overhead condition to check from these tests.

Quoted 7 lines

acpi_disabled is exported:

arch/arm64/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/loongarch/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/riscv/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/x86/kernel/acpi/boot.c:EXPORT_SYMBOL(acpi_disabled);

I don't think that would work. Looking through the use of acpi_init, I don't think that of_root is always NULL when acpi_init is false; that just happens to be the case on arm64 when booting through efi. However, even arm64 has the following code.

Quoted 9 lines

         if (acpi_disabled)
                 psci_dt_init();
         else
                 psci_acpi_init();

While psci_dt_init() doesn't set of_root, it does try to do a devicetree match. So there must be some other condition where acpi_disabled is set but of_root is set anyway. I just have not found that code path.

I ended up disabling all affected unit tests for arm64. I'll do the same for other architectures if I encounter the problem there as well.

Unfortunately that includes all clock unit tests because the tests requiring devicetree support can not be enabled/disabled separately, but that can't be helped and is still better than "mandatory" failures.

Quoted 75 lines · Guenter Roeck

Quoted 73 lines · Guenter Roeck

Quoted 50 lines · Shuah Khan

Quoted 38 lines · Guenter Roeck

Copying devicetree maintainers.

Quoted 14 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Quoted 8 lines

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

 From arch/arm64/kernel/setup.c:setup_arch():

    if (acpi_disabled)
        unflatten_device_tree();        // initializes of_root

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

Quoted 7 lines

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Would it work if these tests check if acpi_disabled and skip if it isn't disabled? It might be low overhead condition to check from these tests.

Quoted 7 lines

acpi_disabled is exported:

arch/arm64/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/loongarch/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/riscv/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/x86/kernel/acpi/boot.c:EXPORT_SYMBOL(acpi_disabled);

I don't think that would work. Looking through the use of acpi_init, I don't think that of_root is always NULL when acpi_init is false; that just happens to be the case on arm64 when booting through efi. However, even arm64 has the following code.

         if (acpi_disabled)
                 psci_dt_init();
         else
                 psci_acpi_init();

While psci_dt_init() doesn't set of_root, it does try to do a devicetree match. So there must be some other condition where acpi_disabled is set but of_root is set anyway. I just have not found that code path.

Quoted 7 lines

I ended up disabling all affected unit tests for arm64. I'll do the same for other architectures if I encounter the problem there as well.

Unfortunately that includes all clock unit tests because the tests requiring devicetree support can not be enabled/disabled separately, but that can't be helped and is still better than "mandatory" failures.

Quoted 76 lines · Shuah Khan

Quoted 75 lines · Guenter Roeck

Quoted 73 lines · Guenter Roeck

Quoted 50 lines · Shuah Khan

Quoted 38 lines · Guenter Roeck

Copying devicetree maintainers.

Quoted 14 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

Quoted 8 lines

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

 From arch/arm64/kernel/setup.c:setup_arch():

    if (acpi_disabled)
        unflatten_device_tree();        // initializes of_root

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

Quoted 7 lines

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Would it work if these tests check if acpi_disabled and skip if it isn't disabled? It might be low overhead condition to check from these tests.

Quoted 7 lines

acpi_disabled is exported:

arch/arm64/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/loongarch/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/riscv/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/x86/kernel/acpi/boot.c:EXPORT_SYMBOL(acpi_disabled);

I don't think that would work. Looking through the use of acpi_init, I don't think that of_root is always NULL when acpi_init is false; that just happens to be the case on arm64 when booting through efi. However, even arm64 has the following code.

         if (acpi_disabled)
                 psci_dt_init();
         else
                 psci_acpi_init();

While psci_dt_init() doesn't set of_root, it does try to do a devicetree match. So there must be some other condition where acpi_disabled is set but of_root is set anyway. I just have not found that code path.

Quoted 8 lines

Quoted 7 lines

I ended up disabling all affected unit tests for arm64. I'll do the same for other architectures if I encounter the problem there as well.

Unfortunately that includes all clock unit tests because the tests requiring devicetree support can not be enabled/disabled separately, but that can't be helped and is still better than "mandatory" failures.

of_root is set in drivers/of/pdt.c when it creates the root node. This could be a definitive test for kunit tests that depend on devicetree support.

It is an exported symbol. drivers/of/base.c exports it.

Quoted 31 lines · Shuah Khan

Quoted 30 lines · Guenter Roeck

Quoted 29 lines · Guenter Roeck

Quoted 28 lines · Shuah Khan

Quoted 27 lines · Guenter Roeck

Quoted 26 lines

Copying devicetree maintainers.

Quoted 22 lines · Guenter Roeck

Quoted 7 lines · Guenter Roeck

Hi Stephen,

Quoted 3 lines · Stephen Boyd

Test that clks registered with 'struct clk_parent_data' work as intended and can find their parents.

Quoted 6 lines

When testing this on arm64, I see the error below. The error is only seen if I boot through efi, i.e., with "-bios QEMU_EFI-aarch64.fd" qemu parameter.

Any idea what might cause the problem ?

I noticed that the new overlay tests fail as well, also with "path '/' not found".

[Maybe] answering my own question: I think the problem may be that there is no devicetree file and thus no devicetree root when booting through efi (in other words, of_root is NULL). Would it make sense to skip the tests in that case ?

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

The problem is that of_root is not initialized in arm64 boots if ACPI is enabled.

 From arch/arm64/kernel/setup.c:setup_arch():

    if (acpi_disabled)
        unflatten_device_tree();        // initializes of_root

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

ACPI is enabled if the system boots from EFI. This also affects CONFIG_OF_KUNIT_TEST, which explicitly checks if of_root exists and fails the test if it doesn't.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

I think those tests need to add a check for this condition, or affected machines won't be able to run those unit tests. The obvious solution would be to check if of_root is set, but then the associated test case in CONFIG_OF_KUNIT_TEST would not make sense.

Any suggestions ?

Quoted 36 lines

Quoted 27 lines

Quoted 27 lines

Quoted 26 lines

Quoted 11 lines

Would it work if these tests check if acpi_disabled and skip if it isn't disabled? It might be low overhead condition to check from these tests.

acpi_disabled is exported:

arch/arm64/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/loongarch/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/riscv/kernel/acpi.c:EXPORT_SYMBOL(acpi_disabled); arch/x86/kernel/acpi/boot.c:EXPORT_SYMBOL(acpi_disabled);

I don't think that would work. Looking through the use of acpi_init, I don't think that of_root is always NULL when acpi_init is false; that just happens to be the case on arm64 when booting through efi. However, even arm64 has the following code.

         if (acpi_disabled)
                 psci_dt_init();
         else
                 psci_acpi_init();

While psci_dt_init() doesn't set of_root, it does try to do a devicetree match. So there must be some other condition where acpi_disabled is set but of_root is set anyway. I just have not found that code path.

Quoted 8 lines

Quoted 7 lines

I ended up disabling all affected unit tests for arm64. I'll do the same for other architectures if I encounter the problem there as well.

Unfortunately that includes all clock unit tests because the tests requiring devicetree support can not be enabled/disabled separately, but that can't be helped and is still better than "mandatory" failures.

Quoted 4 lines

of_root is set in drivers/of/pdt.c when it creates the root node. This could be a definitive test for kunit tests that depend on devicetree support.

That is not always the case, including arm64. It is primarily set in unflatten_devicetree(), which is not called on arm64 unless acpi_is disabled (see above).

Quoted 2 lines

It is an exported symbol. drivers/of/base.c exports it.

Yes, checking if of_root is NULL and skipping the test in that case might help, but then there is the of_dtb_root_node_populates_of_root unit test which explicitly fails if of_root is NULL. The comment describing the test is

/*
* Test that the 'of_root' global variable is always populated when DT code is
* enabled. Remove this test once of_root is removed from global access. */

The devicetree unit test code explicitly assumes that of_root is set if CONFIG_OF_EARLY_FLATTREE is enabled, but that is not always the case (again, of_root is NULL on arm64 unless acpi is disabled).

That DT test has been there for a few releases. Is this the first time those tests have been run on arm64+acpi? I didn't try after sending the patches and forgot that the patch was dropped.

How are you running kunit tests? I installed the qemu-efi-aarch64 debian package to get QEMU_EFI.fd but passing that to the kunit.py run command with --qemu_args="-bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd" didn't get me beyond the point that the EFI stub boots linux. I think the serial console must not be working and thus the kunit wrapper waits for something to show up but nothing ever does. I haven't dug any further though, so maybe you have a working command.

Here's my command that isn't working:

./tools/testing/kunit/kunit.py run --arch=arm64 --kunitconfig=drivers/of --qemu_args="-bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd"

[ ... ]

Quoted 4 lines

That DT test has been there for a few releases. Is this the first time those tests have been run on arm64+acpi? I didn't try after sending the patches and forgot that the patch was dropped.

Previously I had the affected tests disabled and never tracked down the problem. Since the problem is now spreading to additional tests, I finally tracked it down, that is all.

Quoted 9 lines

How are you running kunit tests? I installed the qemu-efi-aarch64 debian package to get QEMU_EFI.fd but passing that to the kunit.py run command with --qemu_args="-bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd" didn't get me beyond the point that the EFI stub boots linux. I think the serial console must not be working and thus the kunit wrapper waits for something to show up but nothing ever does. I haven't dug any further though, so maybe you have a working command.

I run all tests during boot, not from the command line. I also use the -bios command but don't recall any issues with the console. I specify the console on the qemu command line; depending on the qemu machine it is either ttyS0 or ttyAMA0. The init script then finds and selects the active console.

Quoted 5 lines

Here's my command that isn't working:

./tools/testing/kunit/kunit.py run --arch=arm64 --kunitconfig=drivers/of --qemu_args="-bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd"

I can't really see what that command is actually doing ;-).

I'll just keep the affected tests disabled on arm64 for the time being.

Quoting Guenter Roeck (2024-10-03 17:25:37)

Quoted 10 lines · Stephen Boyd

[ ... ]

Quoted 4 lines

That DT test has been there for a few releases. Is this the first time those tests have been run on arm64+acpi? I didn't try after sending the patches and forgot that the patch was dropped.

Previously I had the affected tests disabled and never tracked down the problem. Since the problem is now spreading to additional tests, I finally tracked it down, that is all.

Ok great. Good to know this isn't a new problem. Thanks for tracking it down.

Quoted 10 lines

Quoted 9 lines

How are you running kunit tests? I installed the qemu-efi-aarch64 debian package to get QEMU_EFI.fd but passing that to the kunit.py run command with --qemu_args="-bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd" didn't get me beyond the point that the EFI stub boots linux. I think the serial console must not be working and thus the kunit wrapper waits for something to show up but nothing ever does. I haven't dug any further though, so maybe you have a working command.

Quoted 4 lines

I run all tests during boot, not from the command line. I also use the -bios command but don't recall any issues with the console. I specify the console on the qemu command line; depending on the qemu machine it is either ttyS0 or ttyAMA0. The init script then finds and selects the active console.

Can you please describe how you run the kunit test? And provide the qemu command you run to boot arm64 with acpi?

Quoted 2 lines

I'll just keep the affected tests disabled on arm64 for the time being.

We should skip the tests on arm64+acpi, which is similar to disabling but not exactly the same. There will likely be more DT overlay usage in kunit and so that will lead to more test disabling. Skipping properly is the better solution.

Quoted 4 lines

Can you please describe how you run the kunit test? And provide the qemu command you run to boot arm64 with acpi?

Example command line:

qemu-system-aarch64 -M virt -m 512 \
      -kernel arch/arm64/boot/Image -no-reboot -nographic \
      -snapshot \
      -bios /opt/buildbot/rootfs/arm64/../firmware/QEMU_EFI-aarch64.fd \
      -device virtio-blk-device,drive=d0 \
      -drive file=rootfs.ext2,if=none,id=d0,format=raw \
      -cpu cortex-a57 -serial stdio -monitor none -no-reboot \
      --append "kunit.stats_enabled=2 kunit.filter=speed>slow root=/dev/vda rootwait earlycon=pl011,0x9000000 console=ttyAMA0"

That works fine for me. Configuration is arm64 defconfig plus various debug and kunit options. I built the efi image myself from sources. The root file system is from buildroot with modified init script. kunit tests are all built into the kernel and run during boot.

Quoting Guenter Roeck (2024-10-03 21:52:09)

Quoted 17 lines · Stephen Boyd

Quoted 4 lines

Can you please describe how you run the kunit test? And provide the qemu command you run to boot arm64 with acpi?

Example command line:

qemu-system-aarch64 -M virt -m 512 \
       -kernel arch/arm64/boot/Image -no-reboot -nographic \
       -snapshot \
       -bios /opt/buildbot/rootfs/arm64/../firmware/QEMU_EFI-aarch64.fd \
       -device virtio-blk-device,drive=d0 \
       -drive file=rootfs.ext2,if=none,id=d0,format=raw \
       -cpu cortex-a57 -serial stdio -monitor none -no-reboot \
       --append "kunit.stats_enabled=2 kunit.filter=speed>slow root=/dev/vda rootwait earlycon=pl011,0x9000000 console=ttyAMA0"

Quoted 4 lines

That works fine for me. Configuration is arm64 defconfig plus various debug and kunit options. I built the efi image myself from sources. The root file system is from buildroot with modified init script. kunit tests are all built into the kernel and run during boot.

Thanks. I figured out that I was missing enabling CONFIG_ACPI. Here's my commandline

./tools/testing/kunit/kunit.py run --arch=arm64 \
	--kunitconfig=drivers/of \
	--qemu_args="-bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd -smp 2" \
	--kconfig_add="CONFIG_ACPI=y" \
	--kernel_args="earlycon=pl011,0x9000000"

Now I can boot and reproduce the failure, but there's another problem. ACPI disables itself when it fails to find tables.

ACPI: Unable to load the System Description Tables

This calls disable_acpi() which sets acpi_disabled to 1. This happens before the unit test runs, meaning we can't reliably use 'acpi_disabled' as a method to skip.

The best I can come up with then is to test for a NULL of_root when CONFIG_ARM64 and CONFIG_ACPI are enabled, because the tests intentionally don't work when both those configs are enabled and the 'of_root' isn't populated. In all other cases the 'of_root' missing is a bug. I'll probably make this into some sort of kunit helper function in of_private.h and send it to DT maintainers.

Quoted 7 lines

The best I can come up with then is to test for a NULL of_root when CONFIG_ARM64 and CONFIG_ACPI are enabled, because the tests intentionally don't work when both those configs are enabled and the 'of_root' isn't populated. In all other cases the 'of_root' missing is a bug. I'll probably make this into some sort of kunit helper function in of_private.h and send it to DT maintainers.

Sounds good. Thanks a lot for tracking this down.

That makes me wonder though why only arm64 has that restriction. Both riscv and loongarch have ACPI enabled in their defconfig files but call unflatten_device_tree() unconditionally.

Oh well ...