Support SPINOR + NVMe with %include and multi-disk by lool · Pull Request #90 · qualcomm-linux/qcom-ptool

lool · 2026-03-24T15:01:37Z

Add %include support for partitions.conf files and support for multi-disk partitions.conf files; allows supporting SPINOR + NVMe scenarios properly, and reducing duplication across files.

gen_partition: refactor line reading logic
gen_partition: add %include directive support
gen_partition: add multi-disk support
tests: integration tests for %include and multi-disk
gen_contents: prefix file paths for multi-disk
Makefile: unified stamp-based build for all platforms
glymur-crd: add spinor-nvme combo subdir
iq-x7181-evk: add spinor-nvme combo subdir
qrb2210-unoq: dedup partition configs with %include
check-missing-files: handle multi-disk file paths
Makefile: run tests on specific XML filenames at any depth

Fixes: #86

lool · 2026-03-24T21:10:24Z

Ready for review!

I've obviously built this and ran make check. The output matches my expectation for glymur-crd/spinor-nvme.

Impact on downstream projects I'm somewhat familiar with:

meta-qcom: seems to call make to build all targets, and then recurses to copy all interesting files from subfolders that contain a gpt_main0.bin, so should work the same, except perhaps for flashing
qcom-deb-images: actually parses partitions.conf files, so needs to be adapted; probably it should patch output XML files instead, or use symlinks

Potential future improvements:

add SAIL SPINOR partition lists
share partition lists between ufs and emmc implementations
use separate partitions lists for boot firmware and HLOS

lool · 2026-03-24T21:11:25Z

Ah: there's also now an opportunity to share contents.xml templates between platforms, and to generate all partition entries there rather than just the ones for the default storage (which is inconsistent).

ndechesne · 2026-03-25T09:19:42Z

gen_partition.py

+   if filepath in include_stack:
+      raise ValueError("Circular include detected: %s\nInclude stack: %s" % (
+         filepath, " -> ".join(include_stack)))
+   include_stack = include_stack + [filepath]


.append feels more natural. any reason why you did not use it?

There is! It's actually because I need to make a copy because of the recursion; added a comment to clarify

ndechesne · 2026-03-25T09:21:47Z

gen_partition.py

+      for raw_line in f:
+         stripped = raw_line.strip()
+         if stripped.startswith('%include '):
+            inc_path = stripped[len('%include '):].strip()


do you need .strip again?

I didn't; removed

ndechesne · 2026-03-25T09:26:00Z

gen_partition.py

+   """
+   if include_stack is None:
+      include_stack = []
+   filepath = os.path.realpath(filepath)


what if filepath does not exist? e.g. with a bad %include statemetn?

Indeed, I've changed the code to test, and added a test for that

ndechesne · 2026-03-25T09:35:01Z

gen_partition.py

+      print("Error: no --disk entry found in %s" % input_file)
+      sys.exit(1)
+
+   if output_xml:


I would prefer something different. we should not add '-d'. then the tool is doing the following:

if there is only 1 disk, we generate the file with what we have in -o

if there are more disks, then we generate files by appending an index to what we have in -o. e.g. if you have -o partitions.xml, you generate partitions0.xml, partitions1.xml, ..

we can even define the index as an attribute in --disk

I'm not sure I like the idea of the file passing --index to --disk, it's yet another way that this could go wrong (two disk statements with the same index, weird index numbers, skipping index numbers etc.).

I would have preferred conveying the storage name somewhere, and we'll need a place for all the generated files to go too (two rawprogram0.xml, two gpt_main0.bin etc.).

I can implement the automatic partitionN.xml generation though.

I've changed to generate partitionN.xml, but then I had to generate subdirs to host the generated files for each disk, so I went with disk0/, disk1/ etc.

I think that's ugly and I preferred the previous version.

ndechesne · 2026-03-25T09:45:13Z

platforms/qrb2210-unoq/emmc-16GB-arduino/partitions.conf

--partition --name=SYSFW_VERSION --size=4KB --type-guid=3C44F88B-1878-4C29-B122-EE78766442A7
--partition --name=efi --size=524288KB --type-guid=C12A7328-F81F-11D2-BA4B-00A0C93EC93B --filename=efi.bin
--partition --name=rootfs --size=10460284KB --type-guid=B921B045-1DF0-41C3-AF44-4C6F280D3FAE --filename=rootfs.img
+%include ../emmc.conf.inc


it seems more natural to %include ../emmc-16GB/partition.conf here

Sure; changed

Simplify logic for readability. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Add a pre-processing step that expands %include directives before parsing partitions.conf files. This allows splitting partition definitions into reusable fragments (e.g. shared NHLOS or HLOS partition lists) that can be included from multiple partitions.conf files. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Allow partitions.conf files to contain multiple --disk sections. Each --disk line starts a new section; partitions following it belong to that disk. Automatically generate partitionN.xml files if more than one disk is listed. This enables a single partitions.conf to describe a complete platform spanning multiple storage types (e.g. SPINOR + NVMe for Glymur CRD). Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Add check-include-multidisk covering various %include and multi-disk scenarios. Call as part of integration tests (integration make target). Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

In multi-disk layouts, partitions.xml files are in per-storage subdirs (e.g. spinor-nvme/spinor/partitions.xml). Set file_path from the relative path between the output directory and the partitions.xml directory. Single-disk layouts where both files are in the same directory are unaffected (prefix is empty). Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Replace the separate partitions.xml, gpt, and contents.xml rules with a single stamp-based rule that runs gen_partition.py -d then ptool.py on each generated partitions.xml. This handles both single-disk and future multi-disk partitions.conf files uniformly. Update gitignore for new .built files. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Add a combined SPINOR + NVMe partition definition for Glymur CRD. Use %include to pull in the existing spinor and nvme partitions.conf files. Move contents.xml.in from spinor/ to spinor-nvme/ since it already references both storage types and update file_path references to use subdirectory names (spinor/, nvme/) instead of relative paths (., ../). The combo subdir generates artifacts for both storage types under spinor-nvme/spinor/ and spinor-nvme/nvme/, making it straightforward for downstream consumers to discover all required storages for this platform. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Add a combined SPINOR + NVMe partition definition for IQ-X7181 EVK. Use %include to pull in the existing spinor and nvme partitions.conf files. Move contents.xml.in from spinor/ to spinor-nvme/ since it already references both storage types and update file_path references to use subdirectory names (spinor/, nvme/) instead of relative paths (., ../). The combo subdir generates artifacts for both storage types under spinor-nvme/spinor/ and spinor-nvme/nvme/, making it straightforward for downstream consumers to discover all required storages for this platform. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Extract the shared eMMC partition layout (disk + 72 partitions) into emmc.conf.inc. Both emmc-16GB and emmc-16GB-arduino now include it, with the arduino variant appending only the extra userdata partition. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Use a proper XML parser (python3 xml.etree, part of stdlib) instead of sed to extract file_name and file_path from contents.xml, combining them into relative paths. Match known filenames against the basename so that files in storage-type subdirectories (e.g. spinor/gpt_main0.bin) are recognized correctly, while checking existence with the full path. Don't check presence of sail_nor/ files that ptool might generate as these are not currently generated. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

Replace static glob (platforms/*/*/*.xml) with find to discover XML files at any depth. This is needed for multi-disk use cases that place generated files in per-storage subdirectories. Take this opportunity to use specific filenames that we can scan. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

nivasnaik · 2026-03-31T03:35:31Z

platforms/iq-x7181-evk/spinor-nvme/partitions.conf

+# Combined NVMe + SPINOR partition layout for IQ-X7181 EVK.
+# NHLOS firmware on SPINOR, HLOS (Linux) on NVMe.
+
+%include ../spinor/partitions.conf


How would this solve the problem of which storage is for NHLOS and which one is for HLOS? Will adding a conf file here help to make that decision? For example, qcom-deb-image can use that to unzip boot binaries in that storage sub folder and copy the HLOS binaries to HLOS storage folder.

That's a really good question; I do think we should try to solve this, but I'm not quite sure how. In the above comment you quote, I'm describing a layout where 100% of HLOS partitions/payloags go to NVMe and 100% of NHLOS go to SPINOR. In reality, it's often a bit more nuanced than that:

SAIL: we often have parts that we consider NHLOS go to SPINOR, and other parts go to UFS or eMMC

persist partition: HLOS might want to manage its main partitions (ESP/rootfs) in e.g. UFS LUN0, while managing persist partition in another LUN or in SPINOR

There's a similar question with CDT which is a separate download and which we also need to route to specific storage.

In qcom-deb-images, there's logic to iterate over all needed files, but TBH I don't think that's very scalable and I'd prefer addressing this in ptool too.

Would you have any thoughts on how we could capture that?

Personally, I've been toying with the idea that ptool should help with integration of boot binaries too, but I'm curious what you think!

This is my thought, below could be one way of handling this problem but I am sure this not the best solution.
If an SoC supports a combination of storages for binaries, there will be conf file for that particular combination like for example spinor-nvme/partitions.conf‎. In addition to including partition.conf of respective storage type, can we also include the spinor-nvme-mapping.conf file?
The format of this file could be ?
spinor-nvme-mapping.conf
SPINOR: NHLOS_All_Bins
NVME: HLOS_Bins

If a single storage type includes all binaries then this mapping file should not be defined.
For SAIL case this mapping file could be like below.
ufs-spinor-mapping.conf
UFS: HLOS_NHLOS_NoSail_Bins
SPINOR: NHLOS_Sail_Bins

All these Keywords SPINOR, UFS, NVME, HLOS_Bins, NHLOS_All_Bins, HLOS_NHLOS_NoSail_Bins, NHLOS_Sail_Bins etc must be defined in a meta-data file.

Then projects like qcom-deb-image can create meta-data using this *mapping.conf file.

Does this makes sense?

I mean to say can create flat_images using mapping.conf and other files generated from qcom-ptool.

IIUC, you're saying:

we add new spinor+nvme config directories in ptool (as proposed here)

we add new metadata in these dirs to document that data for this or that storage should come from this or that project (NHLOS boot binaries or HLOS)

ptool generates a tree of storage/*.{xml,bin,...} files

downstream projects like qcom-deb-images and meta-qcom can use the tree of files + new metadata to know what to copy where

Is that right?

Yes, otherwise there is no common spec which describes how to create a flat-meta, every project is creating flat-meta independently with out a defined spec or process.

meta-data can be common, it just describes the meaning of keywords used in mapping file. *-mapping file describes what binaries each storage will contain.

So I agree we need a design for this too; while we're on that topic, I think we also need to fix the contents.xml files to be more completely generated from this data (or use contents.xml files as an input and drop our alternate more human friendly formats if that's preferred).

I'm a bit worried that this PR is already not progressing due to the complexity / impact of the changes, but perhaps it would be easier if we had the full plan. @ndechesne do you want to chime in on how to get this merged?

meta-data can be common, it just describes the meaning of keywords used in mapping file. *-mapping file describes what binaries each storage will contain.

So while I generally agree with the requirements you describe, I personally would prefer avoiding the proliferation of new unspecified formats. There are already a lot of formats / intermediate files with contents.xml. partitions.conf, partitions.xml, rawprogram*.xml... and many are poorly documented.

I've toyed with the idea of having a more structured partitions.yaml or similar that would revisit the partitions.conf syntax and address all the highlighted issues in this thread and a clear ptool frontend command-line tool that would take care of everything in one pass. This is a larger redesign though, and it would require some buy-in before we start making the changes.

lool force-pushed the includes branch from e3f973a to 2f4dab7 Compare March 24, 2026 20:08

lool changed the title ~~Add %include~~ Support SPINOR + NVMe with %include and multi-disk Mar 24, 2026

lool force-pushed the includes branch from 2f4dab7 to 5b9e4b0 Compare March 24, 2026 20:37

lool marked this pull request as ready for review March 24, 2026 20:54

lool requested review from lumag, ndechesne, sbanerjee-quic and vkraleti as code owners March 24, 2026 20:54

ndechesne reviewed Mar 25, 2026

View reviewed changes

gen_partition: refactor line reading logic

a13a8ce

Simplify logic for readability. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

lool mentioned this pull request Mar 25, 2026

Support for NVME + SPINOR flatmeta generation qualcomm-linux/qcom-deb-images#321

Closed

lool added 10 commits March 25, 2026 19:43

tests: integration tests for %include and multi-disk

ddd96d7

Add check-include-multidisk covering various %include and multi-disk scenarios. Call as part of integration tests (integration make target). Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>

lool force-pushed the includes branch from 5b9e4b0 to 774ff89 Compare March 25, 2026 18:43

nivasnaik reviewed Mar 31, 2026

View reviewed changes

Conversation

lool commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lool commented Mar 24, 2026

Uh oh!

lool commented Mar 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lool Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lool commented Mar 24, 2026 •

edited

Loading

lool Apr 2, 2026 •

edited

Loading