firmware-qcom-capsule: stop hamoa's dtb entry leaking to other machines - #3079
Michael Scott (mike-scott) wants to merge 1 commit into
Conversation
|
NOTE: this is a cherry-pick Igor Opaniuk (@igoropaniuk) fix for the Hamoa DTB entry leak fix from: |
Test Results 117 files - 2 709 suites - 6 8h 21m 46s ⏱️ - 1h 5m 11s For more details on these failures, see this check. Results for commit 0d6b766. ± Comparison against base commit c672971. This pull request removes 2 tests.♻️ This comment has been updated with latest results. |
|
Milosz Wasilewski (@mwasilew) Dmitry Baryshkov (@lumag) I can see that 1 check is failing, but I'm not sure if there's an action item to fixing the 33 fails: https://github.com/qualcomm-linux/meta-qcom/pull/3079/checks?check_run_id=100894475989 What do you recommend? |
I meant there are errors in the following tests, that I don't think my change had anything to do with
NOTE: There are some failures on the
But I'm not sure if I should be researching these? There are 3 of the pre-merge jobs for
|
|
We are having some issues with some of the tests, retrying them. |
CAPSULE_FLASH_TYPE and CAPSULE_ENTRIES definitions are machine-qualified. The CAPSULE_ENTRY_dtb[...] flags are not. Varflags don't work with override resolution. This: CAPSULE_ENTRY_dtb[dest_disk]:iq-x7181-evk does not exist. Currently, the CAPSULE_ENTRY_dtb entries apply to every machine. Any other board that declares a "dtb" capsule entry ends up inheriting hamoa's SPINOR destinations without any warning to the user. Nothing catches it: generate_fvupdate() checks that an entry has a binary, a dest_disk and a dest_partition, all of which hamoa's values supply. The build succeeds and produces a capsule aimed at storage the machine may not have, or worse: may use for a different purpose and will be overwritten during a rare capsule update event. Instead of guards or other attempts to rename the "dtb" entry to something more unique for the machine, let's relocate these settings to iq-x7181-evk.conf where they won't affect anyone else. Fixes: a314263 ("firmware-qcom-capsule: add iq-x7181-evk capsule entry definitions") Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com> Signed-off-by: Michael Scott <mike@foundries.io>
8580b36 to
0d6b766
Compare
|
Change of direction: add the CAPSULE_ related variables to the machine.conf I had to make a few changes to the meta-qcom repo in order to test this change:
Step 1: verify the CAPSULE_ENTRY_dtb entries are valid for iq-9075-evk: At the build prompt: Step 2: verify the CAPSULE_ENTRY_dtb entries are valid for iq-x7181-evk: At the build prompt: |
|
|
||
| CAPSULE_GUID = "0F6D58FC-2258-4D27-9E23-D77219B0897C" | ||
| CAPSULE_FLASH_TYPE = "NORUFS" | ||
| CAPSULE_ENTRIES = "dtb" |
There was a problem hiding this comment.
Ricardo Salveti (@ricardosalveti) I can make these ?= if we want to make it easier to change in product overlay layers
There was a problem hiding this comment.
This should be fine, unless we expect this to be customized.
| CAPSULE_ENTRY_dtb[dest_guid] = "{2A1A52FC-AA0B-401C-A808-5EA0F91068F8}" | ||
| CAPSULE_ENTRY_dtb[backup_disk] = "SPINOR" | ||
| CAPSULE_ENTRY_dtb[backup_partition] = "dtb_BACKUP" | ||
| CAPSULE_ENTRY_dtb[backup_guid] = "{A166F11A-2B39-4FAA-B7E7-F8AA080D0587}" |
There was a problem hiding this comment.
One other issue found is that CAPSULE_ENTRY_dtb is not in do_compile's hash, so changes here would not trigger a rebuild.
We would also need something like in qcom-capsule.bbclass:
generate_fvupdate[vardeps] += "${@' '.join('CAPSULE_ENTRY_' + e for e in d.getVar('CAPSULE_ENTRIES').split())}"
There was a problem hiding this comment.
Hm. This can probably be a separate PR or an additional patch on Igor Opaniuk (@igoropaniuk) 's #3043.
Would it stop this PR from moving forward? I think Igor Opaniuk (@igoropaniuk) can take a look?
There was a problem hiding this comment.
Add a new commit with the suggested line, then we can merge, as it #3043 might take longer until it is ready to be merged.
6 participants
CAPSULE_FLASH_TYPE and CAPSULE_ENTRIES next to these definitions are machine-qualified; the CAPSULE_ENTRY_dtb[...] flags beside them are not, and cannot be -- varflags take no part in override resolution, so CAPSULE_ENTRY_dtb[dest_disk]:iq-x7181-evk does not exist.
They therefore apply on every machine. Any other board that declares a "dtb" capsule entry inherits hamoa's SPINOR destinations, and nothing catches it: generate_fvupdate() only checks that an entry has a binary, a dest_disk and a dest_partition, all of which hamoa's values supply. The build succeeds and produces a capsule aimed at storage the machine may not even have.
Guarding on MACHINEOVERRIDES gives the flags the scope the neighbouring overrides already have. Renaming the entry would also work, but the class keys the kernel dependency on the literal name "dtb".
Fixes: a314263 ("firmware-qcom-capsule: add iq-x7181-evk capsule entry definitions")