Re: [PATCH v3 15/15] Introduce v18 dtb version

[Herve Codina <herve.codina@bootlin.com>]

Hi David,

On Fri, 23 Oct 2026 13:57:54 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Oct 22, 2026 at 03:14:54PM +0200, Herve Codina wrote:
> > Hi David,
> > 
> > On Thu, 22 Oct 2026 14:17:38 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Wed, Oct 21, 2026 at 05:20:43PM +0200, Herve Codina wrote:  
> > > > Hi David,
> > > > 
> > > > On Wed, 21 Oct 2026 13:26:49 +1100
> > > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > >     
> > > > > > 
> > > > > > Ok, my proposal seems to be a no-go and so I will follow your proposal given
> > > > > > above.      
> > > > > 
> > > > > To be clear, I'm rejecting the original proposal because I think it's
> > > > > internally inconsistent (if you follow the consequences of decisions
> > > > > far enough).  I'm not necessarily opposed to something other than the
> > > > > above, as long as the rules for when unknown tags must be invalidated
> > > > > (and what that does to the version) are made very clear in the
> > > > > structured tag spec.  e.g. see an alternative proposal below.    
> > > > 
> > > > No problem, I understand your point.
> > > > 
> > > > ...
> > > >     
> > > > > 
> > > > > Essentially the theory of the proposal above is to be maximally
> > > > > conservative: any write (adding, removing, changing, in-place,
> > > > > whatever) will invalidate all unknown tags and downgrade the version.
> > > > > 
> > > > > 
> > > > > But, here's an alternative proposal for invalidation/version semantics
> > > > > that I think is consistent, while being a bit closer to your original
> > > > > proposal.
> > > > > 
> > > > > 1. version and last_comp_version retain their meanings.  No new header
> > > > >    version.    
> > > > 
> > > > Ok
> > > >     
> > > > > 2. If version >= 18 the tree may include any structured tags, even if
> > > > >    they weren't defined as of v18.    
> > > > 
> > > > Ok
> > > >     
> > > > > 3. Some future version might _require_ certain structured tags to be
> > > > >    present.  But a client must only rely on those tags if the version
> > > > >    indicates it can.
> > > > >      - e.g. suppose FDT_FOO tag is introduced in v20.  A client
> > > > >        reading a v19 tree _might_ see some FDT_FOO tags (because of
> > > > >        (2)), but cannot rely on them being present in all the places
> > > > >        they should be.    
> > > > 
> > > > Ok, kind of degraded mode. Whether or not a v20 compatible client can continue
> > > > or not will depend of each tags known by this v20 client. For some tags, having
> > > > them removed will not be a problem whereas it will be a problem for some other.    
> > > 
> > > Exactly.
> > >   
> > > > Makes sense.
> > > >     
> > > > > 4. Any write to a property (including in place) invalidates all
> > > > >    unknown tags on that property (they must be stripped)    
> > > > 
> > > > Ok
> > > >     
> > > > > 5. Any write to a property invalidates all unknown tags on that
> > > > >    property's node    
> > > > 
> > > > Will see in the implementation. The offset of the written property will be
> > > > incorrect after the write. Indeed, some tags (tags related to the node)
> > > > available before the property can be removed. With splices instead of
> > > > FDT_NOP, offsets change.    
> > > 
> > > Oh, good point, this rule doesn't work.  I think we still have a
> > > consistent set of rules if property writes don't invalidate parent
> > > node tags (subnode add/delete still would).  Of coutse, that does
> > > further restrict what sorts of information you could safely put in
> > > node tags under this scheme.
> > >   
> > > > This also brings a question. Are API functions such as fdt_get_property_by_offset_w()
> > > > need to be deprecated?    
> > > 
> > > I think we can keep it - kind of like fdt_next_tag() this is a
> > > low-level function.  If you use it, you're opting in to having to deal
> > > with tag invalidation.
> > >   
> > > > Those kind of functions return a pointer to the data of a property for modification
> > > > by the caller. We don't know if the caller will modify or not the property.
> > > > 
> > > > Without deprecated it we can change the function to
> > > > ---- 8< ----
> > > > struct fdt_property *fdt_get_property_by_offset_w(void *fdt,
> > > > 						  int offset,
> > > > 						  int *lenp)
> > > > {
> > > > 	int new_offset;
> > > > 
> > > >         /*
> > > >          * The modification is goind to be done out of libfdt.
> > > >          * Unknown tags are removed (rules for modification when unknown tags
> > > >          * are involved) but also known tags. Indeed, we cannot know if the
> > > >          * modification will have impacts on known tags
> > > >          */
> > > > 	new_offset = fdt_remove_all_tags_due_to_property_modification(fdt, offset);
> > > > 
> > > > 	return (struct fdt_property *)(uintptr_t)
> > > > 		fdt_get_property_by_offset(fdt, new_offset, lenp);
> > > > }
> > > > ---- 8< ----
> > > > 
> > > > The idea is to be conservative here also and consider that property data retrieved
> > > > for writing will be modified by the caller.
> > > > 
> > > > Also, the offset used as parameter of fdt_get_property_by_offset_w() is no more
> > > > valid after the call.
> > > > 
> > > > Last point to have in mind, getting the node offset from the property offset will
> > > > need to walk the tree from the beginning. It can be costly.    
> > > 
> > > Right, that's what I was thinking of when I said this might not be
> > > practical to implement.  It's doable in the common case where we alter
> > > properties by node offset + property name, but not when we use a
> > > property offset.  But the fact that this would change the offset of
> > > the property in queestion is a much bigger showstopper.
> > >   
> > > > > 6. Adding or deleting a property on a node invalidates all unknown tags
> > > > >    on the node    
> > > > 
> > > > Ok.
> > > >     
> > > > > 7. Adding or deleting a subnode invalidates all unknown tags on the
> > > > >    node    
> > > > 
> > > > Ok.
> > > >     
> > > > > 8. Whenever a tag is invalidated and stripped, the overall version
> > > > >    must be downgraded (to let clients know they can't count on tags
> > > > >    being present)    
> > > > 
> > > > Ok.
> > > >     
> > > > > 
> > > > > Points (4)-(7) are all somewhat negotiable.  In particular, I'm not
> > > > > sure how feasible (6) is to implement.    
> > > > 
> > > > Well, this new proposal is more complex to implement than your first
> > > > proposal but it has a finer grain to handle unknown tags on modification.    
> > > 
> > > Certainly - that's why I proposed the other one first :).
> > >   
> > > > I can try to implement something based on this new proposal.    
> > > 
> > > To be clear, I kind of prefer my first proposal still.  The question
> > > is whether what the updated scheme buys (preserving more tags when a
> > > bootloader makes minor writes) worth the added complexity of
> > > implementation.  You're better placed to answer that question than I
> > > am.  
> > 
> > I would like to avoid bootloaders being blocking points and so:
> >  a) They should not failed if they encounter an unknown skippable tag.
> >     We are in sync on this point.
> > 
> >  b) They should preserve tags as much as possible.
> >     The goal is to allow following software components (probably the Kernel)
> >     to handle new tags.
> >     This is the point where the right balance has be found.  
> 
> Right.  I understand why these are both desirable.  However (b) is
> pretty tricky, because of the question of invalidation.  It
> necessarily involves a tradeoff between how much can be preserved
> versus what sorts of information future tags can carry.
> 
> > Your first proposal is maybe too much aggressive on b). That's why, even it
> > is more complex to implement I would prefer your last proposal (taking into
> > account the property offset issue) which can be summarize with the following
> > table:
> > 
> >                       | existing prop | existing node (1)   | Downgrade
> >                       | unknown tag   | unknown tag         | version (2)
> > ----------------------+---------------+---------------------+-----------
> > Modify a property (5) |    Remove     | Keep (3) 	    | Yes
> > Add a property        |    NA         | Remove              | Yes
> > Delete a property     |    Remove     | Remove              | Yes (4)
> > Add a sub-node        |    NA         | Remove              | Yes
> > Delete a sub-node     |    NA         | Remove              | Yes
> > ----------------------+---------------+---------------------+-----------  
> 
> Great summary, thanks.
> 
> > (1) The existing node is the node were the property or sub-node involved
> >     are present. Not all nodes, not the parent node but the node directly
> >     impacted by the modification.
> > 
> > (2) We downgrade the version only if tag are removed. If no unknown tags are
> >     involved (and so not removed), the version will not be be downgraded.  
> 
> Sadly, that doesn't work.  We have to downgrade on property
> modification for the same reason we must downgrade on adding a
> property: it might be adding data that requires certain tags in the
> later version.

Oh yes, I missed that point.

(2) The version is always downgrade on any changes (modification, addition,
    deletion of a property or a sub-node) even if no tags are removed.
    Indeed the modification done (even a property or sub-node deletion) can
    requires certain tags in the later version.
    Downgrading the version on all changes is the way to state that the
    resulting dtb may be no more compliant with the later version.

> 
> > (3) Removing unknown tags in the node when a property is modified will lead
> >     to the property offset change. Prefer avoid the property offset change
> >     and so keep node related unknown tags in that case.  
> 
> Right, if we did that it would basically mean totally (and
> incompatibly) redesigning the libfdt interface.
> 
> > (4) The version is downgrade only if we remove some tags related to the
> >     node. If we remove only tags related to the property we delete, there
> >     is no reason to downgrade the version.  
> 
> Unlike (2), I'm not certain if this one is unsafe.  But I suspect it
> might be, and having it behave differently from case (2) seems
> confusing to me.

Hum ok, let be conservative and remove the note (4).

The updated note (2) handles all modification. On any changes with and without
tag removal, the version is downgraded.

> 
> > (5) All property modifications done outside libfdt using directly the
> >     property data pointer will not lead to any unknown tag removal.
> >     
> >     This point is for the usage of fdt_get_property_by_offset_w(),
> >     fdt_get_property_namelen_w(), fdt_get_property_w(),
> >     fdt_getprop_by_offset_w(), fdt_getprop_namelen_w(), fdt_getprop_w() and
> >     maybe other similar functions where the property is returned writable
> >     for possible modification. 'modify a property' mentioned in the table
> >     is related to modification done using fdt_setprop_*() and fdt_appendprop_*()
> >     function families.  
> 
> Right.  "I know what I'm doing" is part of the implicit contract when
> using those functions.

Also this note (5) will be updated

(5) All property modifications done outside libfdt using directly the
    property data pointer will not lead to any unknown tag removal nor
    a version downgrade.


> 
> > On your side, is all of that conservative enough to be used when modification
> > involved unknown tags?  
> 
> Not quite, as noted, but we might be able to get it there.

Is it better? 

Best regards,
Hervé