From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753714AbeDEUdU (ORCPT ); Thu, 5 Apr 2018 16:33:20 -0400 Received: from mail-pl0-f51.google.com ([209.85.160.51]:43356 "EHLO mail-pl0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753399AbeDEUdS (ORCPT ); Thu, 5 Apr 2018 16:33:18 -0400 X-Google-Smtp-Source: AIpwx4+bgmRj5vwZyapKk00jRoLI1cz5sZYlwpE8bOt75OmCW2NrFPLeyK0FmW1JdNb02Ob4dpiJhA== Subject: Re: [PATCH v7 2/5] of: change overlay apply input data from unflattened to FDT To: Jan Kiszka , Rob Herring , pantelis.antoniou@konsulko.com, Pantelis Antoniou Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, geert@linux-m68k.org, laurent.pinchart+renesas@ideasonboard.com, Jailhouse References: <1520122673-11003-1-git-send-email-frowand.list@gmail.com> <1520122673-11003-3-git-send-email-frowand.list@gmail.com> <09e3db63-cbf9-52a2-ee77-520979f17fea@web.de> <7bbf615b-3cdd-6bb4-6918-33e48de4225d@gmail.com> <255dffbd-4ff1-42a4-033b-311d49339102@web.de> From: Frank Rowand Message-ID: <70c766c9-aa4b-d67c-37a0-795fc5bc228f@gmail.com> Date: Thu, 5 Apr 2018 13:33:15 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <255dffbd-4ff1-42a4-033b-311d49339102@web.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/05/18 12:38, Jan Kiszka wrote: > On 2018-04-05 21:28, Frank Rowand wrote: >> On 04/05/18 12:13, Jan Kiszka wrote: >>> On 2018-04-05 20:59, Frank Rowand wrote: >>>> Hi Jan, >>>> >>>> On 04/04/18 15:35, Jan Kiszka wrote: >>>>> Hi Frank, >>>>> >>>>> On 2018-03-04 01:17, frowand.list@gmail.com wrote: >>>>>> From: Frank Rowand >>>>>> >>>>>> Move duplicating and unflattening of an overlay flattened devicetree >>>>>> (FDT) into the overlay application code. To accomplish this, >>>>>> of_overlay_apply() is replaced by of_overlay_fdt_apply(). >>>>>> >>>>>> The copy of the FDT (aka "duplicate FDT") now belongs to devicetree >>>>>> code, which is thus responsible for freeing the duplicate FDT. The >>>>>> caller of of_overlay_fdt_apply() remains responsible for freeing the >>>>>> original FDT. >>>>>> >>>>>> The unflattened devicetree now belongs to devicetree code, which is >>>>>> thus responsible for freeing the unflattened devicetree. >>>>>> >>>>>> These ownership changes prevent early freeing of the duplicated FDT >>>>>> or the unflattened devicetree, which could result in use after free >>>>>> errors. >>>>>> >>>>>> of_overlay_fdt_apply() is a private function for the anticipated >>>>>> overlay loader. >>>>> >>>>> We are using of_fdt_unflatten_tree + of_overlay_apply in the >>>>> (out-of-tree) Jailhouse loader driver in order to register a virtual >>>>> device during hypervisor activation with Linux. The DT overlay is >>>>> created from a a template but modified prior to application to account >>>>> for runtime-specific parameters. See [1] for the current implementation. >>>>> >>>>> I'm now wondering how to model that scenario best with the new API. >>>>> Given that the loader lost ownership of the unflattened tree but the >>>>> modification API exist only for the that DT state, I'm not yet seeing a >>>>> clear solution. Should we apply the template in disabled form (status = >>>>> "disabled"), modify it, and then activate it while it is already applied? >>>> >>>> Thank you for the pointer to the driver - that makes it much easier to >>>> understand the use case and consider solutions. >>>> >>>> If you can make the changes directly on the FDT instead of on the >>>> expanded devicetree, then you could move to the new API. >>> >>> Are there some examples/references on how to edit FDTs in-place in the >>> kernel? I'd like to avoid writing the n-th FDT parser/generator. >> >> I don't know of any existing in-kernel edits of the FDT (but they might >> exist). The functions to access an FDT are in libfdt, which is in >> scripts/dtc/libfdt/. >> > > Ah, libfdt is available for kernel drivers as well. That looks like a > viable path on first sight. I'll try that and come back in case it does > not solve all issues. > >> >>>> >>>> Looking at the driver, I see one potential issue with that approach. >>>> The property "interrupt-map" is added directly to the changeset >>>> instead of being an existing property in the overlay. Is it possible >>>> to have this property in the overlay when needed? >>> >>> Well, the size of that property has a runtime dependency on the gic's >>> #address-cells. If that is easy to account for depends a bit on the >>> available FDT manipulation services. Or it would take multiple templates >>> to handle the different cases (0, 1, or 2 IIRC). >> >> If I understand create_vpci_of_overlay() correctly, it is assuming a >> fixed size of 4 cells. Line 314 is: for (n = 0, cell = 0; n < 4; n++) { >> >> Off the top of my head, it is theoretically possible to create a property >> that can contain the largest possible size for the property, then shrink >> the size by inserting NOPs at the end of the property to shrink it. > > Well, I even find fdt_appendprop which sounds like we could keep adding > that property on the fly. > > How does memory management work with libfdt? Do I have to ensure that > the fdt is already backed by an area large enough also for it modified form? I have not looked at those functions, so I don't know. > > Thanks, > Jan >