From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B60ABC4345F for ; Wed, 24 Apr 2024 21:40:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCABD6B02D7; Wed, 24 Apr 2024 17:40:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D7A646B02D8; Wed, 24 Apr 2024 17:40:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C68F26B02DB; Wed, 24 Apr 2024 17:40:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id A77006B02D7 for ; Wed, 24 Apr 2024 17:40:08 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id F002B81073 for ; Wed, 24 Apr 2024 21:40:07 +0000 (UTC) X-FDA: 82045743654.28.5F13283 Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) by imf07.hostedemail.com (Postfix) with ESMTP id 61B2C40012 for ; Wed, 24 Apr 2024 21:40:05 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1wJCQHur; spf=pass (imf07.hostedemail.com: domain of 3NHwpZgYKCAs3plyunrzzrwp.nzxwty58-xxv6lnv.z2r@flex--seanjc.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3NHwpZgYKCAs3plyunrzzrwp.nzxwty58-xxv6lnv.z2r@flex--seanjc.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713994805; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=I1EKZolXdPYAzbRipKSJPkUCjMZ4Y7QNaYncgFsTwr8=; b=Ww66BwBCUTmOBxNpocUPECYuucgOSuZY0Q4xdDnONnB7SnaBN74CrVF4U0XKOqBYezFfcP OYq/8rJ7DPBji+YzE4UE33e8yszfw2raUgIiRk1WTHybjUbX7BhrSdyWKeDkONXh2IJkF2 cTlej4eUKDLmawLfbiMKAJOdQf3oUFs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713994805; a=rsa-sha256; cv=none; b=LqLPdyg182kV1Pms6wDTn8Aru8wV5i1uVtUG2Oz2mG4fNo46HdLANSnqB/DsuGSvcZFQEY 5iHYjLfFZJB+jkhYIP7BHzG6rmyLS+nHuoD+y9Im86h2yzulBy6wQA5sK/x+isaA6KkcRc HKC81acmi5lOW13Vt/SyUMrCZbzYXUI= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1wJCQHur; spf=pass (imf07.hostedemail.com: domain of 3NHwpZgYKCAs3plyunrzzrwp.nzxwty58-xxv6lnv.z2r@flex--seanjc.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3NHwpZgYKCAs3plyunrzzrwp.nzxwty58-xxv6lnv.z2r@flex--seanjc.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yb1-f202.google.com with SMTP id 3f1490d57ef6-dbe9e13775aso698856276.1 for ; Wed, 24 Apr 2024 14:40:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713994804; x=1714599604; darn=kvack.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=I1EKZolXdPYAzbRipKSJPkUCjMZ4Y7QNaYncgFsTwr8=; b=1wJCQHurMiUwHvWrI5jkAKc8xqyWMxMh6lHaeHh/UkuBeUOGdKlv/1Yb+MQht5/1ob 3KoD3vIlTxxm5a/+702RyoZjULgX1lV1ZSvZjczppOCHYXoiRtvjDsg9BUvWwn0K+Rak 8JiDgqFWVteQ9LrmlkGvD9CcDjrcAN2rZfdaAWRGfn63D8zUJtocRfaceuBq/dpZx3yH pv1uyu7TPVyU2hkP29JxctLjygvdl8ZcwdSZoIu8AkPv4jD6lYkZJ/97KE9w+PY+6Z7t uPj3sBrW+KcnHN+6AYk+PFThaItolJi+mOL53bVQ37Zuz7Gm1DCPs6OuE97aW7u4oF2i agRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713994804; x=1714599604; 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=I1EKZolXdPYAzbRipKSJPkUCjMZ4Y7QNaYncgFsTwr8=; b=i14robV0drYsARXy6waWokBM++ok2M166vY2lIi2jVUXMH8XF8RIF/ELKmTiUslYva k8gxS3GpP0NEfAfrFNAfSVJQzW8k9h14dnYI9g7xFjHBP0xWgu+xfxwpSBVWN/euRP3r 20di3Y8Xvb4d4Bhy4ZblYLeiVBew2Bk5mwIzbrctmk9pGN7krC7/Ad8ltxyB0Z1+pIH2 da9hHxjGgxaPBdfKDAdw0sFDWlUFaxbXWiudZSDIBq5D8djL5UPu9QiI8c25ZNW6LpRS oW8Kgfx4S42qq2ojeIdzG367oJU24fPZkvh1muQp/s029rEhHOC8unGoWNRF7RAy5dii s3eQ== X-Forwarded-Encrypted: i=1; AJvYcCWfOxhPamuAl1UOegQzw1E3ZHXLG0x8/x0C9pdUBFFGOL9MjvB8cONaiEDQBDw9AFMBH+jKkvfWPp6ixcYHOnumgRo= X-Gm-Message-State: AOJu0YxsmzqBFEovvWXpDs6Cg7cXIHHI44Ojv702RoXa0mQ5OX9K54M0 tq6IlZtCPqGXP9sXB6vq/iXM5uCYX8hcSY1K5fAKPrdkhj6YCVDy9lQ9xCBDSLxQyK9yu/n8C4N DlA== X-Google-Smtp-Source: AGHT+IHBpn1SOBbV7drc58TxI6npqIiVI1VLzloAxa/Hpo4e1kwyTXpiUjL14BLYZcy0Z92zCwnow659/2U= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a05:6902:708:b0:dcb:e4a2:1ab1 with SMTP id k8-20020a056902070800b00dcbe4a21ab1mr1134066ybt.11.1713994804317; Wed, 24 Apr 2024 14:40:04 -0700 (PDT) Date: Wed, 24 Apr 2024 14:40:02 -0700 In-Reply-To: <20240418194133.1452059-10-michael.roth@amd.com> Mime-Version: 1.0 References: <20240418194133.1452059-1-michael.roth@amd.com> <20240418194133.1452059-10-michael.roth@amd.com> Message-ID: Subject: Re: [PATCH v13 09/26] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command From: Sean Christopherson To: Michael Roth Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com, ardb@kernel.org, pbonzini@redhat.com, vkuznets@redhat.com, jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com, pgonda@google.com, peterz@infradead.org, srinivas.pandruvada@linux.intel.com, rientjes@google.com, dovmurik@linux.ibm.com, tobin@ibm.com, bp@alien8.de, vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com, tony.luck@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com, jarkko@kernel.org, ashish.kalra@amd.com, nikunj.dadhania@amd.com, pankaj.gupta@amd.com, liam.merwick@oracle.com, Brijesh Singh Content-Type: text/plain; charset="us-ascii" X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 61B2C40012 X-Stat-Signature: 1481iacwkj9h4qrn5zrm7kc7yzjbwzeb X-Rspam-User: X-HE-Tag: 1713994805-945598 X-HE-Meta: U2FsdGVkX19gW/FF13F1SukHPX2MicVWH4INs7y4WisHThkETtDUCNsfWBH3Hvt+hi3NdUJikB658FiP/R57N7a8SLUQ6EHn3GSRmBI0OZFJH81v32UzVhHSTPfmVl7W1FT9oHZrMMzzHmHkFlVVFlXncwXi5E+f1Kfo8DL2ANfijbSPzi8C/oTuod6Jqogi8m3TMVEFekoKdADnyNlkAAd5XYFF0Yg51phkOpNVMAieY+yrwIxfFfuqWAOsn9o5HZH9smZfE3TBnVrPFnLvKGTiYLoJYsv/gNZgoKngDVGME1wQgtqwtEnY+2BXq9IlGHVD0ydyZMDq4NuJbQfC449JAVhXVfuiezXdCO9DwOjvKRrtDuCFAnFcJL9GWq85J6FMUJcDDbRnjO4NMQJuacMynHRGW7FMRXM89V4ov1mAuqo/CrbjYAn27S8UO3LPPhsYWA7jEeloWK/HrPtrmHiZvBzcPz8druD0vMWpg16io4ILU1xnsUEtJotphRAfiKi/97rDinFGhgZ0er6QUAvkZLXNARoaTeUiRzZGsWCFGiSS9IQY2DVeWJmJX6p071dHqy9ifrR3O/3GZwbcgp+IOVpAn1dGUZIy2qpyNAHrNXGFvh/clBX8q3hCbvUTogYRMLY+nMKQmXFWDKXspn+jNAEkHfj5TmqdUSqEUH6HamwyB5CiQpo1C5c+5H3Qxuqu68fstZeF7lFB3tZpxopFFmEmJiUY5uo1ZpghMYmPyLDWfGFmM3Zm65U2WFHFZPaZlRbQgHW3wlGOxB5H2hmQque1noEChW08CZmTNb2laU4uZwHGUU2ClbDuMC/Trln7APlOJsMI0G6OA8RpC18zycbNQ1lDhLctSd69vce6dwly7Jb/jQYqViA5znCV/MUVQDh+vHGVhUr2LiJFxx6tpY592Z5P4MESsPgV1jyqAXZVFAjqd0gZGAnnr3uVcef7ChLUYpXuYZcC4h/ wQzwuLFo V0t+lySxL3Ww9JY60VXeO2njmKL+8uVpk+iZZPE1vXejeEaFd+apE96xBAEQmm4Fb4W4Rv9hZpemMhI8aj1G9Ir9T0E+CTDhElxNgWxAtmjFEWJeKj736TL23H73REB3DyLMnBEX4UWW4tGe17SMANr1vYu41ChkIGte9dT4aGKh6kZkodBrXAOjXI14m9x61N/WE79Cjew5krwl8V0aJDtDInFQIJ3wJ3KertzVctVM1780m24HOtHaV2mEa42Abcdu13pFjmCm+DIvMndWhUA8M+qFfhI/IvH4ur1xiCn6VZhQs49LyQQr6v7xURAEq8O2U5jnO4C5jHuR7WqJbjnDOzs48lIqlCuqEaKlPJFf34PgB28v9AbmxyKHg9AgXxgY8Dc5gGqAHADbxQqNgJNflqw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Apr 18, 2024, Michael Roth wrote: > +static inline bool sev_version_greater_or_equal(u8 major, u8 minor) > +{ > + if (major < SNP_POLICY_API_MAJOR) > + return true; > + > + if (major == SNP_POLICY_API_MAJOR && minor <= SNP_POLICY_API_MINOR) > + return true; > + > + return false; > +} > + > +static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + struct sev_data_snp_launch_start start = {0}; > + struct kvm_sev_snp_launch_start params; > + u8 major, minor; > + int rc; > + > + if (!sev_snp_guest(kvm)) > + return -ENOTTY; > + > + if (copy_from_user(¶ms, u64_to_user_ptr(argp->data), sizeof(params))) > + return -EFAULT; > + > + /* Don't allow userspace to allocate memory for more than 1 SNP context. */ > + if (sev->snp_context) { > + pr_debug("SEV-SNP context already exists. Refusing to allocate an additional one.\n"); What's the plan with all these printks? There are far too many in this series. Some might be useful, but many of them have no business landing upstream. > + return -EINVAL; > + } > + > + sev->snp_context = snp_context_create(kvm, argp); > + if (!sev->snp_context) > + return -ENOTTY; > + > + if (params.policy & ~SNP_POLICY_MASK_VALID) { > + pr_debug("SEV-SNP hypervisor does not support requested policy %llx (supported %llx).\n", What does "SEV-SNP hypervisor" even mean? > + params.policy, SNP_POLICY_MASK_VALID); > + return -EINVAL; > + } > + > + if (!(params.policy & SNP_POLICY_MASK_RSVD_MBO)) { > + pr_debug("SEV-SNP hypervisor does not support requested policy %llx (must be set %llx).\n", > + params.policy, SNP_POLICY_MASK_RSVD_MBO); > + return -EINVAL; > + } > + > + if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET) { > + pr_debug("SEV-SNP hypervisor does not support limiting guests to a single socket.\n"); > + return -EINVAL; > + } > + > + if (!(params.policy & SNP_POLICY_MASK_SMT)) { > + pr_debug("SEV-SNP hypervisor does not support limiting guests to a single SMT thread.\n"); > + return -EINVAL; > + } > + > + major = (params.policy & SNP_POLICY_MASK_API_MAJOR); > + minor = (params.policy & SNP_POLICY_MASK_API_MINOR); > + if (!sev_version_greater_or_equal(major, minor)) { Why does this need a someone weirdly named helper? Isn't this just? if (major < SNP_POLICY_API_MAJOR || (major == SNP_POLICY_API_MAJOR && minor < SNP_POLICY_API_MINOR)) > + pr_debug("SEV-SNP hypervisor does not support requested version %d.%d (have %d,%d).\n", > + major, minor, SNP_POLICY_API_MAJOR, SNP_POLICY_API_MINOR); > + return -EINVAL; > + } > + > + start.gctx_paddr = __psp_pa(sev->snp_context); > + start.policy = params.policy; > + memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw)); > + rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_START, &start, &argp->error); > + if (rc) { > + pr_debug("SEV_CMD_SNP_LAUNCH_START firmware command failed, rc %d\n", rc); > + goto e_free_context; > + } > + > + sev->fd = argp->sev_fd; > + rc = snp_bind_asid(kvm, &argp->error); > + if (rc) { > + pr_debug("Failed to bind ASID to SEV-SNP context, rc %d\n", rc); > + goto e_free_context; > + } > + > + return 0; > + > +e_free_context: > + snp_decommission_context(kvm); > + > + return rc; > +} > + > int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > { > struct kvm_sev_cmd sev_cmd; > @@ -1999,6 +2154,15 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > goto out; > } > > + /* > + * Once KVM_SEV_INIT2 initializes a KVM instance as an SNP guest, only > + * allow the use of SNP-specific commands. > + */ > + if (sev_snp_guest(kvm) && sev_cmd.id < KVM_SEV_SNP_LAUNCH_START) { > + r = -EPERM; > + goto out; > + } > + > switch (sev_cmd.id) { > case KVM_SEV_ES_INIT: > if (!sev_es_enabled) { > @@ -2063,6 +2227,9 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > case KVM_SEV_RECEIVE_FINISH: > r = sev_receive_finish(kvm, &sev_cmd); > break; > + case KVM_SEV_SNP_LAUNCH_START: > + r = snp_launch_start(kvm, &sev_cmd); > + break; > default: > r = -EINVAL; > goto out; > @@ -2258,6 +2425,33 @@ int sev_vm_copy_enc_context_from(struct kvm *kvm, unsigned int source_fd) > return ret; > } > > +static int snp_decommission_context(struct kvm *kvm) > +{ > + struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > + struct sev_data_snp_addr data = {}; > + int ret; > + > + /* If context is not created then do nothing */ > + if (!sev->snp_context) > + return 0; > + > + data.address = __sme_pa(sev->snp_context); > + down_write(&sev_deactivate_lock); > + ret = sev_do_cmd(SEV_CMD_SNP_DECOMMISSION, &data, NULL); > + if (WARN_ONCE(ret, "failed to release guest context")) { WARN here, or WARN in the caller, not both. And if you warn here, this can be down_write(&sev_deactivate_lock); ret = sev_do_cmd(SEV_CMD_SNP_DECOMMISSION, &data, NULL); up_write(&sev_deactivate_lock); if (WARN_ONCE(ret, "...")) > + up_write(&sev_deactivate_lock); > + return ret; > + } > + > + up_write(&sev_deactivate_lock); > + > + /* free the context page now */ This doesn't seem like a particularly useful comment. What would be useful is a comment explaining the "decommission" unbinds the ASID. > + snp_free_firmware_page(sev->snp_context); > + sev->snp_context = NULL; > + > + return 0; > +} > + > void sev_vm_destroy(struct kvm *kvm) > { > struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info; > @@ -2299,7 +2493,15 @@ void sev_vm_destroy(struct kvm *kvm) > } > } > > - sev_unbind_asid(kvm, sev->handle); > + if (sev_snp_guest(kvm)) { > + if (snp_decommission_context(kvm)) { > + WARN_ONCE(1, "Failed to free SNP guest context, leaking asid!\n"); WARN on the actually failure, not '1'. And a newline isn't needed. if (WARN_ONCE(snp_decommission_context(kvm) "Failed to free SNP guest context, leaking asid!")) return; > + return; > + } > + } else { > + sev_unbind_asid(kvm, sev->handle); > + } > + > sev_asid_free(sev); > } > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h > index 7f2e9c7fc4ca..0654fc91d4db 100644 > --- a/arch/x86/kvm/svm/svm.h > +++ b/arch/x86/kvm/svm/svm.h > @@ -92,6 +92,7 @@ struct kvm_sev_info { > struct list_head mirror_entry; /* Use as a list entry of mirrors */ > struct misc_cg *misc_cg; /* For misc cgroup accounting */ > atomic_t migration_in_progress; > + void *snp_context; /* SNP guest context page */ > }; > > struct kvm_svm { > -- > 2.25.1 >