From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f201.google.com (mail-pg1-f201.google.com [209.85.215.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4DF8E168DC for ; Fri, 9 Aug 2024 15:40:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723218047; cv=none; b=nfZ9MaY6riBwJE/l+XQGZ0fZ2yfDmMnQj2OL15ZPqUBRXfqmQ9wKz89w6Hj9lixuMRou+uvAQL67yHMqTZOPAh+SY5/PSDU8K56tUhNJ1u05lLKBQhwSDum7ExxKzCZNUnzz3q8xn5qRAfYjZ7GCysoaU+j4Syexovh02mq/1bI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723218047; c=relaxed/simple; bh=qYLK/8+7XbwXyc5gcegIRBpIsaI+pvRczdbaYtaSCdE=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=V56Kzqcj0ZYsMMSoZtMyyMc/FtXyGqZHY9V8elz1yCXEKbMLaQGCN6xkb8jjb9roVSc28lCOeG49onvm0EK8X74yU2E4zh8I4jj2dEoLs5i+koNs92gfP8hg4Hq5goyYEbs3f1IEmUnERdpyOrP7ZlUX5Ocd7PxDKp5o2eQY0dU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=rTukblR+; arc=none smtp.client-ip=209.85.215.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="rTukblR+" Received: by mail-pg1-f201.google.com with SMTP id 41be03b00d2f7-7a242496897so2192375a12.2 for ; Fri, 09 Aug 2024 08:40:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723218045; x=1723822845; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=ErMc17L04Y+frI1FBkQ+3nAwLH6q7Xg4kpfE9NJb3g8=; b=rTukblR+lXJim9aJNe/7HYybeb6EyBeiNVE7XKqVlglAFg3q4MmEtTVq2SVUD9UkHT 9Rc6NiJrfradZSSb7gp9cztf3ZCy9125X8olpgxHccfvc5fNRJgqhDLU7p22Sj4VKs5o gIPcZPa83VcdcSWdT1xxKiREIYExuhunQk5pki8qNybKIF61EML+jp6VJKLkVVEVcGD/ V7PqhthgZjrlPdJW+aA/ydYUHHTia3cYBND6tuyBSM7UvoYgnfUVJpD7DLaYcsSDFOle OV3slopw4zhonHNA86AsxS+CEK3FuK7SM+WWPkhCHVC5AbJmF+ETYVDbCvfCZCQad2m3 pNbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723218045; x=1723822845; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ErMc17L04Y+frI1FBkQ+3nAwLH6q7Xg4kpfE9NJb3g8=; b=qL023iSWP7V890yt5jZy8+x2YS35h4af4kLTWj7bK/oZNCnh634Hv5kC/dEPWQadeX nie23QjT4+XR6nMn4+Kvmr1bW0Q2erI2azeOjdvXL02JAyX4Vwz1A4ZmA13OB331xZmM LilwoC25WzlTwNm4KLm7zYQWlZ6xMJtNIbkatW46y4+RWELHUqVHSKiB10BvsNJujtcd VTpMrWoMoFAwajEUdt9u0AcXCOEotqHLGR5qauJuEZz4X1g7cjHmYVwT0cYG0lTqbekL M4ZRjW5NBMcHYNGApIKPQUBsr3wGszqAwgLM1AFzRP2bcQpFEoL/DXXKcNnEzrwuuAZ/ 2yLA== X-Forwarded-Encrypted: i=1; AJvYcCVIJjKrglbEV5LuINiWzNy1BFO4DOvNcJ/2UM9FpX6JiuLQpy4YkJgh0KO/RbZq6loR3zrff6xopc2Ph+mnBVHgAp29tmzKr3ztUiB4 X-Gm-Message-State: AOJu0YwLowLaawjo1duaeN8U+wDal1K6iYKz2P6BwM2pNsw/ELldY8k7 AuQEr2zjXRINZ1zJqq9RvwBkQSX/Qauxein3PPTahEqLctEVNma+ZnhOgAc8SokbYRsUFSuuYJZ dVA== X-Google-Smtp-Source: AGHT+IGwKw2KKUU7sXggWm9RjiZAHKpy2+WRk1B7z9K2MDWh7RD36JKtkCJ+xCXkC9ifEG/LznRinzDK984= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a63:66c4:0:b0:78b:4703:17af with SMTP id 41be03b00d2f7-7c3d2bed86emr3612a12.6.1723218045232; Fri, 09 Aug 2024 08:40:45 -0700 (PDT) Date: Fri, 9 Aug 2024 08:40:43 -0700 In-Reply-To: <20240710220540.188239-3-pratikrajesh.sampat@amd.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20240710220540.188239-1-pratikrajesh.sampat@amd.com> <20240710220540.188239-3-pratikrajesh.sampat@amd.com> Message-ID: Subject: Re: [RFC 2/5] selftests: KVM: Decouple SEV ioctls from asserts From: Sean Christopherson To: "Pratik R. Sampat" Cc: kvm@vger.kernel.org, shuah@kernel.org, thomas.lendacky@amd.com, michael.roth@amd.com, pbonzini@redhat.com, pgonda@google.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Wed, Jul 10, 2024, Pratik R. Sampat wrote: > This commit separates the SEV, SEV-ES, SEV-SNP ioctl calls from its Don't start with "This commit". Please read Documentation/process/maintainer-kvm-x86.rst, and by extension, Documentation/process/maintainer-tip.rst. > positive test asserts. This is done so that negative tests can be > introduced and both kinds of testing can be performed independently > using the same base helpers of the ioctl. > > This commit also adds additional parameters such as flags to improve > testing coverage for the ioctls. > > Cleanups performed with no functional change intended. > > Signed-off-by: Pratik R. Sampat > --- > .../selftests/kvm/include/x86_64/sev.h | 20 +-- > tools/testing/selftests/kvm/lib/x86_64/sev.c | 145 ++++++++++++------ > 2 files changed, 108 insertions(+), 57 deletions(-) > > diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h > index 43b6c52831b2..ef99151e13a7 100644 > --- a/tools/testing/selftests/kvm/include/x86_64/sev.h > +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h > @@ -37,14 +37,16 @@ enum sev_guest_state { > #define GHCB_MSR_TERM_REQ 0x100 > > void sev_vm_launch(struct kvm_vm *vm, uint32_t policy); > -void sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement); > -void sev_vm_launch_finish(struct kvm_vm *vm); > +int sev_vm_launch_start(struct kvm_vm *vm, uint32_t policy); > +int sev_vm_launch_update(struct kvm_vm *vm, uint32_t policy); > +int sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement); > +int sev_vm_launch_finish(struct kvm_vm *vm); > > bool is_kvm_snp_supported(void); > > -void snp_vm_launch(struct kvm_vm *vm, uint32_t policy); > -void snp_vm_launch_update(struct kvm_vm *vm); > -void snp_vm_launch_finish(struct kvm_vm *vm); > +int snp_vm_launch(struct kvm_vm *vm, uint32_t policy, uint8_t flags); > +int snp_vm_launch_update(struct kvm_vm *vm, uint8_t page_type); > +int snp_vm_launch_finish(struct kvm_vm *vm, uint16_t flags); > > struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code, > struct kvm_vcpu **cpu); > @@ -98,7 +100,7 @@ static inline void sev_register_encrypted_memory(struct kvm_vm *vm, > vm_ioctl(vm, KVM_MEMORY_ENCRYPT_REG_REGION, &range); > } > > -static inline void snp_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa, > +static inline int snp_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa, > uint64_t size, uint8_t type) > { > struct kvm_sev_snp_launch_update update_data = { > @@ -108,10 +110,10 @@ static inline void snp_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa, > .type = type, > }; > > - vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update_data); > + return __vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update_data); Don't introduce APIs and then immediately rewrite all of the users. If you want to rework similar APIs, do the rework, then add the new APIs. Doing things in this order adds a pile of pointless churn. But that's a moot point, because it's far easier to just add __snp_launch_update_data(). And if you look through other APIs in kvm_util.h, you'll see that the strong preference is to let vm_ioctl(), or in this case vm_sev_ioctl(), do the heavy lifting. Yeah, it requires copy+pasting marshalling parameters into the struct, but that's relatively uninteresting code, _and_ piggybacking the "good" version means you can't do things like pass in a garbage virtual address (because the "good" version always guarantees a good virtual address).