Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

intel_debug: introduction: Split firmware lookup table into two for m… #506

Merged

Conversation

ujfalusi
Copy link
Contributor

…onolithic and modular

Separate the firmware lookup table for monolithic and modular SOF release to be able to document the locations and file names the firmware will be looking for the individual files.

Extend the description of the two type of SOF release and convert the list-table to a normal table for better descriptions for the configurations.

See :ref:`loadable-libraries` for details about library support in general.
The release contains:
- **sof-PLAT.ri** : The firmware binary
- **UUID.bin** : Mainly 3rd party libraries identified by UUID. If the library contains multiple modules then a UUID symlink must be provided for each one. Optional, only supported with IPC4 and on Meteor Lake and newer platforms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this really still be called "monolithic?" To me "monolithic" is when you have no loadable modules at all, no run-time code loading. Do we need to document 3 types then? Monolithic, individual modules and bundled modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was a mistake to release the DRC as module in a tech demo style for LNL, but my point here is:
monolithic: basefw (+optional UUID libraries, if any)
modular: basefw + openmodules + debug (+optional UUID libraries, if any)

The item tells that the UUID.bin is only supported on MTL+, which is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi Github ate my comment (or user error), but I did intend to comment at this as well. I get @lyakh 's concern here, were are in fact shipping a modular build in 2024.09 , so it is what it is.

I do like the "Modular Release" name, so not sure how to name the "before" and "after" layouts. Maybe just call it what is, "SOF Modular Release Layout V2 (for PTL and newer)", and "SOF Modular Release Layout V1" (for LNL/ARL/MTL)". Not pretty but describes what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider the UUID based loading as monolithic+library. The LNL is a short sighted tech demo to show that we can load a single module (as library) to the firmware.

But technically MTL+ will be able to load modular release (basefw+openmodules+debug+UUID), it is just that sof-bin will only going top ship modular release for PTL+.

My distinction criteria is:
If there is basefw and openmodules and/or debug) in the firmware path then it is a modular release, if there is only the basefw in there, it is monolithic.

The kernel will not care about any of these platform names, it will try to see if these fixed named libraries are there and if they are, load them.

and the UUID based library loading is also supported from MTL+ So, on PTL also.

Monolithic release == single library in fw path
Modular release == multiple libraries in fw path

Note: basefw is a library.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I now get approach. So you want to keep this generic so that driver can detect the scheme used and in theory if sof-bin ever releases a modular release layout for MTL, kernel could pick it up (unlikely as we don't want to break systems with older kernels, but alas, technically this could be done -- it's just a question of release logistics and dependencies then).

I still don't like the "monolithic release" name, but I like "modular release" I can't come with a better alternative.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the name monolithic being a bit of a misnomer too. It hints at not supporting loadable modules.

I have a suggestion, how about legacy(for the old releases) and bespoke (for the multiple binaries version) releases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand what bespoke means here...
I was thinking of:

1.3.1 Single firmware releases
 blah blah blah.
  1.3.1.1 Support for external libraries
     UUID loading for mtl+
1.3.2 Modular firmware release
 as it is documented now.

The reason why we are defining the 'modular SOF' is that we don't want multiple UUIDs in the library path (either as files or symlinks to a bundle).

Yes, technically the library loading is going to be used for the openmodules and debug, but the distinction is that their name is hardwired, defined. Whereas the external libraries are loaded on demand based on UUID.

Copy link
Contributor

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming is hard. Some comments inline...

- PLAT = mtl, lnl, ...
Linux SOF will look up firmware files at the following paths.

Look-up paths per Intel platform for **mopnolithic releases**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure world is ready for mopnolithic releases yet...?

+-----------------------------------------------------------+------------------------------------------------+-------------------------+--------------------+
|Platform |Load path |File name |Notes |
+===========================================================+================================================+=========================+====================+
|Meteor Lake and newer |/lib/firmware/intel/sof-ipc4/PLAT/ || |PLAT = mtl, lnl, ...|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already know we won't support this for MTL, LNL, so why mention these at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't say never... Technically it is possible.

@ujfalusi ujfalusi force-pushed the pr/intel_ipc4_baselibs_with_basefw_01 branch from 6cd4788 to 7d5c67e Compare December 17, 2024 11:52
@ujfalusi
Copy link
Contributor Author

Changes since v1:

  • fix typo mopnolithic -> monolithic
  • Clarify that the modular release is PTL+ targeted (Moving MTL/LNL to this mode can break users system with older kernels)

|Panther Lake and newer (community signed) |/lib/firmware/intel/sof-ipc4/PLAT/community/ | | |
+------------------------------------------------------------+------------------------------------------------+-------------------------+ -- |
|Panther Lake and newer Loadable libraries |/lib/firmware/intel/sof-ipc4-lib/PLAT/ |UUID.bin | |
+------------------------------------------------------------+------------------------------------------------+ | -- |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi Also, why not just put UUID.bins to the main directory? The UUID namespace is unique by definition, so what do we gain by putting these to sof-ipc4-lib versus sof-ipc4?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find the thread on why we ended up with a separate directory for the libraries, it is mentioned here at least: thesofproject/linux#3826 (comment)

I think it has something to do with the fact that libraries can be signed with a vendor key and in that case they cannot be alongside of the Intel or community signed basefw, but the basefw can be only signed with Intel or community key.

But this has been the rule ever since we have the UUID based library lookup. Initially I wanted to toss them alongside of the basefw, but then we have changed direction and there were a good reason to do that (but what? I cannot recall).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, thanks @ujfalusi . So this goes to "water under the bridge" category, let's stick to what we've done already and be consistent with previous IPC4 FW releases.

kv2019i
kv2019i previously approved these changes Dec 17, 2024
Copy link
Contributor

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some grumblings on "monolithic release" term, but I don't have better alternatives, so +1 for this.

|Panther Lake and newer (community signed) |/lib/firmware/intel/sof-ipc4/PLAT/community/ | | |
+------------------------------------------------------------+------------------------------------------------+-------------------------+ -- |
|Panther Lake and newer Loadable libraries |/lib/firmware/intel/sof-ipc4-lib/PLAT/ |UUID.bin | |
+------------------------------------------------------------+------------------------------------------------+ | -- |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, thanks @ujfalusi . So this goes to "water under the bridge" category, let's stick to what we've done already and be consistent with previous IPC4 FW releases.

See :ref:`loadable-libraries` for details about library support in general.
The release contains:
- **sof-PLAT.ri** : The firmware binary
- **UUID.bin** : Mainly 3rd party libraries identified by UUID. If the library contains multiple modules then a UUID symlink must be provided for each one. Optional, only supported with IPC4 and on Meteor Lake and newer platforms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I now get approach. So you want to keep this generic so that driver can detect the scheme used and in theory if sof-bin ever releases a modular release layout for MTL, kernel could pick it up (unlikely as we don't want to break systems with older kernels, but alas, technically this could be done -- it's just a question of release logistics and dependencies then).

I still don't like the "monolithic release" name, but I like "modular release" I can't come with a better alternative.

@ujfalusi ujfalusi force-pushed the pr/intel_ipc4_baselibs_with_basefw_01 branch from 7d5c67e to 155fa2a Compare December 17, 2024 16:40
@ujfalusi
Copy link
Contributor Author

Changes since v2:

  • rename the monolithic to single firmware.

@ujfalusi
Copy link
Contributor Author

ujfalusi commented Dec 17, 2024

What about this naming:
1.3.1 Single firmware release + UUID on demand library support on MTL+
1.3.2 Split firmware release (basefw+openmodules+debug) + UUID on demand library support

instead of the Single/Modular.

@ranj063
Copy link
Contributor

ranj063 commented Dec 18, 2024

What about this naming: 1.3.1 Single firmware release + UUID on demand library support on MTL+ 1.3.2 Split firmware release (basefw+openmodules+debug) + UUID on demand library support

instead of the Single/Modular.

or how about unified/fragmented? I'm good with single/split too.

@ujfalusi ujfalusi force-pushed the pr/intel_ipc4_baselibs_with_basefw_01 branch from 155fa2a to 0576be4 Compare December 18, 2024 07:48
…elease

Separate the firmware lookup table for non-modular and modular SOF releases
to be able to document the locations and file names the firmware will
be looking for the individual files.

Extend the description of the two type of SOF release and convert the
list-table to a normal table for better descriptions for the
configurations.

Signed-off-by: Peter Ujfalusi <[email protected]>
@ujfalusi
Copy link
Contributor Author

Changes since v3:
new terms to use: Non-modular and Modular SOF:
non-modular: No loadable library support (IPC3, IPC4 prior to MTL)
modular: supports library loading, two distribution mode: single firmware and split firmware

@lyakh
Copy link
Contributor

lyakh commented Dec 18, 2024

What about this naming: 1.3.1 Single firmware release + UUID on demand library support on MTL+ 1.3.2 Split firmware release (basefw+openmodules+debug) + UUID on demand library support

instead of the Single/Modular.

@ujfalusi I like it! I was about to propose something like multi-part / single-part / modular, but split is ok too!

Copy link
Contributor

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better!

@lgirdwood lgirdwood merged commit 6eee88f into thesofproject:master Dec 20, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants