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 5490DC77B76 for ; Fri, 21 Apr 2023 18:50:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AD7616B0074; Fri, 21 Apr 2023 14:50:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A87F56B0075; Fri, 21 Apr 2023 14:50:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 928236B0078; Fri, 21 Apr 2023 14:50:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 834F46B0074 for ; Fri, 21 Apr 2023 14:50:22 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 538CC40628 for ; Fri, 21 Apr 2023 18:50:22 +0000 (UTC) X-FDA: 80706288684.28.E7AEF0C Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by imf26.hostedemail.com (Postfix) with ESMTP id 75D40140003 for ; Fri, 21 Apr 2023 18:50:20 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=rivosinc-com.20221208.gappssmtp.com header.s=20221208 header.b="THDL4+u/"; spf=pass (imf26.hostedemail.com: domain of atishp@rivosinc.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=atishp@rivosinc.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682103020; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/IIUhUlmRecN66Z6gik+9JylRPrSHv9qYt9esJpbqSI=; b=Ewv0woNeOPrXVyZON1mfnktv7iGhzLqeK4tyLvPyef/lQFE1TAAypT7k3a9GB4QKb3neHx Xbd6wZUiEibUHoNqXo+aS9KZrH8leaRWvAIsnH1UWd5naUtaMszO7fWphvMypO2n4iT5Pq 9xfWZ8WARUep8A62uUcC2Ah+0b+8kPc= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=rivosinc-com.20221208.gappssmtp.com header.s=20221208 header.b="THDL4+u/"; spf=pass (imf26.hostedemail.com: domain of atishp@rivosinc.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=atishp@rivosinc.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682103020; a=rsa-sha256; cv=none; b=kbdamnYZ0cDg13x1nRRdxFn33r99zjbxoy4DassIlUdK8Q4gOJ7bI+OaAGSrJINvIt6Y2l UEE5IqBHQ2POHDgNp9DgMK1/jdkd+A6Qv3nq/QdPJP91qj5uRzDJRd1nr5SZzJwblqSKik ao28S+FiljOqo2LabDvB49tdDtnR2U4= Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-1a66b9bd7dfso27796765ad.2 for ; Fri, 21 Apr 2023 11:50:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1682103019; x=1684695019; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/IIUhUlmRecN66Z6gik+9JylRPrSHv9qYt9esJpbqSI=; b=THDL4+u/vOfwLaOnCW6eh/CjgCzPbUBKAHWf1LgoVb5urObrbbDHtE6LnFnJmkcGbJ X8sScn53VOwmiufg6b6vh6lFIqrfOMJmRlqDY7muic2fAlWCCsOBcmJbCjOoIvNUxnTw CCi7AskMw6mhK+++me+sRjwikLpeFgIwFV0nF0mmjg428f8Rammes3zg4WwGMq8g786W u8Qe//FfgFBTmm/8czZ4ckylnzNPDOezoP2aS60VK1m3/oMe/h5ZP8lwmqiqiNZdCveU hRoUCo88MTFudnwN/fMDvgNm+b5CVsDTOcg14bYbIUZFekjOGitMEoCJ5+oTpJJu4z/U HanA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682103019; x=1684695019; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/IIUhUlmRecN66Z6gik+9JylRPrSHv9qYt9esJpbqSI=; b=B4qF6KmzrXJsF8VBQ276Qq5zgr6TM1hSDhj5mmDottaOjx1R7lNhorTGx8+qp2PFGP /tKckClQtYXaL9yiqj1uX+pdwI0d8q9AufGzVLHeo/SeMmbXMCuZr6zSSqmnvV0b07sh r58f95U5rEC/6YR2CBrk5mSPs9QZfyTYSX0jG6Uxsdv2YHbVT7+bU16+xPYcEP8N6uAt /nMBfe/ysDjKQaYX8MkhFzbyZU40rAJK2hLD1L0yF+I7aQRjLjpWkkx2WpBHfK72Ww4a M5reyH6l1jU605Qz+MalpT4Ca8QcvWq6QQ478jGCXEujMc8JWLymBZCdy3tncCxBInjQ ZkSw== X-Gm-Message-State: AAQBX9dwYGq+XpEsE1KMmlzYdeHq786G6q7vsWHOlKZy6PyQG+rMvS9b 1yl8xQEPoab+UvoEqhEplE+A3wTk2QBsalO9WkD/2A== X-Google-Smtp-Source: AKy350YdSljDnRrQJM9jfWzeZ/GLttvpy4dWrgL7+90Nmaems+z3L9naUfpMZAy9sq8Fz7lbqPOj5hZdKf+W5l/Qavg= X-Received: by 2002:a17:902:8501:b0:1a1:f95a:24f2 with SMTP id bj1-20020a170902850100b001a1f95a24f2mr5252822plb.19.1682103019136; Fri, 21 Apr 2023 11:50:19 -0700 (PDT) MIME-Version: 1.0 References: <20230419221716.3603068-1-atishp@rivosinc.com> <20230419221716.3603068-11-atishp@rivosinc.com> In-Reply-To: From: Atish Kumar Patra Date: Sat, 22 Apr 2023 00:20:08 +0530 Message-ID: Subject: Re: [RFC 10/48] RISC-V: KVM: Implement static memory region measurement To: Sean Christopherson Cc: linux-kernel@vger.kernel.org, Alexandre Ghiti , Andrew Jones , Andrew Morton , Anup Patel , Atish Patra , =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , Suzuki K Poulose , Will Deacon , Marc Zyngier , linux-coco@lists.linux.dev, Dylan Reid , abrestic@rivosinc.com, Samuel Ortiz , Christoph Hellwig , Conor Dooley , Greg Kroah-Hartman , Guo Ren , Heiko Stuebner , Jiri Slaby , kvm-riscv@lists.infradead.org, kvm@vger.kernel.org, linux-mm@kvack.org, linux-riscv@lists.infradead.org, Mayuresh Chitale , Palmer Dabbelt , Paolo Bonzini , Paul Walmsley , Rajnesh Kanwal , Uladzislau Rezki Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 73b5exccf77huxbk4g4qth9paiss3yfh X-Rspam-User: X-Rspamd-Queue-Id: 75D40140003 X-Rspamd-Server: rspam06 X-HE-Tag: 1682103020-917824 X-HE-Meta: U2FsdGVkX19FYoxH4gSmXyJaGnMCp47zHzcC8q9m1YI7d2OYY/YLCUDDfEoFo0NMq/2rsysJUDE8JZshyD8KMXepe7glsIDS1skGxz1gFfIK86tjm77NxisR/CWa2tFB2Galt38pSgZMsSfgT65aIwm12+ihPksb9q9h3eZDkLznpk3WsnffoQtK7QqV/G/GyDp5MNswFa4A4DNkxBTnYukxpneIF4eEHPokhe6xwmzLJoOecyNwip4k65agK6Pjdw74JCZiA9R3DVmCXf+FcHIVfiuE8WycXcTHdRRgKCm0/Un9sF9/UOBMSQFeoQSH1Rtan3pjsuaAM5OPas3+RJXuDQ58EpUpjgVykhe7uSRXBchr+99DFLfmQ0IzuOuk9zMwXumgXPf+TWRDZa7hHr79yO4AVcUzQx0auunTHQdqi8zcCgfrbMsbp9BOp9hDedVWbHvCfT2paA0oog0M4vQflyFbpgpEV2OjQe7r8KWdtK/8OyGrTIw427D+pICT9VBIrXy7X6kNqiWIosx2sRwW3E0GlxHjdk7CxovVE44NE+t2yEAuOg/J8aorc4iJ7oJHH8IvKlUaXeytb+QAV2s88j607hguGvVtSuHZobaaOavyltU5/j1B0Nk+uxldiDpFfgYgJcy+6CjWM5qnGZ8iIk3CEaa78Hc42fiey+NGV1iXwHkto+s5DY9I4TiyVefvTqEuewEOcPGDwWbKg3av4FK57HfV6lIjD0s2uaQcy6PDvM7KW0tfWKmuQZ51LFnGpLG0eZuasfGcn1x3A49zcOvWBED83sZqvAg+qd8IQhViRrrMhB7SOnFL0skFmg53qEqcdvHiyhu7EhBc46eQEwg/h2Pfu03Nuus8Tv+mJoGWGL5fyP0s1BkF+Y8pBNTgc03+L7+bx3a880mM6eCt6u7uf7F9x+PnFi8q3gYB1nLoVJvCc7WkNgdHtw1xhWBqKilwtJNpizBI+Ty pcY7k7+F NIEpAtrnfpCkxKqad6tpHParJC2jTrwW4/Vq8x0iFmQZ30Ot9xcvtjHD6KJokgctaaSmu2JgvgkVcSiaJE7QugPyHnQ/IkCozzpQH00aWYpgJ63nc2KsN/FQnQbHVLI2QWGPgGLtvdWeWEv/9i5L9wDjY4oy6wQB5AiD+ieHBVqOP2g5891ZlxPuOcdJ04/Gh4ohxnUY54tOi08ZXbWk3QrHO1ecmsxCaMsp4XFlw2YmjVP/zvMPnquDMQbpTO23OkQIA61vF1eC71hm3PjXscwsvNpWK2tI6KLiLZa9e/tN8JIE= 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: On Thu, Apr 20, 2023 at 8:47=E2=80=AFPM Sean Christopherson wrote: > > On Wed, Apr 19, 2023, Atish Patra wrote: > > +int kvm_riscv_cove_vm_measure_pages(struct kvm *kvm, struct kvm_riscv_= cove_measure_region *mr) > > +{ > > + struct kvm_cove_tvm_context *tvmc =3D kvm->arch.tvmc; > > + int rc =3D 0, idx, num_pages; > > + struct kvm_riscv_cove_mem_region *conf; > > + struct page *pinned_page, *conf_page; > > + struct kvm_riscv_cove_page *cpage; > > + > > + if (!tvmc) > > + return -EFAULT; > > + > > + if (tvmc->finalized_done) { > > + kvm_err("measured_mr pages can not be added after finaliz= e\n"); > > + return -EINVAL; > > + } > > + > > + num_pages =3D bytes_to_pages(mr->size); > > + conf =3D &tvmc->confidential_region; > > + > > + if (!IS_ALIGNED(mr->userspace_addr, PAGE_SIZE) || > > + !IS_ALIGNED(mr->gpa, PAGE_SIZE) || !mr->size || > > + !cove_is_within_region(conf->gpa, conf->npages << PAGE_SHIFT,= mr->gpa, mr->size)) > > + return -EINVAL; > > + > > + idx =3D srcu_read_lock(&kvm->srcu); > > + > > + /*TODO: Iterate one page at a time as pinning multiple pages fail= with unmapped panic > > + * with a virtual address range belonging to vmalloc region for s= ome reason. > > I've no idea what code you had, but I suspect the fact that vmalloc'd mem= ory isn't > guaranteed to be physically contiguous is relevant to the panic. > Ahh. That makes sense. Thanks. > > + */ > > + while (num_pages) { > > + if (signal_pending(current)) { > > + rc =3D -ERESTARTSYS; > > + break; > > + } > > + > > + if (need_resched()) > > + cond_resched(); > > + > > + rc =3D get_user_pages_fast(mr->userspace_addr, 1, 0, &pin= ned_page); > > + if (rc < 0) { > > + kvm_err("Pinning the userpsace addr %lx failed\n"= , mr->userspace_addr); > > + break; > > + } > > + > > + /* Enough pages are not available to be pinned */ > > + if (rc !=3D 1) { > > + rc =3D -ENOMEM; > > + break; > > + } > > + conf_page =3D alloc_page(GFP_KERNEL | __GFP_ZERO); > > + if (!conf_page) { > > + rc =3D -ENOMEM; > > + break; > > + } > > + > > + rc =3D cove_convert_pages(page_to_phys(conf_page), 1, tru= e); > > + if (rc) > > + break; > > + > > + /*TODO: Support other pages sizes */ > > + rc =3D sbi_covh_add_measured_pages(tvmc->tvm_guest_id, pa= ge_to_phys(pinned_page), > > + page_to_phys(conf_page),= SBI_COVE_PAGE_4K, > > + 1, mr->gpa); > > + if (rc) > > + break; > > + > > + /* Unpin the page now */ > > + put_page(pinned_page); > > + > > + cpage =3D kmalloc(sizeof(*cpage), GFP_KERNEL_ACCOUNT); > > + if (!cpage) { > > + rc =3D -ENOMEM; > > + break; > > + } > > + > > + cpage->page =3D conf_page; > > + cpage->npages =3D 1; > > + cpage->gpa =3D mr->gpa; > > + cpage->hva =3D mr->userspace_addr; > > Snapshotting the userspace address for the _source_ page can't possibly b= e useful. > Yeah. Currently, the hva in the kvm_riscv_cove_page is not used anywhere in the code. We can remove it. > > + cpage->is_mapped =3D true; > > + INIT_LIST_HEAD(&cpage->link); > > + list_add(&cpage->link, &tvmc->measured_pages); > > + > > + mr->userspace_addr +=3D PAGE_SIZE; > > + mr->gpa +=3D PAGE_SIZE; > > + num_pages--; > > + conf_page =3D NULL; > > + > > + continue; > > + } > > + srcu_read_unlock(&kvm->srcu, idx); > > + > > + if (rc < 0) { > > + /* We don't to need unpin pages as it is allocated by the= hypervisor itself */ > > This comment makes no sense. The above code is doing all of the allocati= on and > pinning, which strongly suggests that KVM is the hypervisor. But this co= mment > implies that KVM is not the hypervisor. > I mean to say here the conf_page is allocated in the kernel using alloc_page. So no pinning/unpinning is required. It seems the comment is probably misleading & confusing at best. I will remove it. > And "pinned_page" is cleared unpinned in the loop after the page is added= +measured, > which looks to be the same model as TDX where "pinned_page" is the source= and > "conf_page" is gifted to the hypervisor. But on failure, e.g. when alloc= ating > "conf_page", that reference is not put. > Thanks. Will fix it. > > + cove_delete_page_list(kvm, &tvmc->measured_pages, false); > > + /* Free the last allocated page for which conversion/meas= urement failed */ > > + kfree(conf_page); > > Assuming my guesses about how the architecture works are correct, this is= broken > if sbi_covh_add_measured_pages() fails. The page has already been gifted = to the Yeah. The last conf_page should be reclaimed as well if measured_pages fail at any point in the loop. All other allocated ones would be reclaimed as a part of cove_delete_page_l= ist. > TSM by cove_convert_pages(), but there is no call to sbi_covh_tsm_reclaim= _pages(), > which I'm guessing is necesary to transition the page back to a state whe= re it can > be safely used by the host.