On Mon, Oct 21, 2024, Pratik R. Sampat wrote:
test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);test_sev_es_shutdown(); if (kvm_has_cap(KVM_CAP_XCRS) && (xgetbv(0) & XFEATURE_MASK_X87_AVX) == XFEATURE_MASK_X87_AVX) {
test_sync_vmsa(0);test_sync_vmsa(SEV_POLICY_NO_DBG);
test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES | SEV_POLICY_NO_DBG);}- }
- if (kvm_cpu_has(X86_FEATURE_SNP) && is_kvm_snp_supported()) {
Why do we need both? KVM shouldn't advertise SNP if it's not supported.
My rationale behind needing this was that the feature can be advertised but it may not have the right API major or minor release which could be updated post boot and could determine it's support during runtime.
KVM will never determine support after KVM has been loaded. If *KVM* has a dependency on the API major.minor, then X86_FEATURE_SNP must be set if and only if the supported API version is available.
If the API major.minor is purely a userspace thing, then is_kvm_snp_supported() is misnamed, because the check has nothing to do with KVM. E.g. something like is_snp_api_version_supported() would be more appropriate.
unsigned long snp_policy = SNP_POLICY;u64, no?
Yes, sorry for the oversight. Will change it to u64.
if (unlikely(!is_smt_active()))snp_policy &= ~SNP_POLICY_SMT;Why does SNP_POLICY assume SMT? And what is RSVD_MBO? E.g. why not this?
u64 policy = is_smt_active() ? SNP_POLICY_SMT : SNP_POLICY;I think most systems support SMT so I enabled the bit in by default and only unset it when there isn't any support.
That's confusing though, because you're mixing architectural defines with semi- arbitrary selftests behavior. RSVD_MBO on the other is apparently tightly coupled with SNP, i.e. SNP can't exist without that bit, so it makes sense that RSVD_MBO needs to be part of SNP_POLICY
If you want to have a *software*-defined default policy, then make it obvious that it's software defined. E.g. name the #define SNP_DEFAULT_POLICY, not simply SNP_POLICY, because the latter is too easily misconstrued as the base SNP policy, which it is not. That said, IIUC, SMT *must* match the host configuration, i.e. whether or not SMT is set is non-negotiable. In that case, there's zero value in defining SNP_DEFAULT_POLICY, because it can't be a sane default for all systems.
Side topic, I assume one of SEV_POLICY_NO_DBG or SNP_POLICY_DBG *must* be specified, and that they are mutualy exclusive? E.g. what happens if the full policy is simply SNP_POLICY_RSVD_MBO?