On Fri, Feb 14, 2025, Pratik Rajesh Sampat wrote:
On 2/11/25 8:12 PM, Sean Christopherson wrote:
On Mon, Feb 03, 2025, Pratik R. Sampat wrote:
Extend the SEV library to include support for SNP ioctl() wrappers, which aid in launching and interacting with a SEV-SNP guest.
Tested-by: Srikanth Aithal sraithal@amd.com Signed-off-by: Pratik R. Sampat prsampat@amd.com
[..snip..]
+/*
- A SEV-SNP VM requires the policy reserved bit to always be set.
- The SMT policy bit is also required to be set based on SMT being
- available and active on the system.
- */
+static inline u64 snp_default_policy(void) +{
- bool smt_active = false;
- FILE *f;
- f = fopen("/sys/devices/system/cpu/smt/active", "r");
Please add a helper to query if SMT is enabled. I doubt there will ever be many users of this, but it doesn't seem like something that should buried in SNP code.
Ha! smt_possible() in tools/testing/selftests/kvm/x86/hyperv_cpuid.c is already guilty of burying a related helper, and it looks like it's a more robust version.
You're right, a more general helper is in order here.
Since the hyperv_cpuid selftest only seems to care about whether SMT is possible (i.e., it may or may not be enabled) and we care about it being enabled as well, for the flag to be set. I should make a more generic variant(s) that can be accessible to both. Maybe I can implement it within testing/selftests/kvm/include/x86_processor.h?
It should go in kvm_util.h, /sys/devices/system/cpu/smt/active is a generic interface provided by kernel/cpu.c.
@@ -93,7 +124,7 @@ void sev_vm_launch(struct kvm_vm *vm, uint32_t policy) TEST_ASSERT_EQ(status.state, SEV_GUEST_STATE_LAUNCH_UPDATE); hash_for_each(vm->regions.slot_hash, ctr, region, slot_node)
encrypt_region(vm, region);
encrypt_region(vm, region, 0);Please add an enum/macro instead of open coding a literal '0'. I gotta assume there's an appropriate name for page type '0'.
For SNP, we supply this parameter to determine the page type for SNP launch update defined as KVM_SEV_SNP_PAGE_TYPE_*. For SEV/SEV-ES, however, the page type doesn't really get factored in and falls through unaccounted in that case, so I had passed a zero to it.
Having said that, having a literal here is quite unclean. Maybe I can pass one of the existing page types to it or, better yet, define a new KVM_SEV_PAGE_TYPE_[RESERVED|UNUSED] type instead for our selftest header?
Ya, define something new and arbitrary. I vote for either KVM_SEV_PAGE_TYPE_NONE or KVM_SEV_PAGE_TYPE_INVALID. RESERVED suggests the page is "valid" but reserved for some entity. Ditto for UNUSED; valid, but not yet claimed.