Switch to KVM hypervisor on Rb3Gen2-Core-Kit and QCM6490-IDP - #2879
Switch to KVM hypervisor on Rb3Gen2-Core-Kit and QCM6490-IDP#2879Viswanath Kraleti (vkraleti) wants to merge 5 commits into
Conversation
7d56df9 to
a0d5c14
Compare
Test Results 119 files + 38 715 suites +289 10h 17m 50s ⏱️ + 2h 51m 31s For more details on these failures, see this check. Results for commit 62eb8a7. ± Comparison against base commit 1efd355. ♻️ This comment has been updated with latest results. |
|
|
||
| local_conf_header: | ||
| gunyah: | | ||
| MACHINE_FEATURES:remove = "kvm" |
There was a problem hiding this comment.
It's no a distro configuration. Changing it in the distro-like Kas fragment is invalid.
There was a problem hiding this comment.
Renamed the config as ci/gunyah.yml and removed distro include so that, any machine+distro combination can be build with Gunyah.
| - machine: iq-615-evk | ||
| distro: | ||
| name: qcom-distro-gunyah | ||
| yamlfile: ':ci/qcom-distro-gunyah.yml' |
There was a problem hiding this comment.
We are not changing the distro here, so this is incorrect.
There was a problem hiding this comment.
qcom-distro in the name is to indicate the build is with gunyah and qcom-distro. It can't be called as qcom-distro+gunyah, as having any other separator apart from hypen(-) / underscore(_) caused issues while copying to S3 in the past.
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-el2 kodiak-staging" | ||
|
|
||
| FIT_DTB_COMPATIBLE[qcom_qcs5430-iot-el2kvm] = \ | ||
| FIT_DTB_COMPATIBLE[qcom_qcs5430-iot] = \ |
There was a problem hiding this comment.
This will break users switching from QLI 2.0 to QLI 2.1 or master without erasing UEFI variables. This doesn't sound good.
There was a problem hiding this comment.
But is there any option to clear efivars? Even for L,M, and T this was an agreed short coming and users need to perform one time efivars cleanup.
There was a problem hiding this comment.
The switch itself is already something that is not really ideal to push as part of 2.x, as this is a major change in the end.
The user will have to remove efivars, update both the OS and the firmware via capsule-updates, and hopefully it will work :-)
c9890b0 to
d3973df
Compare
|
Dmitry Baryshkov (@lumag) Ricardo Salveti (@ricardosalveti) KVM enablement for QCS6490 is planned for the upcoming release, and we are running short on time to complete the full L4 test cycle. Could you please review this PR and let me know if you see any issues or required modifications? As you are already aware, supporting both hypervisors is being tracked in another PR, on which we can continue discussions for a clean switching solution. |
We have two different items here. KVM enablement and KVM being a default. Could we separate them? I don't think we require KVM being a default for completing L4 testing? |
Commitment for Sep release is to switch to default KVM and perform complete L4 testing on KVM. |
This means it must be implemented correctly. No shortcuts, |
d3973df to
b6c9efc
Compare
b6c9efc to
373420d
Compare
|
Dmitry Baryshkov (@lumag) Ricardo Salveti (@ricardosalveti) as discussed over the call with sahitya-tummala, I updated the commit messages in the current PR and also created #3052 to add Gunyah support in CI. Can you please review? |
| "qcs6490-rb3gen2 qcs6490-rb3gen2-industrial-mezzanine kodiak-el2 qcs6490-rb3gen2-industrial-mezzanine-staging" | ||
| FIT_DTB_COMPATIBLE[qcom_qcs6490-iot-subtype9-staging] = \ | ||
| "qcs6490-rb3gen2 qcs6490-rb3gen2-industrial-mezzanine kodiak-staging qcs6490-rb3gen2-industrial-mezzanine-staging" | ||
| "qcs6490-rb3gen2 qcs6490-rb3gen2-industrial-mezzanine kodiak-el2 qcs6490-rb3gen2-industrial-mezzanine-staging" |
There was a problem hiding this comment.
kodiak-staging removed on both lines here.
|
linux-yocto builds will boot without the el2 overlay and with the kvm firmware, that will be broken (lemans is also broken similarly today). |
|
It seems it is not really working correctly on rb3gen2, ADSP seems to be failing, and iris seems to be giving errors as well. |
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-staging" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-el2kvm] = "qcm6490-idp kodiak-el2" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp] = "qcm6490-idp kodiak-el2" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-el2 kodiak-staging" |
There was a problem hiding this comment.
Don't reorder the lines. It's impossible to review your changes. If you want to retain a certain order, split the reordering into a separate commit.
There was a problem hiding this comment.
Broke this commit into two, for easy review.
24d0264 to
37cc3bb
Compare
|
|
||
| # ---------- glymur ---------- | ||
| FIT_DTB_COMPATIBLE[qcom_glymur-crd-staging] = "glymur-crd glymur-staging" | ||
| FIT_DTB_COMPATIBLE[qcom_glymur-crd-camx] = "glymur-crd glymur-crd-camx" |
There was a problem hiding this comment.
What kind of sorting order do you have in mind? In my opinion, camx comes before staging.
| FIT_DTB_COMPATIBLE[qcom_qcs6490-iot-subtype9] = \ | ||
| "qcs6490-rb3gen2 qcs6490-rb3gen2-industrial-mezzanine" | ||
|
|
||
| FIT_DTB_COMPATIBLE[qcom_qcs5430-iot-subtype2] = \ |
There was a problem hiding this comment.
Again, subtype2 < subtype9.
| "qcs6490-rb3gen2 qcs6490-rb3gen2-vision-mezzanine kodiak-staging" | ||
|
|
||
| FIT_DTB_COMPATIBLE[qcom_qcs5430-iot-camx] = \ | ||
| "qcs6490-rb3gen2 qcs5430-fps-camx" |
There was a problem hiding this comment.
These entries got moved. You wrote that you are changing compats. No mention of moving anything
|
|
||
| # ---------- kodiak ---------- | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-staging" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-el2 kodiak-staging" |
There was a problem hiding this comment.
Why is this a separate commit? Why are you adding overlays? Just add -el2gh where applicable. Can you glance at the commit and see what was changed? No. If you just changed the compatible strings, it would have been much easier.
| # | ||
| # Syntax: | ||
| # FIT_DTB_COMPATIBLE[<compatible-encoded>] = "<dtb-stem> [<overlay-stem> [<overlay-stem>...]]" | ||
| # |
There was a problem hiding this comment.
"Foo, in addition bar" means two different actions. Two separate commits.
The issue narrowed down to the devconfig.mbn currently being used. This version doesn't support KVM, preventing the PILs from initializing correctly. |
And how was KVM tested then? |
The issue is with devcfg.mbn, but other devcfg variants its fine and those other devcfg variants are for internal consumption only, not to be released externally. Hence, a new FW bin is being released with the fix to devcfg.mbn, which is the right one to use on Kodiak. The fix is part of #3069. Viswanath Kraleti (@vkraleti) pls rebase your change on that and trigger the CI tests again, these should pass now. Note that the above PR also has boot changes to update efivar with el2gh/el2kvm based on the XBL image flashed. |
This is all fine, but how was this PR tested? Why was it not tested with the Viswanath Kraleti (@vkraleti) should we treat your PRs as completely untested and thus requiring more scrunity? |
Due to HLOS and nHLOS interdependencies (both are developed in different environments) this couldn't be tested with the |
Was this ever mentioned in the PR description or in the commit message? How was this supposed to be merged at all if this PR was opened before the NHLOS update? |
37cc3bb to
457c794
Compare
The el2gh suffix selects the Gunyah hypervisor variant of a board's compatible string, the same way el2kvm selects the KVM variant. Like camx and staging, it is a feature/hypervisor selector rather than board metadata, so it has no corresponding node in qcom-metadata.dtb. test_fitimage_compatible_metadata_validation validates every dash-separated suffix of the generated ITS compatible strings against the metadata node names, and so already fails today on the pre-existing el2gh entries in fit-dtb-compatible-linux-qcom.inc. Add el2gh to the skip set to match the metadata-check script's blacklist, and to COMPAT_EXTENSIONS so the unit-level fixtures stay in sync. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
KVM is the preffered hypervisor for Qualcomm Linux SoCs. Gunyah was used as an interim solution till KVM is fully functional. Now that KVM support on Rb3Gen2 Core Kit has been validated, it no longer requires Gunyah as an interim solution. Add 'kvm' to MACHINE_FEATURES so that the KVM-specific XBL configuration is selected during boot. This switches the default hypervisor to KVM and aligns Rb3Gen2 Core Kit with IQ-615-EVK, IQ-8275-EVK, and IQ-9075-EVK. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
KVM is the preffered hypervisor for Qualcomm Linux SoCs. Gunyah was used as an interim solution till KVM is fully functional. Now that KVM support on QCM6490-IDP has been validated, it no longer requires Gunyah as an interim solution. Add 'kvm' to MACHINE_FEATURES so that the KVM-specific XBL configuration is selected during boot. This switches the default hypervisor to KVM and aligns QCM6490-IDP with IQ-615-EVK, IQ-8275-EVK, and IQ-9075-EVK. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
Currently FIT_DTB_COMPATIBLE entries in fit-dtb-compatible-linux-qcom.inc and fit-dtb-compatible.inc are not sorted and it is hard to identify missing and duplicate entires. Group them per SoC for easier navigation. Within each SoC section, compatible string entries containing el2gh string (indicating Gunyah hypervisor usage) are further grouped together and labeled for easier identification. No entries were added, removed, or changed, so there is no functional impact. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
Kodiak FIT_DTB_COMPATIBLE entries currently use el2kvm-specific compatible strings to load el2.dtbo. As KVM is the default hypervisor now, update the compatible entries to load kodiak-el2.dtbo without requiring an el2kvm suffix. Introduce dedicated el2gh variants for Gunyah configurations and group them under the corresponding Gunyah compatible strings. After dropping el2kvm suffix from Kodiak FIT_DTB_COMPATIBLE keys, four keys, qcom_qcs5430-iot-camx, qcom_qcs5430-iot-subtype2-camx, qcom_qcs6490-iot-camx and qcom_qcs6490-iot-subtype2-camx each defined twice with different values. Drop the stale first occurrence of each of the four keys, keeping the kodiak-el2-inclusive value that these are supposed to be resolving to. Signed-off-by: Viswanath Kraleti <viswanath.kraleti@oss.qualcomm.com>
457c794 to
62eb8a7
Compare
|
Dmitry Baryshkov (@lumag) Ricardo Salveti (@ricardosalveti) updated nHLOS is consumed by meta-qcom and boot tests on Rb3Gen2 are successful. Can you please review one moretime? |
|
Let's wait for the tests to conclude |
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-el2kvm] = "qcm6490-idp kodiak-el2" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-staging" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp] = "qcm6490-idp kodiak-el2" | ||
| FIT_DTB_COMPATIBLE[qcom_qcm6490-idp-staging] = "qcm6490-idp kodiak-el2 kodiak-staging" |
There was a problem hiding this comment.
I think it was written several times. Don't mix functional changes and refactoring / reordering. This commit really should have only one kind of changes: drop -el2kvm if it's a part of the compat string, otherwise add -el2gh. No reordering, no moving, etc.
All PILs are functioning correctly with KVM on Rb3Gen2-Core-Kit and QCM6490-IDP. Update
FIT_DTB_COMPATIBLE entries and machine configurations to switch to KVM on these boards.