On Fri, Oct 23, 2026 at 10:55:11AM +0200, Herve Codina wrote: > Hi David, > > On Fri, 23 Oct 2026 13:57:54 +1100 > David Gibson 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 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 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. Probably wants some editing for clarity in the final version, but basically sounds good. > > > (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. Ok. > > > (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. The idea's right, though I don't think that expresses it particularly well. Not a priority right now, we can revise that later. > > > 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? Yes, it is. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson