Code First: Remove the restriction on "Package" of Properties of the Package() data structure

[edhay]

Description
The UEFI DSD Guide defines the data structure format for Package() in
section "3.1.1. Data Format Definition"

"data format for the Package() (Data Structure) immediately following it as a list of Packages of length two (2), known as Properties. The first element of each Property (the Key) must be a String and the second element (the Value) must be:
an Integer,
a String,
a Reference, or
a Package consisting entirely of Integer, String, or Reference objects (and specifically not containing a nested Package)"

The Package (the value of the <key, value> pair) is restricted to consist of "entirely of Integer, String, or Reference objects (and specifically not containing a nested Package)." is not valid as
as we have examples of nested Packages

Reference: "Arm Firmware Framework for Arm A-profile" https://developer.arm.com/documentation/den0077/m/?lang=en
18.8.2.1 FF-A Notifications Property
Package {
Revision, // Integer (DWORD)
Count, // Integer (DWORD)
NTF_Package[1], // Package
...
NTF_Package[N] // Package
}

Proposal -- updated

3.1.1. Data Format Definition
...
...
  an Integer,
  a String,
- a Reference, or
+ a Reference (but not referencing a package which in turn contains nested package references), or
- a Package consisting entirely of Integer, String, or Reference objects
- (and specifically not containing a nested Package).
+ a Package (but not containing a reference to another Package).

Benefits of this change
The Package value will be able to accommodate nested Packages and more.

[rafaeljw]

I think that in practice nested packages are used in this context, but that leads to enumeration issues.

For instance, if a reference to a named object is present in a hierarchical extension _DSD section, the referenced object will be evaluated in place producing a package that is effectively nested in the package containing the reference. However, in that case the contents of the nested package cannot be referenced from the outside as a separate object (it has no name).

In fact, one can argue that this use violates the "no nested packages rule" because a reference to a named object is not a reference object and it would be better to update the hierarchical extension section to forbid this use of references (and only allow namepaths represented as strings to be used for pointing to "data nodes").

For this reason, I'm wondering what the background of this proposal is.

[edhay]

The proposal has been updated above after internal discussions.

Hi @rafaeljw

1. Our requirement is to enable nested Package (not references) inside Data Prop UUID, since some of our specs need it for optimal data representation purposes.
2. We are NOT presenting any requirement to reference a “reference to named Package inside Hierarchical Data UUID” from somewhere else in ASL. It is an existing restriction. If we want to explicitly disbar references and only allow namepaths (or just point out the difference and restrictions), then a separate ECR would be preferrable since it’s a slightly different discussion.

[rafaeljw]

The proposal has been updated above after internal discussions.

Hi @rafaeljw

1. Our requirement is to enable nested Package (not references) inside Data Prop UUID, since some of our specs need it for optimal data representation purposes.

It is unclear to me why nested packages would be needed. The example given in the proposal can easily be flattened into a multivalued property.

If I remember correctly, the purpose of this restriction was to avoid overly complicated property values on the premise that if a single multivalued property were insufficient, multiple properties should be used.

Also, this is not only a matter of specification language. The code parsing ACPI device properties in Linux takes this limitation into account. There is currently no mechanism in the code allowing property values including nested packages to be retrieved and adding one will not be straightforward.

For this reason, I'm against making the change.

3. We are NOT presenting any requirement to reference a “reference to named Package inside Hierarchical Data UUID” from somewhere else in ASL. It is an existing restriction. If we want to explicitly disbar references and only allow namepaths (or just point out the difference and restrictions), then a separate ECR would be preferrable since it’s a slightly different discussion.

OK

[souvikkc]

Hi Rafael,

The spec currently has the following package value wrt a "NTF" key. It basically describes the "abstract" sources of notifications which FFA (an Arm standard) complaint implementation can generate:

Package {
Revision, // Integer (DWORD)
Count, // Integer (DWORD)
NTF_Package[1], // Package
…
NTF_Package[N] // Package
}

Each NTF_Package comprises:

Package {
Service_UUID, // Buffer
Cookie_Package // Package
}

Each Cookie_package comprises:

Package {
Cookie[1], // Integer (DWORD)
…
Cookie[N] // Integer (DWORD)
}

And the number of cookies is variable per NTF_Packages, and not the same across all packages.

If we split the package into constituent contents, we would end up with N*(M+2) + 1 keys instead of 1. (I am assuming the same number M here for simplicity in calculations)

E.g keys:

“Ffa-ntf-uuid-total”

“Ffa-ntf-uuid1”
“Ffa-ntf-uuid1-cookie-total”
“Ffa-ntf-uuid1-cookie1”
“Ffa-ntf-uuid1-cookie2”
….
“Ffa-ntf-uuid1-cookieM”

“Ffa-ntf-uuid2”
“Ffa-ntf-uuid2-cookie-total”
“Ffa-ntf-uuid2-cookie1”
“Ffa-ntf-uuid2-cookie2”
…
“Ffa-ntf-uuid2-cookieM”

…

“Ffa-ntf-uuidN”
“Ffa-ntf-uuidN-cookie-total”
“Ffa-ntf-uuidN-cookie1”
“Ffa-ntf-uuidN-cookie2”
…
“Ffa-ntf-uuidN-cookieM”

The sheer volume of keys would be a hard to maintain and keep track of. In addition it would be error prone to specify and implement across the Arm partner ecosystem. And for any new additions, we would end up creating yet additional keys which having a versioned package avoids. Hopefully this explains to some extent why we didn't want to flatten this.

I would like to understand a bit more reg. your comment: "There is currently no mechanism in the code allowing property values including nested packages to be retrieved and adding one will not be straightforward."

We proposed to prohibit references in the ECR text above so that everything is inlined inside the Package (and thus there is no expectation of referencing the nested packages from outside separately) to try to make some of the property parsing code easier to reason about.

However if that is not the complication you were referring to, it would help if you could please elaborate a bit.

Regards,
Souvik Chakravarty
Arm Limited.

[rafaeljw]

One problem with this approach is that Linux has no code allowing properties with nested packages to be retrieved by API users.

Also, if I'm not mistaken, the package in your example can be flattened in the following way:

Package {
Revision, // Integer (DWORD)
NTF_Package_Count, // Integer (DWORD)
Service_UUID_1, // Buffer
Cookie_Count_1, // Integer (DWORD)
Cookie_1_1, // Integer (DWORD)
...
Cookie_1_N, // Integer (DWORD)
Service_UUID_2, // Buffer
Cookie_Count_2, // Integer (DWORD)
Cookie_2_1, // Integer (DWORD)
...
Cookie_2_N, // Integer (DWORD)
...
}

so it can become one multi-value property with one key, but there is still a problem with buffer items in it which will be rejected by acpi_property_value_ok() in Linux.

It looks like you may be better off if you define a special _DSD UUID for this, in which you can retain the nested package structure.