[PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[frowand.list]

From: Frank Rowand <frank.rowand@sony.com>

The Devicetree standard specifies an 8 byte alignment of the FDT.
Code in libfdt expects this alignment for an FDT image in memory.
kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
with kmalloc(), align pointer, memcpy() to get proper alignment.

The 4 byte alignment exposed a related bug which triggered a crash
on openrisc with:
commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
as reported in:
https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Frank Rowand <frank.rowand@sony.com>

---

changes since version 1:
  - use pointer from kmalloc() for kfree() instead of using pointer that
    has been modified for FDT alignment

changes since version 2:
  - version 1 was a work in progress version, I failed to commit the following
    final changes
  - reorder first two arguments of of_overlay_apply()

changes since version 3:
  - size of memory allocation and size of copy after pointer alignment
    differ, use separate variables with correct values for each case
  - edit comment to more clearly describe that ovcs->fdt is the allocated
    memory region, which may be different than where the aligned pointer points
  - remove unused parameter from of_overlay_apply()

 drivers/of/of_private.h |  2 ++
 drivers/of/overlay.c    | 27 +++++++++++++++++----------
 drivers/of/unittest.c   | 13 ++++++++++---
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index d9e6a324de0a..d717efbd637d 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -8,6 +8,8 @@
  * Copyright (C) 1996-2005 Paul Mackerras.
  */
 
+#define FDT_ALIGN_SIZE 8
+
 /**
  * struct alias_prop - Alias property in 'aliases' node
  * @link:	List node to link the structure in aliases_lookup list
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 50bbe0edf538..ecf967c57900 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -57,7 +57,7 @@ struct fragment {
  * struct overlay_changeset
  * @id:			changeset identifier
  * @ovcs_list:		list on which we are located
- * @fdt:		FDT that was unflattened to create @overlay_tree
+ * @fdt:		base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree
  * @overlay_tree:	expanded device tree that contains the fragment nodes
  * @count:		count of fragment structures
  * @fragments:		fragment nodes in the overlay expanded device tree
@@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node *info_node)
 /**
  * init_overlay_changeset() - initialize overlay changeset from overlay tree
  * @ovcs:	Overlay changeset to build
- * @fdt:	the FDT that was unflattened to create @tree
- * @tree:	Contains all the overlay fragments and overlay fixup nodes
+ * @fdt:	base of memory allocated to hold aligned FDT that was unflattened to create @tree
+ * @tree:	Contains the overlay fragments and overlay fixup nodes
  *
  * Initialize @ovcs.  Populate @ovcs->fragments with node information from
  * the top level of @tree.  The relevant top level nodes are the fragment
@@ -873,7 +873,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
  * internal documentation
  *
  * of_overlay_apply() - Create and apply an overlay changeset
- * @fdt:	the FDT that was unflattened to create @tree
+ * @fdt:	base of memory allocated to hold the aligned FDT
  * @tree:	Expanded overlay device tree
  * @ovcs_id:	Pointer to overlay changeset id
  *
@@ -913,7 +913,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
  */
 
 static int of_overlay_apply(const void *fdt, struct device_node *tree,
-		int *ovcs_id)
+			    int *ovcs_id)
 {
 	struct overlay_changeset *ovcs;
 	int ret = 0, ret_revert, ret_tmp;
@@ -953,7 +953,9 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
 	/*
 	 * after overlay_notify(), ovcs->overlay_tree related pointers may have
 	 * leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree;
-	 * and can not free fdt, aka ovcs->fdt
+	 * and can not free memory containing aligned fdt.  The aligned fdt
+	 * is contained within the memory at ovcs->fdt, possibly at an offset
+	 * from ovcs->fdt.
 	 */
 	ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY);
 	if (ret) {
@@ -1014,9 +1016,10 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
 int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 			 int *ovcs_id)
 {
-	const void *new_fdt;
+	void *new_fdt;
+	void *new_fdt_align;
 	int ret;
-	u32 size;
+	u32 size, size_alloc;
 	struct device_node *overlay_root;
 
 	*ovcs_id = 0;
@@ -1036,11 +1039,15 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
 	 * will create pointers to the passed in FDT in the unflattened tree.
 	 */
-	new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
+	size_alloc = size + FDT_ALIGN_SIZE;
+	new_fdt = kmalloc(size_alloc, GFP_KERNEL);
 	if (!new_fdt)
 		return -ENOMEM;
 
-	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
+	new_fdt_align = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
+	memcpy(new_fdt_align, overlay_fdt, size);
+
+	of_fdt_unflatten_tree(new_fdt_align, NULL, &overlay_root);
 	if (!overlay_root) {
 		pr_err("unable to unflatten overlay_fdt\n");
 		ret = -EINVAL;
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index eb100627c186..b43a86cc2068 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
+#include <linux/kernel.h>
 
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
@@ -1408,6 +1409,7 @@ static void attach_node_and_children(struct device_node *np)
 static int __init unittest_data_add(void)
 {
 	void *unittest_data;
+	void *unittest_data_align;
 	struct device_node *unittest_data_node, *np;
 	/*
 	 * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically
@@ -1415,7 +1417,8 @@ static int __init unittest_data_add(void)
 	 */
 	extern uint8_t __dtb_testcases_begin[];
 	extern uint8_t __dtb_testcases_end[];
-	const int size = __dtb_testcases_end - __dtb_testcases_begin;
+	u32 size = __dtb_testcases_end - __dtb_testcases_begin;
+	u32 size_alloc;
 	int rc;
 
 	if (!size) {
@@ -1425,11 +1428,15 @@ static int __init unittest_data_add(void)
 	}
 
 	/* creating copy */
-	unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL);
+	size_alloc = size + FDT_ALIGN_SIZE;
+	unittest_data = kmalloc(size_alloc, GFP_KERNEL);
 	if (!unittest_data)
 		return -ENOMEM;
 
-	of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
+	unittest_data_align = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
+	memcpy(unittest_data_align, __dtb_testcases_begin, size);
+
+	of_fdt_unflatten_tree(unittest_data_align, NULL, &unittest_data_node);
 	if (!unittest_data_node) {
 		pr_warn("%s: No tree to attach; not running tests\n", __func__);
 		kfree(unittest_data);
-- 
Frank Rowand <frank.rowand@sony.com>

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Rob Herring]

On Thu, Apr 8, 2021 at 3:45 PM <frowand.list@gmail.com> wrote:
>
> From: Frank Rowand <frank.rowand@sony.com>
>
> The Devicetree standard specifies an 8 byte alignment of the FDT.
> Code in libfdt expects this alignment for an FDT image in memory.
> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
> with kmalloc(), align pointer, memcpy() to get proper alignment.
>
> The 4 byte alignment exposed a related bug which triggered a crash
> on openrisc with:
> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
> as reported in:
> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/
>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>
> ---
>
> changes since version 1:
>   - use pointer from kmalloc() for kfree() instead of using pointer that
>     has been modified for FDT alignment
>
> changes since version 2:
>   - version 1 was a work in progress version, I failed to commit the following
>     final changes
>   - reorder first two arguments of of_overlay_apply()
>
> changes since version 3:
>   - size of memory allocation and size of copy after pointer alignment
>     differ, use separate variables with correct values for each case
>   - edit comment to more clearly describe that ovcs->fdt is the allocated
>     memory region, which may be different than where the aligned pointer points
>   - remove unused parameter from of_overlay_apply()
>
>  drivers/of/of_private.h |  2 ++
>  drivers/of/overlay.c    | 27 +++++++++++++++++----------
>  drivers/of/unittest.c   | 13 ++++++++++---
>  3 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index d9e6a324de0a..d717efbd637d 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -8,6 +8,8 @@
>   * Copyright (C) 1996-2005 Paul Mackerras.
>   */
>
> +#define FDT_ALIGN_SIZE 8
> +
>  /**
>   * struct alias_prop - Alias property in 'aliases' node
>   * @link:      List node to link the structure in aliases_lookup list
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 50bbe0edf538..ecf967c57900 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -57,7 +57,7 @@ struct fragment {
>   * struct overlay_changeset
>   * @id:                        changeset identifier
>   * @ovcs_list:         list on which we are located
> - * @fdt:               FDT that was unflattened to create @overlay_tree
> + * @fdt:               base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree
>   * @overlay_tree:      expanded device tree that contains the fragment nodes
>   * @count:             count of fragment structures
>   * @fragments:         fragment nodes in the overlay expanded device tree
> @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node *info_node)
>  /**
>   * init_overlay_changeset() - initialize overlay changeset from overlay tree
>   * @ovcs:      Overlay changeset to build
> - * @fdt:       the FDT that was unflattened to create @tree
> - * @tree:      Contains all the overlay fragments and overlay fixup nodes
> + * @fdt:       base of memory allocated to hold aligned FDT that was unflattened to create @tree
> + * @tree:      Contains the overlay fragments and overlay fixup nodes
>   *
>   * Initialize @ovcs.  Populate @ovcs->fragments with node information from
>   * the top level of @tree.  The relevant top level nodes are the fragment
> @@ -873,7 +873,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>   * internal documentation
>   *
>   * of_overlay_apply() - Create and apply an overlay changeset
> - * @fdt:       the FDT that was unflattened to create @tree
> + * @fdt:       base of memory allocated to hold the aligned FDT
>   * @tree:      Expanded overlay device tree
>   * @ovcs_id:   Pointer to overlay changeset id
>   *
> @@ -913,7 +913,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>   */
>
>  static int of_overlay_apply(const void *fdt, struct device_node *tree,
> -               int *ovcs_id)
> +                           int *ovcs_id)
>  {
>         struct overlay_changeset *ovcs;
>         int ret = 0, ret_revert, ret_tmp;
> @@ -953,7 +953,9 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>         /*
>          * after overlay_notify(), ovcs->overlay_tree related pointers may have
>          * leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree;
> -        * and can not free fdt, aka ovcs->fdt
> +        * and can not free memory containing aligned fdt.  The aligned fdt
> +        * is contained within the memory at ovcs->fdt, possibly at an offset
> +        * from ovcs->fdt.
>          */
>         ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY);
>         if (ret) {
> @@ -1014,9 +1016,10 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>                          int *ovcs_id)
>  {
> -       const void *new_fdt;
> +       void *new_fdt;
> +       void *new_fdt_align;
>         int ret;
> -       u32 size;
> +       u32 size, size_alloc;
>         struct device_node *overlay_root;
>
>         *ovcs_id = 0;
> @@ -1036,11 +1039,15 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>          * Must create permanent copy of FDT because of_fdt_unflatten_tree()
>          * will create pointers to the passed in FDT in the unflattened tree.
>          */
> -       new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
> +       size_alloc = size + FDT_ALIGN_SIZE;
> +       new_fdt = kmalloc(size_alloc, GFP_KERNEL);

As size_alloc is only used once, you can just do:

new_fdt = kmalloc(size + FDT_ALIGN_SIZE, GFP_KERNEL);

Same for the unittest. I can fix up.

Applying now so this gets into linux-next this week.

Rob

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 2:28 PM, Rob Herring wrote:
> 
> Applying now so this gets into linux-next this week.
> 
The patch doesn't apply on top of today's -next; it conflicts
with "of: properly check for error returned by fdt_get_name()".

I reverted that patch and applied this one, and the DT unittests
run with it on openrisc. I do get a single test failure, but I that
is a different problem (possibly with the test case itself).

### dt-test ### FAIL of_unittest_dma_ranges_one():923 of_dma_get_range: wrong DMA addr 0x00000000
	(expecting 100000000) on node /testcase-data/address-tests/bus@80000000/device@1000

Tested-by: Guenter Roeck <linux@roeck-us.net>

Guenter

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Frank Rowand]

On 4/8/21 4:28 PM, Rob Herring wrote:
> On Thu, Apr 8, 2021 at 3:45 PM <frowand.list@gmail.com> wrote:
>>
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> The Devicetree standard specifies an 8 byte alignment of the FDT.
>> Code in libfdt expects this alignment for an FDT image in memory.
>> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
>> with kmalloc(), align pointer, memcpy() to get proper alignment.
>>
>> The 4 byte alignment exposed a related bug which triggered a crash
>> on openrisc with:
>> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
>> as reported in:
>> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/
>>
>> Reported-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>>
>> ---
>>
>> changes since version 1:
>>   - use pointer from kmalloc() for kfree() instead of using pointer that
>>     has been modified for FDT alignment
>>
>> changes since version 2:
>>   - version 1 was a work in progress version, I failed to commit the following
>>     final changes
>>   - reorder first two arguments of of_overlay_apply()
>>
>> changes since version 3:
>>   - size of memory allocation and size of copy after pointer alignment
>>     differ, use separate variables with correct values for each case
>>   - edit comment to more clearly describe that ovcs->fdt is the allocated
>>     memory region, which may be different than where the aligned pointer points
>>   - remove unused parameter from of_overlay_apply()
>>
>>  drivers/of/of_private.h |  2 ++
>>  drivers/of/overlay.c    | 27 +++++++++++++++++----------
>>  drivers/of/unittest.c   | 13 ++++++++++---
>>  3 files changed, 29 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>> index d9e6a324de0a..d717efbd637d 100644
>> --- a/drivers/of/of_private.h
>> +++ b/drivers/of/of_private.h
>> @@ -8,6 +8,8 @@
>>   * Copyright (C) 1996-2005 Paul Mackerras.
>>   */
>>
>> +#define FDT_ALIGN_SIZE 8
>> +
>>  /**
>>   * struct alias_prop - Alias property in 'aliases' node
>>   * @link:      List node to link the structure in aliases_lookup list
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 50bbe0edf538..ecf967c57900 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -57,7 +57,7 @@ struct fragment {
>>   * struct overlay_changeset
>>   * @id:                        changeset identifier
>>   * @ovcs_list:         list on which we are located
>> - * @fdt:               FDT that was unflattened to create @overlay_tree
>> + * @fdt:               base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree
>>   * @overlay_tree:      expanded device tree that contains the fragment nodes
>>   * @count:             count of fragment structures
>>   * @fragments:         fragment nodes in the overlay expanded device tree
>> @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node *info_node)
>>  /**
>>   * init_overlay_changeset() - initialize overlay changeset from overlay tree
>>   * @ovcs:      Overlay changeset to build
>> - * @fdt:       the FDT that was unflattened to create @tree
>> - * @tree:      Contains all the overlay fragments and overlay fixup nodes
>> + * @fdt:       base of memory allocated to hold aligned FDT that was unflattened to create @tree
>> + * @tree:      Contains the overlay fragments and overlay fixup nodes
>>   *
>>   * Initialize @ovcs.  Populate @ovcs->fragments with node information from
>>   * the top level of @tree.  The relevant top level nodes are the fragment
>> @@ -873,7 +873,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>>   * internal documentation
>>   *
>>   * of_overlay_apply() - Create and apply an overlay changeset
>> - * @fdt:       the FDT that was unflattened to create @tree
>> + * @fdt:       base of memory allocated to hold the aligned FDT
>>   * @tree:      Expanded overlay device tree
>>   * @ovcs_id:   Pointer to overlay changeset id
>>   *
>> @@ -913,7 +913,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>>   */
>>
>>  static int of_overlay_apply(const void *fdt, struct device_node *tree,
>> -               int *ovcs_id)
>> +                           int *ovcs_id)
>>  {
>>         struct overlay_changeset *ovcs;
>>         int ret = 0, ret_revert, ret_tmp;
>> @@ -953,7 +953,9 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>>         /*
>>          * after overlay_notify(), ovcs->overlay_tree related pointers may have
>>          * leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree;
>> -        * and can not free fdt, aka ovcs->fdt
>> +        * and can not free memory containing aligned fdt.  The aligned fdt
>> +        * is contained within the memory at ovcs->fdt, possibly at an offset
>> +        * from ovcs->fdt.
>>          */
>>         ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY);
>>         if (ret) {
>> @@ -1014,9 +1016,10 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>>  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>>                          int *ovcs_id)
>>  {
>> -       const void *new_fdt;
>> +       void *new_fdt;
>> +       void *new_fdt_align;
>>         int ret;
>> -       u32 size;
>> +       u32 size, size_alloc;
>>         struct device_node *overlay_root;
>>
>>         *ovcs_id = 0;
>> @@ -1036,11 +1039,15 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>>          * Must create permanent copy of FDT because of_fdt_unflatten_tree()
>>          * will create pointers to the passed in FDT in the unflattened tree.
>>          */
>> -       new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
>> +       size_alloc = size + FDT_ALIGN_SIZE;
>> +       new_fdt = kmalloc(size_alloc, GFP_KERNEL);
> 
> As size_alloc is only used once, you can just do:
> 
> new_fdt = kmalloc(size + FDT_ALIGN_SIZE, GFP_KERNEL);
> 
> Same for the unittest. I can fix up.
> 
> Applying now so this gets into linux-next this week.

Thanks, that change looks like an improvement to me.

> 
> Rob
> 

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Frank Rowand]

On 4/8/21 4:54 PM, Guenter Roeck wrote:
> On 4/8/21 2:28 PM, Rob Herring wrote:
>>
>> Applying now so this gets into linux-next this week.
>>
> The patch doesn't apply on top of today's -next; it conflicts
> with "of: properly check for error returned by fdt_get_name()".
> 
> I reverted that patch and applied this one, and the DT unittests
> run with it on openrisc. I do get a single test failure, but I that
> is a different problem (possibly with the test case itself).
> 
> ### dt-test ### FAIL of_unittest_dma_ranges_one():923 of_dma_get_range: wrong DMA addr 0x00000000
> 	(expecting 100000000) on node /testcase-data/address-tests/bus@80000000/device@1000

That is a known regression on the target that I use for testing (and
has been since 5.10-rc1) - the 8074 dragonboard, arm 32.  No
one else has reported it on the list, so even though I want to debug
and fix it "promptly", other tasks have had higher priority.  In my
notes I list two suspect commits:

  e0d072782c73 dma-mapping: introduce DMA range map, supplanting dma_pfn_offset
  0a0f0d8be76d dma-mapping: split <linux/dma-mapping.h>

I think that was purely based on looking at the list of commits that
may have touched OF dma.  I have not done a bisect.

One specific report of not seeing the FAIL was Vireshk on 5.11-rc6 with
a Hikey board.

> 
> Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks for testing!

> 
> Guenter
> 

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Guenter Roeck]

On 4/8/21 3:53 PM, Frank Rowand wrote:
> On 4/8/21 4:54 PM, Guenter Roeck wrote:
>> On 4/8/21 2:28 PM, Rob Herring wrote:
>>>
>>> Applying now so this gets into linux-next this week.
>>>
>> The patch doesn't apply on top of today's -next; it conflicts
>> with "of: properly check for error returned by fdt_get_name()".
>>
>> I reverted that patch and applied this one, and the DT unittests
>> run with it on openrisc. I do get a single test failure, but I that
>> is a different problem (possibly with the test case itself).
>>
>> ### dt-test ### FAIL of_unittest_dma_ranges_one():923 of_dma_get_range: wrong DMA addr 0x00000000
>> 	(expecting 100000000) on node /testcase-data/address-tests/bus@80000000/device@1000
> 
> That is a known regression on the target that I use for testing (and
> has been since 5.10-rc1) - the 8074 dragonboard, arm 32.  No
> one else has reported it on the list, so even though I want to debug
> and fix it "promptly", other tasks have had higher priority.  In my
> notes I list two suspect commits:
> 
>   e0d072782c73 dma-mapping: introduce DMA range map, supplanting dma_pfn_offset
>   0a0f0d8be76d dma-mapping: split <linux/dma-mapping.h>
> 
> I think that was purely based on looking at the list of commits that
> may have touched OF dma.  I have not done a bisect.
> 

Here you are:

# bad: [2c85ebc57b3e1817b6ce1a6b703928e113a90442] Linux 5.10
# good: [bbf5c979011a099af5dc76498918ed7df445635b] Linux 5.9
git bisect start 'v5.10' 'v5.9'
# bad: [4d0e9df5e43dba52d38b251e3b909df8fa1110be] lib, uaccess: add failure injection to usercopy functions
git bisect bad 4d0e9df5e43dba52d38b251e3b909df8fa1110be
# good: [f888bdf9823c85fe945c4eb3ba353f749dec3856] Merge tag 'devicetree-for-5.10' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
git bisect good f888bdf9823c85fe945c4eb3ba353f749dec3856
# good: [640eee067d9aae0bb98d8706001976ff1affaf00] Merge tag 'drm-misc-next-fixes-2020-10-13' of git://anongit.freedesktop.org/drm/drm-misc into drm-next
git bisect good 640eee067d9aae0bb98d8706001976ff1affaf00
# good: [c6dbef7307629cce855aa6b482b60cbf7777ed88] Merge tag 'usb-5.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect good c6dbef7307629cce855aa6b482b60cbf7777ed88
# good: [ce1558c285f9ad04c03b46833a028230771cc0a7] ALSA: hda/hdmi: fix incorrect locking in hdmi_pcm_close
git bisect good ce1558c285f9ad04c03b46833a028230771cc0a7
# good: [c48b75b7271db23c1b2d1204d6e8496d91f27711] Merge tag 'sound-5.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good c48b75b7271db23c1b2d1204d6e8496d91f27711
# bad: [0cd7d9795fa82226e7516d38b474bddae8b1ff26] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching
git bisect bad 0cd7d9795fa82226e7516d38b474bddae8b1ff26
# good: [b1839e7c2a42ccd9a0587c0092e880c7a213ee2a] dmaengine: xilinx: dpdma: convert tasklets to use new tasklet_setup() API
git bisect good b1839e7c2a42ccd9a0587c0092e880c7a213ee2a
# bad: [0de327969b61a245e3a47b60009eae73fe513cef] cma: decrease CMA_ALIGNMENT lower limit to 2
git bisect bad 0de327969b61a245e3a47b60009eae73fe513cef
# good: [6eb0233ec2d0df288fe8515d5b0b2b15562e05bb] usb: don't inherity DMA properties for USB devices
git bisect good 6eb0233ec2d0df288fe8515d5b0b2b15562e05bb
# bad: [48d15814dd0fc429e3205b87f1af6cc472018478] lib82596: move DMA allocation into the callers of i82596_probe
git bisect bad 48d15814dd0fc429e3205b87f1af6cc472018478
# bad: [eba304c6861613a649ba46cfab835b1eddeacd8e] dma-mapping: better document dma_addr_t and DMA_MAPPING_ERROR
git bisect bad eba304c6861613a649ba46cfab835b1eddeacd8e
# bad: [b9bb694b9f62f4b31652223ed3ca38cf98bbb370] iommu/io-pgtable-arm: Clean up faulty sanity check
git bisect bad b9bb694b9f62f4b31652223ed3ca38cf98bbb370
# bad: [a97740f81874c8063c12c24f34d25f10c4f5e9aa] dma-debug: convert comma to semicolon
git bisect bad a97740f81874c8063c12c24f34d25f10c4f5e9aa
# bad: [e0d072782c734d27f5af062c62266f2598f68542] dma-mapping: introduce DMA range map, supplanting dma_pfn_offset
git bisect bad e0d072782c734d27f5af062c62266f2598f68542
# first bad commit: [e0d072782c734d27f5af062c62266f2598f68542] dma-mapping: introduce DMA range map, supplanting dma_pfn_offset

Guenter

Re: [PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT

[Frank Rowand]

On 4/8/21 9:20 PM, Guenter Roeck wrote:
> On 4/8/21 3:53 PM, Frank Rowand wrote:
>> On 4/8/21 4:54 PM, Guenter Roeck wrote:
>>> On 4/8/21 2:28 PM, Rob Herring wrote:
>>>>
>>>> Applying now so this gets into linux-next this week.
>>>>
>>> The patch doesn't apply on top of today's -next; it conflicts
>>> with "of: properly check for error returned by fdt_get_name()".
>>>
>>> I reverted that patch and applied this one, and the DT unittests
>>> run with it on openrisc. I do get a single test failure, but I that
>>> is a different problem (possibly with the test case itself).
>>>
>>> ### dt-test ### FAIL of_unittest_dma_ranges_one():923 of_dma_get_range: wrong DMA addr 0x00000000
>>> 	(expecting 100000000) on node /testcase-data/address-tests/bus@80000000/device@1000
>>
>> That is a known regression on the target that I use for testing (and
>> has been since 5.10-rc1) - the 8074 dragonboard, arm 32.  No
>> one else has reported it on the list, so even though I want to debug
>> and fix it "promptly", other tasks have had higher priority.  In my
>> notes I list two suspect commits:
>>
>>   e0d072782c73 dma-mapping: introduce DMA range map, supplanting dma_pfn_offset
>>   0a0f0d8be76d dma-mapping: split <linux/dma-mapping.h>
>>
>> I think that was purely based on looking at the list of commits that
>> may have touched OF dma.  I have not done a bisect.
>>
> 
> Here you are:
> 
> # bad: [2c85ebc57b3e1817b6ce1a6b703928e113a90442] Linux 5.10
> # good: [bbf5c979011a099af5dc76498918ed7df445635b] Linux 5.9
> git bisect start 'v5.10' 'v5.9'
> # bad: [4d0e9df5e43dba52d38b251e3b909df8fa1110be] lib, uaccess: add failure injection to usercopy functions
> git bisect bad 4d0e9df5e43dba52d38b251e3b909df8fa1110be
> # good: [f888bdf9823c85fe945c4eb3ba353f749dec3856] Merge tag 'devicetree-for-5.10' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
> git bisect good f888bdf9823c85fe945c4eb3ba353f749dec3856
> # good: [640eee067d9aae0bb98d8706001976ff1affaf00] Merge tag 'drm-misc-next-fixes-2020-10-13' of git://anongit.freedesktop.org/drm/drm-misc into drm-next
> git bisect good 640eee067d9aae0bb98d8706001976ff1affaf00
> # good: [c6dbef7307629cce855aa6b482b60cbf7777ed88] Merge tag 'usb-5.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> git bisect good c6dbef7307629cce855aa6b482b60cbf7777ed88
> # good: [ce1558c285f9ad04c03b46833a028230771cc0a7] ALSA: hda/hdmi: fix incorrect locking in hdmi_pcm_close
> git bisect good ce1558c285f9ad04c03b46833a028230771cc0a7
> # good: [c48b75b7271db23c1b2d1204d6e8496d91f27711] Merge tag 'sound-5.10-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
> git bisect good c48b75b7271db23c1b2d1204d6e8496d91f27711
> # bad: [0cd7d9795fa82226e7516d38b474bddae8b1ff26] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching
> git bisect bad 0cd7d9795fa82226e7516d38b474bddae8b1ff26
> # good: [b1839e7c2a42ccd9a0587c0092e880c7a213ee2a] dmaengine: xilinx: dpdma: convert tasklets to use new tasklet_setup() API
> git bisect good b1839e7c2a42ccd9a0587c0092e880c7a213ee2a
> # bad: [0de327969b61a245e3a47b60009eae73fe513cef] cma: decrease CMA_ALIGNMENT lower limit to 2
> git bisect bad 0de327969b61a245e3a47b60009eae73fe513cef
> # good: [6eb0233ec2d0df288fe8515d5b0b2b15562e05bb] usb: don't inherity DMA properties for USB devices
> git bisect good 6eb0233ec2d0df288fe8515d5b0b2b15562e05bb
> # bad: [48d15814dd0fc429e3205b87f1af6cc472018478] lib82596: move DMA allocation into the callers of i82596_probe
> git bisect bad 48d15814dd0fc429e3205b87f1af6cc472018478
> # bad: [eba304c6861613a649ba46cfab835b1eddeacd8e] dma-mapping: better document dma_addr_t and DMA_MAPPING_ERROR
> git bisect bad eba304c6861613a649ba46cfab835b1eddeacd8e
> # bad: [b9bb694b9f62f4b31652223ed3ca38cf98bbb370] iommu/io-pgtable-arm: Clean up faulty sanity check
> git bisect bad b9bb694b9f62f4b31652223ed3ca38cf98bbb370
> # bad: [a97740f81874c8063c12c24f34d25f10c4f5e9aa] dma-debug: convert comma to semicolon
> git bisect bad a97740f81874c8063c12c24f34d25f10c4f5e9aa
> # bad: [e0d072782c734d27f5af062c62266f2598f68542] dma-mapping: introduce DMA range map, supplanting dma_pfn_offset
> git bisect bad e0d072782c734d27f5af062c62266f2598f68542
> # first bad commit: [e0d072782c734d27f5af062c62266f2598f68542] dma-mapping: introduce DMA range map, supplanting dma_pfn_offset
> 
> Guenter
> 

Thank you !!!!!!!!!!!!!!!!