Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752764AbdLEQef (ORCPT ); Tue, 5 Dec 2017 11:34:35 -0500 Received: from mail.kernel.org ([198.145.29.99]:43024 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752481AbdLEQeb (ORCPT ); Tue, 5 Dec 2017 11:34:31 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 19AFF218B3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=atull@kernel.org X-Google-Smtp-Source: AGs4zMbT/BqojU/AvDJj9jjwI9szCK3PC4kzFNWMz4DdYi0nEPQsGp2/6g8rQogMHH699fXrGRd18cxmfaDE7letYmI= MIME-Version: 1.0 In-Reply-To: <24161ebf-81be-bec7-9fe8-36279a8b5a8d@gmail.com> References: <1511816284-12145-1-git-send-email-atull@kernel.org> <24161ebf-81be-bec7-9fe8-36279a8b5a8d@gmail.com> From: Alan Tull Date: Tue, 5 Dec 2017 10:33:49 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC 0/2] of: Add whitelist To: Frank Rowand Cc: Rob Herring , Pantelis Antoniou , Moritz Fischer , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , linux-fpga@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5082 Lines: 116 On Thu, Nov 30, 2017 at 6:46 AM, Frank Rowand wrote: > On 11/29/17 11:11, Alan Tull wrote: >> On Wed, Nov 29, 2017 at 7:31 AM, Rob Herring wrote: >>> On Wed, Nov 29, 2017 at 3:20 AM, Frank Rowand wrote: >>>> On 11/27/17 15:58, Alan Tull wrote: >>>>> Here's a proposal for a whitelist to lock down the dynamic device tree. >>>>> >>>>> For an overlay to be accepted, all of its targets are required to be >>>>> on a target node whitelist. >>>>> >>>>> Currently the only way I have to get on the whitelist is calling a >>>>> function to add a node. That works for fpga regions, but I think >>>>> other uses will need a way of having adding specific nodes from the >>>>> base device tree, such as by adding a property like 'allow-overlay;' >>>>> or 'allow-overlay = "okay";' If that is acceptable, I could use some >>>>> advice on where that particular code should go. >>>>> >>>>> Alan >>>>> >>>>> Alan Tull (2): >>>>> of: overlay: add whitelist >>>>> fpga: of region: add of-fpga-region to whitelist >>>>> >>>>> drivers/fpga/of-fpga-region.c | 9 ++++++ >>>>> drivers/of/overlay.c | 73 +++++++++++++++++++++++++++++++++++++++++++ >>>>> include/linux/of.h | 12 +++++++ >>>>> 3 files changed, 94 insertions(+) >>>>> >>>> >>>> The plan was to use connectors to restrict where an overlay could be applied. >>>> I would prefer not to have multiple methods for accomplishing the same thing >>>> unless there is a compelling reason to do so. >>> >>> Connector nodes need a mechanism to enable themselves, too. I don't >>> think connector nodes are going to solve every usecase. >>> >>> Rob >> >> The two methods I'm suggesting are intended to handle different cases. >> There will exist some drivers that by their nature will want every >> instance to be enabled for overlays, such as fpga regions. The other >> case is where drivers could support overlays but that's not the >> widespread use for them. So no need to enable every instance of that >> driver for overlays. > > I understand what the paragraph, to this point, means. But I had to > read it several times to understand it because the way the concept is > phrased clashed with my mental model. Hi Frank, I see where my explanation is confusing things. I was talking about two methods for marking a node as being a valid target for an overlay (use a function or add a DT property). I'll drop the idea of using a DT property to enable a node for overlays and only focus on my proposal of a function to enable nodes. > > The device node is not an instance of a driver, which is why I was > getting confused. (Yes, I do understand that the paragraph is talking > about multiple device nodes that are bound to the same driver, but > my mental model is tied to the device node, not to the driver.) > > If each of the device nodes in question is a connector, then each of > the nodes will bind to a connector driver, based on the value of the > compatible property. (This is of course a theoretical assumption on > my part since the connectors are not yet implemented.) > > If the connector node is an fpga, or an fpga region (I may be getting > my terminology wrong here - please correct as needed) then an fpga > overlay could be applied to the node. We're still pre-connector currently, but yes I want to mark FPGA regions as being valid targets. Then I can use Pantelis' configfs interface to apply overlays while leaving the rest of the DT locked down. That's the FPGA use of this patch in the pre-connector era of things. > > If I understand what you are saying, there will be some fpga connector > nodes for which the usage at a given moment might be programmed to > function in a manner that will not be described by an overlay, but > at a different moment in time may be programmed in a way that needs > to be described by an overlay. So there may be some times that it > is valid to apply an overlay to the connector node and times that > it is not valid to apply an overlay to the connector node. I think connectors would likely always be valid targets (but I could be wrong) and other nodes would not be valid targets. The DT needs a way to mark some nodes as valid targets, currently it doesn't have a way of doing that. Every connector driver's probe could use this code to mark itself as a valid target. > > Is my understanding correct, or am I still confused? Hope that helps, sorry for the muddled explanation earlier. Alan > > -Frank > >> In that case the DT property provides some >> granularity, only enabling overlays for specific instances of that >> driver, leaving the rest of the DT locked down.> >> If we only want one method, I would choose having the DT property only >> and not exporting the functions. Users would have to add the property >> for every FPGA region but that's not really painful. This would have >> the benefit of still keeping the DT locked down unless someone >> specifically wanted to enable some regions for overlays for their >> particular use. >> >> Alan >> >