linux-kernel - Re: [PATCH 2/2] of: Add unit tests for applying overlays.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 24 Apr 2017 15:40:36 -0500
From:   Rob Herring <robh+dt@...nel.org>
To:     Frank Rowand <frowand.list@...il.com>
Cc:     Stephen Boyd <stephen.boyd@...aro.org>,
        Michal Marek <mmarek@...e.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH 2/2] of: Add unit tests for applying overlays.

On Mon, Apr 24, 2017 at 12:47 PM, Frank Rowand <frowand.list@...il.com> wrote:
> On 04/24/17 10:16, Rob Herring wrote:
>> On Mon, Apr 24, 2017 at 12:43 AM,  <frowand.list@...il.com> wrote:
>>> From: Frank Rowand <frank.rowand@...y.com>
>>>
>>> Existing overlay unit tests examine individual pieces of the overlay
>>> code.  The new tests target the entire process of applying an overlay.
>>>
>>> Signed-off-by: Frank Rowand <frank.rowand@...y.com>

[...]

>>> @@ -1256,11 +1258,54 @@ bool __init early_init_dt_scan(void *params)
>>>   */
>>>  void __init unflatten_device_tree(void)
>>>  {
>>> +#ifdef CONFIG_OF_UNITTEST
>>> +       extern uint8_t __dtb_ot_base_begin[];
>>> +       extern uint8_t __dtb_ot_base_end[];
>>> +       struct device_node *ot_base_root;
>>> +       void *ot_base;
>>> +       u32 data_size;
>>> +       u32 size;
>>> +#endif
>>> +
>>>         __unflatten_device_tree(initial_boot_params, NULL, &of_root,
>>>                                 early_init_dt_alloc_memory_arch, false);
>>>
>>>         /* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
>>>         of_alias_scan(early_init_dt_alloc_memory_arch);
>>
>> Just make __unflatten_device_tree accessible to the unit test code and
>> move all this to it. Then you don't need the ifdefery.
>
> Good idea.  I'll do that.
>
>
>> Does this need to be immediately after unflattening the base tree?
>
> My goal is to make the creation of the test data in the tree follow
> the normal process as much as possible, so that real code is tested
> instead of testing test code.
>
> This flattened device tree contains the base information that the
> test overlays are applied against.

Okay. If you need it here, then you can put this all into a unittest
function and call it from here.


>>> +#ifdef CONFIG_OF_OVERLAY
>>> +/*
>>> + * The purpose of of_unittest_overlay_test_data_add is to add an
>>> + * overlay in the normal fashion.  This is a test of the whole
>>> + * picture, instead of testing individual elements.
>>> + *
>>> + * A secondary purpose is to be able to verify that the contents of
>>> + * /proc/device-tree/ contains the updated structure and values from
>>> + * the overlay.  That must be verified separately in user space.
>>> + *
>>> + * Return 0 on unexpected error.
>>> + */
>>> +static int __init overlay_test_data_add(int onum)
>>
>> There's a need for a general function to apply built-in overlays
>> beyond just unittests. See
>> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c. It's pretty close to the
>> same set of calls.
>
> Yes, agreed.
>
> My plan in the next release cycle is to first clean up drivers/of/overlay.c.
> No functional changes, just cosmetic such as aligning function names with
> what they actually do.
>
> Then make some (hopefully) minor correctness changes, such as locking
> correctly around phandle adjustments.
>
> Then create the general function to apply built-in overlays and convert
> all (two) separate implementations to use the common function.  I did
> not want to delay adding the unit tests to wait for this step.

Okay. Whatever order you want to do it is fine.

Rob