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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5e3e9d9f-fa14-4744-9067-c93a210eb5f2@amd.com>
Date: Mon, 13 Jan 2025 14:10:17 -0600
From: Pratik Rajesh Sampat <prsampat@....com>
To: "Nikunj A. Dadhania" <nikunj@....com>,
 "Pratik R. Sampat" <pratikrajesh.sampat@....com>, kvm@...r.kernel.org
Cc: seanjc@...gle.com, pbonzini@...hat.com, pgonda@...gle.com,
 thomas.lendacky@....com, michael.roth@....com, shuah@...nel.org,
 linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/8] KVM: selftests: Add library support for
 interacting with SNP



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@....com>
>> ---
>>  .../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