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

[Herve Codina]

Hi David,

On Thu, 10 Sep 2026 19:32:48 +1000
David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx> 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".

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

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?

[1] https://lore.kernel.org/all/20260826094950.1088288-52-herve.codina@xxxxxxxxxxx/

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

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?


Best regards,
Hervé