Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name
From: Alan Tull <hidden>
Date: 2017-10-18 18:39:59
Also in:
linuxppc-dev, lkml
On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou [off-list ref] wrote:
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
On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand [off-list ref] wrote: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
On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring [off-list ref] wrote: Hi Rob,quoted
With dependencies on a statically allocated full path name converted to use %pOF format specifier, we can store just the basename of node, and the unflattening of the FDT can be simplified. This commit will affect the remaining users of full_name. After analyzing these users, the remaining cases should only change some print messages. The main users of full_name are providing a name for struct resource. The resource names shouldn't be important other than providing /proc/iomem names. We no longer distinguish between pre and post 0x10 dtb formats as either a full path or basename will work. However, less than 0x10 formats have been broken since the conversion to use libfdt (and no one has cared). The conversion of the unflattening code to be non-recursive also broke pre 0x10 formats as the populate_node function would return 0 in that case. Signed-off-by: Rob Herring <redacted> --- v2: - rebase to linux-next drivers/of/fdt.c | 69 +++++++++----------------------------------------------- 1 file changed, 11 insertions(+), 58 deletions(-)I've just updated to the latest next branch and am finding problems applying overlays. Reverting this commit alleviates things. The errors I get are: [ 88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0 [ 88.513447] OF: overlay: apply failed '/__symbols__' [ 88.518423] create_overlay: Failed to create overlay (err=-12)Frank's series with overlay updates should fix this.Yes, it does: [PATCH v3 11/12] of: overlay: remove a dependency on device node full_nameThanks for the fast response. I fetched the dt/next branch to test this but there are sufficient changes that Pantelis' "OF: DT-Overlay 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 that can go upstream now?With a drive-by posting once every few years, no.I take offense to that. There's nothing changed in the patch for years. Reposting the same patch without changes would achieve nothing.quoted
The issue remains that the kernel is not really setup to deal with any random property or node to be changed at any point in run-time.
Yeah, I'm not super surprised :) I have some whitelist ideas below.
quoted
I think there needs to be some restrictions around what the overlays can touch. We can't have it be wide open and then lock things down later and break users. One example of what you could do is you can only add sub-trees to whitelisted nodes. That's probably acceptable for your usecase.
I can take a look at making OF_OVERLAY_PRE_APPLY and OF_OVERLAY_PRE_REMOVE notifiers mandatory if that's interesting. The behavior would be: If an overlay is applied, there's got to be some handler in the kernel that verifies that it is acceptable. In my case, the handler for FPGA regions would look at the overlay and see it only added stuff under a FPGA region. And we would change the default to be: if there is no handler, reject the overlay.
quoted
Defining what can and what cannot be changed is not as trivial as a list of white-listed nodes. In some cases there is a whole node hierarchy being inserted (like in a FPGA).
For FPGA, the insertion points are always FPGA regions.
In others, it's merely changing a status property to "okay" and a few device parameters. 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. 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 funky state. 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.quoted
RobRegards -- Pantelis
Alan Tull
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html