2021-04-08 20:48:40

by Frank Rowand

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

From: Frank Rowand <[email protected]>

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/[email protected]/

Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Frank Rowand <[email protected]>

---

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 <[email protected]>


2021-04-08 21:29:07

by Rob Herring

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

On Thu, Apr 8, 2021 at 3:45 PM <[email protected]> wrote:
>
> From: Frank Rowand <[email protected]>
>
> 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/[email protected]/
>
> Reported-by: Guenter Roeck <[email protected]>
> Signed-off-by: Frank Rowand <[email protected]>
>
> ---
>
> 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

2021-04-08 21:56:16

by Guenter Roeck

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

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 <[email protected]>

Guenter

2021-04-08 22:41:06

by Frank Rowand

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

On 4/8/21 4:28 PM, Rob Herring wrote:
> On Thu, Apr 8, 2021 at 3:45 PM <[email protected]> wrote:
>>
>> From: Frank Rowand <[email protected]>
>>
>> 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/[email protected]/
>>
>> Reported-by: Guenter Roeck <[email protected]>
>> Signed-off-by: Frank Rowand <[email protected]>
>>
>> ---
>>
>> 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
>

2021-04-08 22:56:28

by Frank Rowand

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

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 <[email protected]>

Thanks for testing!

>
> Guenter
>

2021-04-09 02:21:07

by Guenter Roeck

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

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

2021-04-09 04:00:24

by Frank Rowand

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

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 !!!!!!!!!!!!!!!!