Define QCOM_XBL_CONFIG in machine conf for XBL config selection#2289
Define QCOM_XBL_CONFIG in machine conf for XBL config selection#2289vkraleti wants to merge 2 commits into
Conversation
Test Results 103 files ± 0 632 suites - 1 5h 10m 48s ⏱️ - 1h 40m 36s For more details on these failures, see this check. Results for commit 8a9f743. ± Comparison against base commit e89eaea. This pull request removes 5 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
lumag
left a comment
There was a problem hiding this comment.
Replace ci/qcom-distro-kvm.yml with a new ci/kvm.yml KAS
Why? What is the issue that you are trying to solve? I was under assumption that the machine configs are going to have the feature enabled. Now you are adding a config fragment to enable it.
Not all machine are ready to switch to KVM. Only Lemans and Monaco are tested and ready to switch. For Kodiak, Talos, Hamoa, Purwa meta-qcom still need to provide a parallel build for KVM verifications while continuing regular testing with Gunyah. |
Commit message. |
|
Updated commit messages. #2251 is the follow‑up PR to enable KVM by default for Lemans. Changes for Monaco, Kodiak, and Talos will be submitted shortly in separate PRs. |
| distro: qcom-distro-kvm | ||
| local_conf_header: | ||
| kvm: | | ||
| MACHINE_FEATURES:append = " kvm" |
There was a problem hiding this comment.
Having non-machines poking at MACHINE_FEATURES is a very bad idea
There was a problem hiding this comment.
Yeah. But is there a better way to have two builds with and without a feature? We must have two builds with and without KVM till all SoCs are ready to migrate.
There was a problem hiding this comment.
@koenkooi @lumag @ricardosalveti are you aligned to this approach? If not, is it fine to restore qualcomm-linux/meta-qcom-distro@32e7492 `
# Enable KVM distro feature if corresponding machine feature is set.
# This allows users to enable KVM either via MACHINE_FEATURES or DISTRO_FEATURES.
DISTRO_FEATURES:append = "${@bb.utils.contains('MACHINE_FEATURES', 'kvm', ' kvm', '', d)}"
Test teams are expecting KVM builds in CI for boards that are fully not ready.
There was a problem hiding this comment.
No, setting DISTRO_FEATURES based on MACHINE_FEATURES is a clear NAK.
There was a problem hiding this comment.
Ok, how about adding a check in image_types_qcom.bbclass to check for kvm either in machine or distro features?
if ${@bb.utils.contains('MACHINE_FEATURES', 'kvm', 'true', 'false', d)} || \
${@bb.utils.contains('DISTRO_FEATURES', 'kvm', 'true', 'false', d)}; then
xbl_config="xbl_config_kvm.elf"
fi
There was a problem hiding this comment.
What for? What does it logically mean? Should it be instead if ${bb.utils.contains('COMBINED_FEATURES', 'gunyah', 'true', 'false'} ; then ...
There was a problem hiding this comment.
After exploring additional options, seems this transition needs to happen in incremental steps as listed below:
-
Move XBL config selection logic from image_types_qcom.bbclass to the respective machine .conf files.
This PR Define QCOM_XBL_CONFIG in machine conf for XBL config selection #2289 is updated for this change. -
Introduce a Gunyah-specific distro to clearly model the alternate hypervisor use case.
Updated Add Gunyah-specific distro configuration meta-qcom-distro#306 -
For KVM-ready machines, update XBL config selection based on the Gunyah distro. Updated PR Enable KVM by default on Lemans and Monaco Platforms #2251
-
Once all machines are migrated, drop the KVM distro feature and standardize on the Gunyah distro for alternate hypervisor builds.
…BL selection Avoid hardcoding XBL configuration selection in image_types_qcom.bbclass, which limits flexibility to enable KVM selectively across boards. Define QCOM_XBL_CONFIG in machine configuration files to select the appropriate XBL config based on DISTRO_FEATURES. This enables per-board control of XBL configuration when KVM is enabled. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
|
I find this all a bit confusing, and we are not really improving the current state of things here. The configuration nobs we have in the end:
Now for gunyah, do we need any distro specific policy to support it to the same way we support KVM? If indeed the case, then we should indeed have multiple distros and pick one by default (KVM). Here COMBINED_FEATURES would work better when deciding the XBL to flash (and done in a common file, not in every machine conf), but for this to work we would have to define a KVM/Gynuah machine feature. |
|
Yes, Gunyah needs to be another distro feature, otherwise managing builds becomes really hard. qualcomm-linux/meta-qcom-distro#306 is defining this. Once this distro feature is available, machine configurations can start making decisions like To convert KVM to a combined feature, first machine configs need to have the control on XBL config installations. Current PR (#2289) provides this control. Once this and gunyah distro feature are available, `kvm' can be added to DISTRO_FEATURES by default to facilitate combined feature checks. Post that machine confs can define 'kvm' as a machine feature and convert the XBL config installation checks to use 'kvm' COMBINED_FEATURE. All this juggling is to avoid CI breakages and to continue providing a secondary build for alternate hypervisor testing. |
…BL selection Avoid hardcoding XBL configuration selection in image_types_qcom.bbclass, which limits flexibility to enable KVM selectively across boards. Define QCOM_XBL_CONFIG in machine configuration files to select the appropriate XBL config based on DISTRO_FEATURES. This enables per-board control of XBL configuration when KVM is enabled. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
|
If you agree for a brief CI downtime, I can make changes to add |
|
@vkraleti I'd rather not. If something fails, we end up with broken CI or broken config. |
Avoid hardcoding XBL configuration selection in image_types_qcom bbclass, which limits flexibility to enable KVM selectively across boards. Define QCOM_XBL_CONFIG in machine configuration files to select the appropriate XBL config based on `kvm` in COMBINED_FEATURES. This enables per-board control of XBL configuration selection. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
Adjust CI to handle removal of qcom-distro-kvm configuration in meta-qcom-distro, where KVM is now enabled by default. Replace usage of the qcom-distro-kvm distro with explicit MACHINE_FEATURES and DISTRO_FEATURES additions to retain KVM-enabled builds during the transition. This is a temporary workaround until CI fully adopts the new KVM-by-default model. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
…BL selection Avoid hardcoding XBL configuration selection in image_types_qcom bbclass, which limits flexibility to enable KVM selectively across boards. Define QCOM_XBL_CONFIG in machine configuration files to select the appropriate XBL config based on `kvm` in COMBINED_FEATURES. This enables per-board control of XBL configuration selection. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
…stro-kvm Adjust CI to handle removal of qcom-distro-kvm configuration in meta-qcom-distro, where KVM is now enabled by default. Replace usage of the qcom-distro-kvm distro with explicit MACHINE_FEATURES and DISTRO_FEATURES additions to retain KVM-enabled builds during the transition. This is a temporary workaround until CI fully adopts the new KVM-by-default model. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
|
Ok. PRs should be merged in following order to make the switch.
|
Avoid hardcoding XBL configuration selection in image_types_qcom.bbclass, which limits flexibility to
enable KVM selectively across boards.
Define QCOM_XBL_CONFIG in machine configuration files to select the appropriate XBL config based on DISTRO_FEATURES. This enables per-board control of XBL configuration when KVM is enabled.