Hi Frank,

On Oct 19, 2017, at 00:46 , Frank Rowand [off-list ref] =

wrote:

=20
On 10/18/17 11:30, Rob Herring wrote:

quoted

On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
[off-list ref] wrote:

quoted

On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:

quoted

On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull [off-list ref] =

wrote:

quoted

quoted

quoted

quoted

On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand =

[off-list ref] wrote:

quoted

quoted

quoted

quoted

quoted

On 10/17/17 14:46, Rob Herring wrote:

quoted

On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull [off-list ref] =

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring [off-list ref] =

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

=20
Hi Rob,
=20

quoted

With dependencies on a statically allocated full path name =

converted to

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

use %pOF format specifier, we can store just the basename of =

node, and

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

the unflattening of the FDT can be simplified.
=20
This commit will affect the remaining users of full_name. =

After

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

analyzing these users, the remaining cases should only change =

some print

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

messages. The main users of full_name are providing a name for =

struct

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

resource. The resource names shouldn't be important other than =

providing

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

/proc/iomem names.
=20
We no longer distinguish between pre and post 0x10 dtb formats =

as either

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

a full path or basename will work. However, less than 0x10 =

formats have

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

been broken since the conversion to use libfdt (and no one has =

cared).

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

The conversion of the unflattening code to be non-recursive =

also broke

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

pre 0x10 formats as the populate_node function would return 0 =

in that

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

case.
=20
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- rebase to linux-next
=20
drivers/of/fdt.c | 69 =

+++++++++-----------------------------------------------

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

1 file changed, 11 insertions(+), 58 deletions(-)

=20
I've just updated to the latest next branch and am finding =

problems

quoted

quoted

quoted

quoted

quoted

quoted

quoted

applying overlays.   Reverting this commit alleviates things.  =

The

quoted

quoted

quoted

quoted

quoted

quoted

quoted

errors I get are:
=20
[   88.498704] OF: overlay: Failed to apply prop =

@/__symbols__/clk_0

quoted

quoted

quoted

quoted

quoted

quoted

quoted

[   88.513447] OF: overlay: apply failed '/__symbols__'
[   88.518423] create_overlay: Failed to create overlay =

(err=3D-12)

quoted

quoted

quoted

quoted

quoted

quoted

=20
Frank's series with overlay updates should fix this.

=20
Yes, it does:
=20
 [PATCH v3 11/12] of: overlay: remove a dependency on device node =

full_name

quoted

quoted

quoted

quoted

=20
Thanks for the fast response.  I fetched the dt/next branch to =

test

quoted

quoted

quoted

quoted

this but there are sufficient changes that Pantelis' "OF: =

DT-Overlay

quoted

quoted

quoted

quoted

configfs interface (v7)" is broken now.  I've been adding that
downstream since 4.4.  We're using it as an interface for applying
overlays to program FPGAs.  If we fix it again, is there any =

chance

quoted

quoted

quoted

quoted

that can go upstream now?

=20
With a drive-by posting once every few years, no.
=20

=20
I take offense to that. There's nothing changed in the patch for =

years.

quoted

quoted

Reposting the same patch without changes would achieve nothing.

=20
Are you still expecting review comments on it or something?
Furthermore, If something is posted infrequently, then I'm not
inclined to comment or care if the next posting is going to be after =

I

quoted

forget what I previously said (which is not very long).
=20
I'm just saying, don't expect to forward port, post and it will be
accepted. Below is minimally one of the issues that needs to be
addressed.
=20

=20
=20

quoted

quoted

quoted

The issue remains that the kernel is not really setup to deal with =

any

quoted

quoted

quoted

random property or node to be changed at any point in run-time. I
think there needs to be some restrictions around what the overlays =

can

quoted

quoted

quoted

touch. We can't have it be wide open and then lock things down =

later

quoted

quoted

quoted

and break users.

=20
That paragraph is key to any discussion of accepting code to apply =

overlays.

Solving that issue has been stated to be a gating factor for such code =

from

the beginning of overlay development.
=20

Overlays are not only used only for cases where you have external =
expansion boards, or
FPGAs where every change is contained under a few designated nodes, so =
that=E2=80=99s why I=E2=80=99m
pushing for a in-kernel validator that=E2=80=99s more flexible than a =
single whitelist.

An eBPF validator would handle a whitelist trivially easy, and would be =
flexible enough
for any other more complicated use case.

=20

quoted

quoted

quoted

One example of what you could do is you can only add
sub-trees to whitelisted nodes. That's probably acceptable for your
usecase.
=20

=20
Defining what can and what cannot be changed is not as trivial as a
list of white-listed nodes.

=20
No, but we have to start somewhere and we are not starting with any
change allowed anywhere at anytime. If that is what people want, then
they are going to get to maintain that out of tree.
=20

quoted

In some cases there is a whole node hierarchy being inserted (like =

in

quoted

quoted

a FPGA).

=20
Yes, so you'd have a target fpga region. That sounds fine to me. =

Maybe

quoted

its not a static whitelist, but drivers have to register target
nodes/paths.
=20

quoted

In others, it's merely changing a status property to "okay" and
a few device parameters.

=20
That seems fine too. Disabled nodes could be allowed. But what if you
add/change properties on a node that is not disabled? Once a node is
enabled, who is responsible for registering the device?
=20
What about changing a node from enabled to disabled? The kernel would
need to handle that or not allow it.
=20

quoted

The real issue is that the kernel has no way to verify that a given
device tree, either at boot time or at overlay application time, is
correct.
=20
When the tree is wrong at boot-time you'll hang (if you're lucky).
If the tree is wrong at run-time you'll get some into some =

unidentified

quoted

quoted

funky state.

=20
Or have some security hole or a mechanism for userspace to crash the =

system.

quoted

=20

quoted

Finally what is, and what is not 'correct' is not for the kernel to
decide arbitrarily, it's a matter of policy, different for each
use-case.

=20
It is if the kernel will break doing so.
=20
Rob
=20

=20