Physical partitions for emmc#66
Conversation
Physical partitions were a feature limited to UFS storage, allow --lun flag in partitions.conf whatever the storage type, and generate partitions.xml with multiple physical_partitions when used anywhere. This is useful at least on eMMC, e.g. using physical partition 1 to program boot0. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>
Add a --phys-part flag as an alias to --lun. LUN is an UFS specific concept while physical partition is what the partitions.xml files use. The latter makes sense e.g. on eMMC as well in some cases. Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>
Rewrite partitions.conf based on rawprogram0 and rawprogram1.xml files from upcoming boot binaries for emmc on QCS8300. The list of partitions and their sizes are essentially the same, except: - a second physical partition is introduced for the cdt - ordering of partitions changes in first physical partition, with eMMC-specific partitions first, boot partitions next, then firmware partitions and finally HLOS partitions - ALIGN_TO_128K_1 is dropped – shouldn't be needed Signed-off-by: Loïc Minier <loic.minier@oss.qualcomm.com>
|
Currently untested, I'm not clear on whether last_parti is needed at all or needed on both physical parts. |
|
@loicpoulain Could you help review? |
|
This will probably also make sense on RB1 and UNO Q. |
|
@loicpoulain I tested the changes with linux-next, but I'm not able to boot, I suspect I also need some dtb tweaks. This is my log: |
| --partition --name=xbl_a --size=5120KB --type-guid=DEA0BA2C-CBDD-4805-B4F9-F428251C3E98 --filename=xbl.elf | ||
| --partition --name=xbl_b --size=5120KB --type-guid=7A3DF1A3-A31A-454D-BD78-DF259ED486BE --filename=xbl.elf | ||
| --partition --name=xbl_config_a --size=512KB --type-guid=5A325AE4-4276-B66D-0ADD-3494DF27706A --filename=xbl_config.elf | ||
| --partition --name=shrm_a --size=80KB --type-guid=CB74CA22-2F0D-4B82-A1D6-C4213F348D73 --filename=shrm.elf |
There was a problem hiding this comment.
I cannot leave a comment on the commit message, so adding it here.
can you please clearly say which version of the boot binaries you used as a reference, it might be useful to keep that in the history
There was a problem hiding this comment.
I've used an internal boot binaries build that @loicpoulain shared with me, I'll share it with you
There was a problem hiding this comment.
@ndechesne I've compared this partitions.conf with the partition_emmc.xml in the QCS8300 boot binaries (r1.0_00114.0) and they match except for these differences:
- HLOS partition is called system / system.img in .xml, and rootfs / rootfs.img in .conf (matches other boards)
- apdp_a/b partitions have no filename in .xml, but point to zeros_33sectors.bin in .conf (matches other boards)
- efi and persist patitions are at start of list in .xml, but at the end in .conf (matched other boards)
- GUID of persist partition is 0FC63DAF-8483-4772-8E79-3D69D8477DE4 in .xml and
6C95E238-E343-4BA8-B489-8681ED22AD0B in .conf (matches other boards)
The last one is interesting, the .xml GUID makes more sense to me than the unknown one used in .conf (https://uapi-group.org/specifications/specs/discoverable_partitions_specification/) but I'd still stick to the same GUID as other boards as I believe persist is a Yocto specific HLOS partition.
There was a problem hiding this comment.
persist is a partition used in QTEE (TZ) to store its encrypted data. not sure we can consider it as an HLOS partition. and it is not always at the end of the emmc (it is always on the last LUN on UFS, that is by design).
we seem to be using 0FC63DAF-8483-4772-8E79-3D69D8477DE4 for the qcs615 (talos) boards. we should check if there is a GUID that is strictly required, since we are spotting an inconsistency here.
There was a problem hiding this comment.
I briefly researched this, IIUC QTEE has different dependencies on storage. There's notably some RPMB usage, but we don't currently attempt to provision RPMB from ptool reference files (we could), and "secure file system" (SFS).
SFS is actually accessed through HLOS, even if QTEE handles the encryption itself. The /persist path can be anything that the HLOS choses, and it's typically a regular fs.
We are currently discussing adding a persist partition to meta-qcom:
qualcomm-linux/meta-qcom#1269
qualcomm-linux/meta-qcom#1484
but as you can see in this discussion, it's an HLOS specific integration, and we don't necessarily need a separate partition in the first place, so I think the HLOS classification makes sense in partitions.conf.
There was a problem hiding this comment.
@harshaldev27 which means that the keys should never be stored on a partition that anything writes after the provision.
Looks like I over-focused on the one-time provisioned use-case. Apart from this, both OPTEE and QTEE use /var/lib/tee for storing per Trusted Application GP Persistent Objects as per the TEE Core API specification. So we do need to be able to constantly write to this partition while use-cases are running.
HDCP keys and DRM keys are just one type of GP Persistent Objects even though they are created and stored once for the lifetime of the device.
All in all, we are asking for the same thing that OPTEE requires. The only difference is that for us, integrity of the contents of /var/lib/tee and minimizing the risk of corruption is of added importance, which is why we are requesting for the burden of increased complexity of a separate partition to be accommodated.
There was a problem hiding this comment.
I'm sorry, not "on the fly", of course. I meant that if we detect that persist partition is damaged or doesn't contain keys (or something else), it should be reinited automatically during the boot from the separately stored HDCP / DRM keys.
There was a problem hiding this comment.
That's correct, this behavior is already implemented by the Secure File System as per the TEE Internal Core API specification,
"Any function that accesses a persistent object handle MAY return a status of
TEE_ERROR_CORRUPT_OBJECT or TEE_ERROR_CORRUPT_OBJECT_2, which indicates that corruption
of the object has been detected. Before this status is returned, the implementation SHALL delete the
corrupt object and MAY close the associated handle"
So yes, in case of corruption, we do 'reinit' or 'RESET' the object. But mind you, before doing so, we do make an attempt to recover from a backup version of the object. So like I said, we make all sorts of efforts to avoid corruption both in our implementation of the Secure File System, as well as in the properties of the Persist partition (being separate from rootfs).
There was a problem hiding this comment.
By the way, the 'backup' versions are also stored in the /var/lib/tee, not in a separate storage. That's just the design choice we have, since we can't keep asking for multiple partitions.
There was a problem hiding this comment.
(We should probably continue discussing in the meta-qcom issue or PR)
I understand the idea of precious data that we want to backup on device, perhaps in a partition not managed by the HLOS. I don't find the design of a singled out partition for TEE particularly elegant, but it's also pragmatic.
I've been thinking that ptool ought to have a way to mix:
- boot firmware partition requirements
- HLOS partition requirements
And in this case, it almost feels like a third use case, a little bit like the Arduino one:
- HLOS use case partition requirements (userdata partition for Arduino, persist partition for TEE)
| generate_single_disk_xml(disk_params, partition_entries_dict, output_xml) | ||
| elif disk_params["type"] == "ufs": | ||
|
|
||
| if disk_params["type"] in ("emmc", "nvme", "spinor", "ufs"): |
There was a problem hiding this comment.
this is a really nice change. this code gives me headache each time i need to read it.. we probably should clean up even further (not in this PR) the generate_multi_lun_xml() function. you might have noticed that we are doing the main loop 6 times even there is just 1 or 2 physical partitions..
but for the purpose of this PR, I am +1
I also generated all partitions xml file before/after this PR, and they are unchanged (expected), except for the 8275 EVK of course.
|
I was able to boot test this on Monaco EVK (RB4) with DIP switches set to boot from eMMC, linux-next, and a slightly tweaked Device Tree from @loicpoulain |
|
Nice, LGTM |
|
@lool note, we were not using |
Should we though? :) |
This adds support for physical partitions on eMMC and makes use of it to
provide a rawprogram1.xml with CDT on IQ-8275 EVK: