| 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
| ||
|
Message-ID: <fee2a99b-794d-76b5-1a26-131349442130@xilinx.com>
Date: Mon, 10 Jan 2022 17:14:20 -0800
From: Lizhi Hou <lizhi.hou@...inx.com>
To: <robh@...nel.org>, Tom Rix <trix@...hat.com>,
Lizhi Hou <lizhi.hou@...inx.com>,
<linux-kernel@...r.kernel.org>
CC: <linux-fpga@...r.kernel.org>, <maxz@...inx.com>,
<sonal.santan@...inx.com>, <yliu@...inx.com>,
<michal.simek@...inx.com>, <stefanos@...inx.com>,
<devicetree@...r.kernel.org>, <mdf@...nel.org>,
<dwmw2@...radead.org>, Max Zhen <max.zhen@...inx.com>
Subject: Re: [PATCH V4 XRT Alveo Infrastructure 3/5] of: create empty of root
Hi Rob,
We have modified the required device tree change according to your
latest comments.
https://lore.kernel.org/lkml/YaWE%2F2ikgpXi2hzY@robh.at.kernel.org/
https://lore.kernel.org/lkml/YaWFksVvfQQWqKcG@robh.at.kernel.org/
Waiting for your feedback on the design and code changes.
Tom, thanks for your suggestions. Will incorporate your feedback in the
next patch series.
Thanks,
Lizhi
On 1/9/22 10:39 AM, Tom Rix wrote:
>
> On 1/5/22 2:50 PM, Lizhi Hou wrote:
>> When OF_FLATTREE is selected and there is not a device tree, create an
>> empty device tree root node. of/unittest.c code is referenced.
>
> Looks like this patch is doing two things, creating the empty node and
> refactoring the unit tests.
>
> This patch should be split.
>
> It is not clear why the unit test should be refactored.
>
> Tom
>
>>
>> Signed-off-by: Sonal Santan <sonal.santan@...inx.com>
>> Signed-off-by: Max Zhen <max.zhen@...inx.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@...inx.com>
>> ---
>> drivers/of/Makefile | 5 +++
>> drivers/of/fdt.c | 90 ++++++++++++++++++++++++++++++++++++++
>> drivers/of/fdt_default.dts | 5 +++
>> drivers/of/of_private.h | 17 +++++++
>> drivers/of/unittest.c | 72 ++----------------------------
>> 5 files changed, 120 insertions(+), 69 deletions(-)
>> create mode 100644 drivers/of/fdt_default.dts
>>
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index c13b982084a3..a2989055c578 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -1,5 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>> obj-y = base.o device.o platform.o property.o
>> +
> extra lf, remove
>> obj-$(CONFIG_OF_KOBJ) += kobj.o
>> obj-$(CONFIG_OF_DYNAMIC) += dynamic.o
>> obj-$(CONFIG_OF_FLATTREE) += fdt.o
>> @@ -20,4 +21,8 @@ obj-y += kexec.o
>> endif
>> endif
>>
>> +ifndef CONFIG_OF_UNITTEST
>> +obj-$(CONFIG_OF_FLATTREE) += fdt_default.dtb.o
>> +endif
>> +
>> obj-$(CONFIG_OF_UNITTEST) += unittest-data/
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 4546572af24b..66ef9ac97829 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -466,6 +466,96 @@ void *of_fdt_unflatten_tree(const unsigned long
>> *blob,
>> }
>> EXPORT_SYMBOL_GPL(of_fdt_unflatten_tree);
>>
>> +static int __init of_fdt_root_init(void)
>> +{
>> + struct device_node *dt = NULL, *np;
>> + void *fdt = NULL, *fdt_aligned;
>> + struct property *prop = NULL;
>> + __be32 *val = NULL;
>> + int size, rc = 0;
>> +
>> +#if !defined(CONFIG_OF_UNITTEST)
>> + if (of_root)
>> + return 0;
>> +#endif
>> + size = __dtb_fdt_default_end - __dtb_fdt_default_begin;
>> +
>> + fdt = kmalloc(size + FDT_ALIGN_SIZE, GFP_KERNEL);
>> + if (!fdt)
>> + return -ENOMEM;
>> +
>> + fdt_aligned = PTR_ALIGN(fdt, FDT_ALIGN_SIZE);
>> + memcpy(fdt_aligned, __dtb_fdt_default_begin, size);
>> +
>> + if (!of_fdt_unflatten_tree((const unsigned long *)fdt_aligned,
>> + NULL, &dt)) {
>> + pr_warn("%s: unflatten default tree failed\n", __func__);
>> + kfree(fdt);
>> + return -ENODATA;
>> + }
>> + if (!dt) {
>> + pr_warn("%s: empty default tree\n", __func__);
>> + kfree(fdt);
>> + return -ENODATA;
>> + }
>> +
>> + /*
>> + * This lock normally encloses of_resolve_phandles()
>> + */
>> + of_overlay_mutex_lock();
>> +
>> + rc = of_resolve_phandles(dt);
>> + if (rc) {
>> + pr_err("%s: Failed to resolve phandles (rc=%i)\n",
>> __func__, rc);
>> + goto failed;
>> + }
>> +
>> + if (!of_root) {
>> + prop = kcalloc(2, sizeof(*prop), GFP_KERNEL);
>> + if (!prop) {
>> + rc = -ENOMEM;
>> + goto failed;
>> + }
>> + val = kzalloc(sizeof(*val), GFP_KERNEL);
>> + if (!val) {
>> + rc = -ENOMEM;
>> + goto failed;
>> + }
>> + *val = cpu_to_be32(sizeof(void *) / sizeof(u32));
>> +
>> + prop->name = "#address-cells";
>> + prop->value = val;
>> + prop->length = sizeof(u32);
>> + of_add_property(dt, prop);
>> + prop++;
>> + prop->name = "#size-cells";
>> + prop->value = val;
>> + prop->length = sizeof(u32);
>> + of_add_property(dt, prop);
>> + of_root = dt;
>> + for_each_of_allnodes(np)
>> + __of_attach_node_sysfs(np);
>> + of_aliases = of_find_node_by_path("/aliases");
>> + of_chosen = of_find_node_by_path("/chosen");
>> + of_overlay_mutex_unlock();
>> +pr_info("OF ROOT FLAG %lx\n", of_root->_flags);
>> + return 0;
>> + }
>> +
>> + unittest_data_add(dt);
>> +
>> + of_overlay_mutex_unlock();
>> +
>> + return 0;
>> +
>> +failed:
>> + of_overlay_mutex_unlock();
>> + kfree(val);
>> + kfree(prop);
>> + return rc;
>> +}
>> +pure_initcall(of_fdt_root_init);
>> +
>> /* Everything below here references initial_boot_params directly. */
>> int __initdata dt_root_addr_cells;
>> int __initdata dt_root_size_cells;
>> diff --git a/drivers/of/fdt_default.dts b/drivers/of/fdt_default.dts
>> new file mode 100644
>> index 000000000000..d1f12a76dfc6
>> --- /dev/null
>> +++ b/drivers/of/fdt_default.dts
>> @@ -0,0 +1,5 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/dts-v1/;
>> +
>> +/ {
>> +};
>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>> index 631489f7f8c0..1ef93bccfdba 100644
>> --- a/drivers/of/of_private.h
>> +++ b/drivers/of/of_private.h
>> @@ -41,6 +41,18 @@ extern struct mutex of_mutex;
>> extern struct list_head aliases_lookup;
>> extern struct kset *of_kset;
>>
>> +#if defined(CONFIG_OF_UNITTEST)
>> +extern u8 __dtb_testcases_begin[];
>> +extern u8 __dtb_testcases_end[];
>> +#define __dtb_fdt_default_begin __dtb_testcases_begin
>> +#define __dtb_fdt_default_end __dtb_testcases_end
>> +void __init unittest_data_add(struct device_node *dt);
>> +#else
>> +extern u8 __dtb_fdt_default_begin[];
>> +extern u8 __dtb_fdt_default_end[];
>> +static inline void unittest_data_add(struct device_node *dt) {}
>> +#endif
>> +
>> #if defined(CONFIG_OF_DYNAMIC)
>> extern int of_property_notify(int action, struct device_node *np,
>> struct property *prop, struct property
>> *old_prop);
>> @@ -84,6 +96,11 @@ static inline void __of_detach_node_sysfs(struct
>> device_node *np) {}
>>
>> #if defined(CONFIG_OF_RESOLVE)
>> int of_resolve_phandles(struct device_node *tree);
>> +#else
>> +static inline int of_resolve_phandles(struct device_node *tree)
>> +{
>> + return 0;
>> +}
>> #endif
>>
>> void __of_phandle_cache_inv_entry(phandle handle);
>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>> index 8c056972a6dd..745f455235cc 100644
>> --- a/drivers/of/unittest.c
>> +++ b/drivers/of/unittest.c
>> @@ -1402,73 +1402,15 @@ static void attach_node_and_children(struct
>> device_node *np)
>> * unittest_data_add - Reads, copies data from
>> * linked tree and attaches it to the live tree
>> */
>> -static int __init unittest_data_add(void)
>> +void __init unittest_data_add(struct device_node *dt)
>> {
>> - void *unittest_data;
>> - void *unittest_data_align;
>> - struct device_node *unittest_data_node = NULL, *np;
>> - /*
>> - * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically
>> - * created by cmd_dt_S_dtb in scripts/Makefile.lib
>> - */
>> - extern uint8_t __dtb_testcases_begin[];
>> - extern uint8_t __dtb_testcases_end[];
>> - const int size = __dtb_testcases_end - __dtb_testcases_begin;
>> - int rc;
>> - void *ret;
>> -
>> - if (!size) {
>> - pr_warn("%s: testcases is empty\n", __func__);
>> - return -ENODATA;
>> - }
>> -
>> - /* creating copy */
>> - unittest_data = kmalloc(size + FDT_ALIGN_SIZE, GFP_KERNEL);
>> - if (!unittest_data)
>> - return -ENOMEM;
>> -
>> - unittest_data_align = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
>> - memcpy(unittest_data_align, __dtb_testcases_begin, size);
>> -
>> - ret = of_fdt_unflatten_tree(unittest_data_align, NULL,
>> &unittest_data_node);
>> - if (!ret) {
>> - pr_warn("%s: unflatten testcases tree failed\n",
>> __func__);
>> - kfree(unittest_data);
>> - return -ENODATA;
>> - }
>> - if (!unittest_data_node) {
>> - pr_warn("%s: testcases tree is empty\n", __func__);
>> - kfree(unittest_data);
>> - return -ENODATA;
>> - }
>> -
>> - /*
>> - * This lock normally encloses of_resolve_phandles()
>> - */
>> - of_overlay_mutex_lock();
>> -
>> - rc = of_resolve_phandles(unittest_data_node);
>> - if (rc) {
>> - pr_err("%s: Failed to resolve phandles (rc=%i)\n",
>> __func__, rc);
>> - of_overlay_mutex_unlock();
>> - return -EINVAL;
>> - }
>> -
>> - if (!of_root) {
>> - of_root = unittest_data_node;
>> - for_each_of_allnodes(np)
>> - __of_attach_node_sysfs(np);
>> - of_aliases = of_find_node_by_path("/aliases");
>> - of_chosen = of_find_node_by_path("/chosen");
>> - of_overlay_mutex_unlock();
>> - return 0;
>> - }
>> + struct device_node *np;
>>
>> EXPECT_BEGIN(KERN_INFO,
>> "Duplicate name in testcase-data, renamed to
>> \"duplicate-name#1\"");
>>
>> /* attach the sub-tree to live tree */
>> - np = unittest_data_node->child;
>> + np = dt->child;
>> while (np) {
>> struct device_node *next = np->sibling;
>>
>> @@ -1479,10 +1421,6 @@ static int __init unittest_data_add(void)
>>
>> EXPECT_END(KERN_INFO,
>> "Duplicate name in testcase-data, renamed to
>> \"duplicate-name#1\"");
>> -
>> - of_overlay_mutex_unlock();
>> -
>> - return 0;
>> }
>>
>> #ifdef CONFIG_OF_OVERLAY
>> @@ -3258,7 +3196,6 @@ static inline __init void
>> of_unittest_overlay_high_level(void) {}
>> static int __init of_unittest(void)
>> {
>> struct device_node *np;
>> - int res;
>>
>> pr_info("start of unittest - you will see error messages\n");
>>
>> @@ -3267,9 +3204,6 @@ static int __init of_unittest(void)
>> if (IS_ENABLED(CONFIG_UML))
>> unittest_unflatten_overlay_base();
>>
>> - res = unittest_data_add();
>> - if (res)
>> - return res;
>> if (!of_aliases)
>> of_aliases = of_find_node_by_path("/aliases");
>>
>
Powered by blists - more mailing lists