On 1/13/25 2:32 AM, Nikunj A. Dadhania wrote: > > > On 11/15/2024 5:11 AM, 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. >> >> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@xxxxxxx> >> --- >> .../selftests/kvm/include/x86_64/sev.h | 49 ++++++++++- >> tools/testing/selftests/kvm/lib/x86_64/sev.c | 81 ++++++++++++++++++- >> 2 files changed, 125 insertions(+), 5 deletions(-) > > >> diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c >> index d6e7a422b69d..40b90d3a5769 100644 >> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c >> +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c >> @@ -31,7 +31,8 @@ bool is_sev_vm(struct kvm_vm *vm) >> * and find the first range, but that's correct because the condition >> * expression would cause us to quit the loop. >> */ >> -static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *region) >> +static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *region, >> + uint8_t page_type) >> { >> const struct sparsebit *protected_phy_pages = region->protected_phy_pages; >> const vm_paddr_t gpa_base = region->region.guest_phys_addr; >> @@ -41,16 +42,39 @@ static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *regio >> if (!sparsebit_any_set(protected_phy_pages)) >> return; >> >> - sev_register_encrypted_memory(vm, region); >> + if (!is_sev_snp_vm(vm)) >> + sev_register_encrypted_memory(vm, region); >> >> sparsebit_for_each_set_range(protected_phy_pages, i, j) { >> const uint64_t size = (j - i + 1) * vm->page_size; >> const uint64_t offset = (i - lowest_page_in_region) * vm->page_size; >> >> + if (is_sev_snp_vm(vm)) { >> + snp_launch_update_data(vm, gpa_base + offset, >> + (uint64_t)addr_gpa2hva(vm, gpa_base + offset), >> + size, page_type); >> + continue; >> + } >> + > > Instead of using continue, if/else is the better code flow: > > if (is_sev_snp_vm(vm)) > snp_launch_update_data(vm, gpa_base + offset, > (uint64_t)addr_gpa2hva(vm, gpa_base + offset), > size, page_type); > else > sev_launch_update_data(vm, gpa_base + offset, size); > > Right? Sure, I can change that. That's definitely cleaner. Thanks! Pratik