Re: [PATCH v4 5/8] KVM: selftests: Add library support for interacting with SNP

[Nikunj A. Dadhania]

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?

Regards,
Nikunj