Re: [PATCH 2/8] of: Enable DTB loading on UML for KUnit tests

[Rob Herring]

On Wed, Mar 01, 2023 at 05:38:15PM -0800, Stephen Boyd wrote:
> To fully exercise common clk framework code in KUnit we need to
> associate 'struct device' pointers with 'struct device_node' pointers so
> that things like clk_get() can parse DT nodes for 'clocks' and so that
> clk providers can use DT to provide clks; the most common mode of
> operation for clk providers.
> 
> Adding support to KUnit so that it loads a DTB is fairly simple after
> commit b31297f04e86 ("um: Add devicetree support"). We can simply pass a
> pre-compiled deviectree blob (DTB) on the kunit.py commandline and UML
> will load it. The problem is that tests won't know that the commandline
> has been modified, nor that a DTB has been loaded. Take a different
> approach so that tests can skip if a DTB hasn't been loaded.
> 
> Reuse the Makefile logic from the OF unittests to build a DTB into the
> kernel. This DTB will be for the mythical machine "linux,kunit", i.e.
> the devicetree for the KUnit "board". In practice, it is a dtsi file
> that will gather includes for kunit tests that rely in part on a
> devicetree being loaded. The devicetree should only be loaded if
> CONFIG_OF_KUNIT=y. Make that a choice config parallel to the existing
> CONFIG_OF_UNITTEST so that only one devicetree can be loaded in the
> system at a time. Similarly, the kernel commandline option to load a
> DTB is ignored if CONFIG_OF_KUNIT is enabled so that only one DTB is
> loaded at a time.
> 
> Add a simple unit test to confirm that the DTB loading worked. Future
> tests will add to the kunit.dtsi file to include their specific test
> nodes.
> 
> Cc: Richard Weinberger <richard@xxxxxx>
> Cc: Anton Ivanov <anton.ivanov@xxxxxxxxxxxxxxxxxx>
> Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> Cc: Vincent Whitchurch <vincent.whitchurch@xxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> Cc: Frank Rowand <frowand.list@xxxxxxxxx>
> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxx>
> ---
>  arch/um/kernel/dtb.c            | 29 +++++++++++++++--
>  drivers/of/Kconfig              | 26 ++++++++++++++++
>  drivers/of/Makefile             |  1 +
>  drivers/of/kunit/.kunitconfig   |  4 +++
>  drivers/of/kunit/Makefile       |  4 +++
>  drivers/of/kunit/kunit.dtsi     |  8 +++++
>  drivers/of/kunit/kunit.dtso     |  4 +++
>  drivers/of/kunit/uml_dtb_test.c | 55 +++++++++++++++++++++++++++++++++
>  8 files changed, 128 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/of/kunit/.kunitconfig
>  create mode 100644 drivers/of/kunit/Makefile
>  create mode 100644 drivers/of/kunit/kunit.dtsi
>  create mode 100644 drivers/of/kunit/kunit.dtso
>  create mode 100644 drivers/of/kunit/uml_dtb_test.c
> 
> diff --git a/arch/um/kernel/dtb.c b/arch/um/kernel/dtb.c
> index 484141b06938..ee63951b12df 100644
> --- a/arch/um/kernel/dtb.c
> +++ b/arch/um/kernel/dtb.c
> @@ -15,9 +15,32 @@ void uml_dtb_init(void)
>  	long long size;
>  	void *area;
>  
> -	area = uml_load_file(dtb, &size);
> -	if (!area)
> -		return;
> +	if (IS_ENABLED(CONFIG_OF_KUNIT)) {
> +		/*
> +		 * __dtbo_kunit_begin[] and __dtbo_kunit_end[] are magically
> +		 * created by cmd_dt_S_dtbo in scripts/Makefile.lib from the
> +		 * drivers/of/kunit/kunit.dtsi file.
> +		 */
> +		extern uint8_t __dtbo_kunit_begin[];
> +		extern uint8_t __dtbo_kunit_end[];
> +
> +		size = __dtbo_kunit_end - __dtbo_kunit_begin;
> +		if (!size) {
> +			pr_warn("%s: kunit testcases is empty\n", __func__);
> +			return;
> +		}
> +
> +		/* creating copy */
> +		area = memblock_alloc(size, 8);
> +		if (!area)
> +			return;
> +
> +		memcpy(area, __dtbo_kunit_begin, size);
> +	} else {
> +		area = uml_load_file(dtb, &size);
> +		if (!area)
> +			return;
> +	}
>  
>  	if (!early_init_dt_scan(area)) {
>  		pr_err("invalid DTB %s\n", dtb);
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 80b5fd44ab1c..1f968b6a3dde 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -12,6 +12,20 @@ menuconfig OF
>  
>  if OF
>  
> +choice

No. This needs to be reworked such that a kernel rebuild is not needed 
to run different tests. I suspect that the overlay approach will do that 
for you.

> +	prompt "Devicetree Runtime Tests"
> +	default OF_UNITTEST
> +
> +config OF_KUNIT
> +	bool "Devicetree KUnit support" if KUNIT
> +	depends on UML

This is not a great dependency either...

Rob