Re: [PATCH v3 06/15] Introduce structured tag value definition

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6428 bytes --]

On Fri, Sep 11, 2026 at 09:16:53AM +0200, Herve Codina wrote:
> Hi David,
> 
> On Thu, 10 Sep 2026 19:32:48 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, Sep 10, 2026 at 09:41:26AM +0200, Herve Codina wrote:
> > > Hi David,
> > > 
> > > On Thu, 10 Sep 2026 14:51:08 +1000
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > On Wed, Aug 26, 2026 at 10:31:37AM +0200, Herve Codina wrote:  
> > > > > The goal of structured tag values is to ease the introduction of new
> > > > > tags in future releases with the capability for an already existing
> > > > > release to ignore those structured tags. In order to do that data length
> > > > > related to the unknown tag needs to be identified.
> > > > > 
> > > > > Also add a flag to tell an old release if this tag can be simply skipped
> > > > > or must lead to an error.    
> > > > 
> > > > I suggested/insisted on this in the past, but I've since realised this
> > > > isn't actually useful.  The "structured tag" format is only useful
> > > > because it allows you to skip over unknown tags, which means there's
> > > > no point using it for ags that can't be skipped.  If we need new tags
> > > > that can't be safely skipped, they can be added as old-style tags
> > > > instead.
> > > >
> > > > In which case "old style" versus "new style" is no longer a good
> > > > characterization: they would exist side by side, so the distinction is
> > > > more "skippable" versus "non skippable" tags.  Which suggests that
> > > > "skippable tags" or "metadata tags" might be a better term than
> > > > "structured tags" - that would focus more on the why than the how.  
> > > 
> > > Do you mean that the SKIP_SAFE bit should be removed ?  
> > 
> > Yes.
> 
> So, in that case the term "skippable" tags makes sense.
> 
> Any non-skippable tag are then defined using the "old style".

Right.  I kind of prefer the term "metadata" *if* it's actually
accurate, but I'm not sure it is.

> The only non-skippable tag that will be introduce later by addons is
> FDT_BEGIN_NODE_REF (patch 51/71 [1]).

Ok.

> Data related to this tag is the symbol name and so a string.
> 
> Using the "old style" for this tag, I will remove the 32-bit data lengh.
> The next tag will be after the end of string '\0' + potential alignment.
> 
> This is consistent with "old style" tags which have a string as data. Indedd
> no 32-bit data lengh were present for those "old style" tags.
> 
> Does it make sense on your side?

Yes, that makes sense.

> [1] https://lore.kernel.org/all/20260826094950.1088288-52-herve.codina@bootlin.com/
> 
> > 
> > > I like the defined structure with the DATA_LEN_ENCODING part. Even for tags
> > > which are not "skippable".
> > > 
> > > This ensures a kind of standardized format for all future tags instead of a 
> > > specific definition (related to the length of the data) for each new tag.  
> > 
> > If we were designing the dtb format from scratch, I'd agree.  But
> > given we already have what we have I think the drawbacks of
> > introducing a second way of doing tags outweighs the benefits.
> > 
> > > For this kind of information related to the length of data, I prefer a global
> > > rule instead of tag-specific rules.  
> > 
> > Right, but it can't truly be global, because we have the existing
> > tags.
> > 
> > > > > Structured tag value is defined on 32bit and is defined as follow:
> > > > > 
> > > > >   Bits  | 31 | 30        | 29             28 | 27    0|
> > > > >   ------+----+-----------+-------------------+--------+
> > > > >   Fields| 1  | SKIP_SAFE | DATA_LEN_ENCODING | TAG_ID |
> > > > >   ------+----+-----------+-------------------+--------+
> > > > > 
> > > > > Bit 31 is always set to 1 to identify a structured tag value.
> > > > > 
> > > > > Bit 30 (SKIP_SAFE) is set to 1 if the tag can be safely ignored when its
> > > > > TAG_ID value is not a known value (unknown tag). If the SKIP_SAFE bit is
> > > > > set to 0 this tag must not be ignored and an error should be reported
> > > > > when its TAG_ID value is not a known value (unknown tag).
> > > > > 
> > > > > Bits 29..28 (DATA_LEN_ENCODING) indicates the length of the data related
> > > > > to the tag. Following values are possible:
> > > > >   - 0b00: No data.
> > > > >           The tag is followed by the next tag value.    
> > > > 
> ... 
> > > > >   - 0b11: Data length encoding
> > > > >           The tag is followed by a cell (u32) indicating the size of the
> > > > >           data. This size is given in bytes. Data are available right
> > > > >           after this cell.
> > > > > 
> > > > >           The next tag is available after the data. Padding is present
> > > > >           after the data in order to have the next tag aligned on 32bits.
> > > > >           This padding is not included in the size of the data.    
> > > > 
> > > > I'm guessing the 1 & 2 cell cases are pretty common so it saves a
> > > > moderate amount of dtb size to have this encoding.  However, it does
> > > > come at the cost of greater code complexity.  Is it worth it?  I'm
> > > > willing to believe it is, but I think the case needs to be made.  
> > > 
> > > In term of code complexity, FDT_PROPDATA_PHANDLE introduced in the addons
> > > series, patch 7/74 [0], is a 1-cell (or 1-fdt32) "structured" tag.
> > > 
> > > It can be compared with FDT_PROPDATA_PHANDLE_REF, introduced in patch 11/74 [1].
> > > FDT_PROPDATA_PHANDLE_REF is a "Data length encoding" tag.
> > > 
> > > Modifications in libfdt/fdt.c are pretty similar for both tags.  
> > 
> > Right, it won't be in the case of specific tag handling.  It's the
> > general case skipping that's more complex, because it has to consider
> > both cases.
> 
> I don't understand. What do you mean ?
> 
> Current implementation for skipping tags is not so complex.

Not that complex, no, but still more complex than it would be if we
always have the 32-bit length field.

> Do you mean that we should avoid the DATA_LEN_ENCODING and always have the
> 32-bit value right after the tag to give the size for all "skippable" tags?

Yes.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]