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 F1842C7EE30 for ; Wed, 1 Mar 2023 23:37:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2F7436B0078; Wed, 1 Mar 2023 18:37:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 27F436B007B; Wed, 1 Mar 2023 18:37:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0FC726B007D; Wed, 1 Mar 2023 18:37:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id F0A286B0078 for ; Wed, 1 Mar 2023 18:37:29 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id B81574025B for ; Wed, 1 Mar 2023 23:37:29 +0000 (UTC) X-FDA: 80521943418.20.EEB3852 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by imf25.hostedemail.com (Postfix) with ESMTP id 51DA5A001A for ; Wed, 1 Mar 2023 23:37:26 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=EvuQJ7T3; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf25.hostedemail.com: domain of dave.hansen@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=dave.hansen@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677713847; 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=w6jzJkY6KInduFeYO89hwte5Ps6cBmDcXL1dWBnigAo=; b=5ZKhv0B/CJT05+AiBs1vYr8zuebDWN6lyvUBDpcxkpZuyKh8mrtappSlyzep94XAV3Ptsy SXu1RoQeNAl8Ursc21bCtDb6QGSszmAqlfItsKVm2kx/06QqOTYeUX55b5f+mqV4AiJULo VzB6b4njJIUFYFKhX0yeO7S9jrp0El4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=EvuQJ7T3; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf25.hostedemail.com: domain of dave.hansen@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=dave.hansen@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677713847; a=rsa-sha256; cv=none; b=w7iUguHzjG/xH6dbW52Ja5DiLm7gH+vUv+t1ygwgmZbXAJJFnKuarnvuOJqeiR+3Tg4fs1 ThV066SZOF4HLT0DRT5K9NFI1Tp0j4WgRcjay2eSf/ko8wwyMQbSaZP+/LbvIDvZJxuKc8 UEq3JEZbGeWPJla6AihgRiAamisqYZw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1677713846; x=1709249846; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=FgO69G8MZkaqniX2vBowz6BTvzzRuECLqSJBvz0Dm10=; b=EvuQJ7T3Vu4DZG5zPBQXBkVH54s3xICHjm7KZGnHQxqcow9Lw6whqNOh RW6rBM1TwO93W52vZ6+l7+tTEZsknE7WcXNuCaVULa1ZAA01T0y4fU5La e4ZnebKWz7JuZBknrb25yoCwgyVuYeehuObqWNQjsO4rwJuGFKZanAwoi e7OdIoIOSgcEIN0kXsPI4Hi1zIpzFfNv9GWRhq5YdgEYV03WF6ED/d3Ut mPPLYYSy5Yn4IFIyFKaynM4hYcYqUwQEgSsGRCKOADi6Zj1pds7ncBSdX PVNyRcmcExdrzbepYl1JHKJIMKQJa1/SuXh4+0/77sXwb8ebxU5wiWX3m w==; X-IronPort-AV: E=McAfee;i="6500,9779,10636"; a="314223232" X-IronPort-AV: E=Sophos;i="5.98,225,1673942400"; d="scan'208";a="314223232" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Mar 2023 15:37:01 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10636"; a="705033304" X-IronPort-AV: E=Sophos;i="5.98,225,1673942400"; d="scan'208";a="705033304" Received: from asaha3-mobl1.amr.corp.intel.com (HELO [10.251.12.67]) ([10.251.12.67]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Mar 2023 15:37:00 -0800 Message-ID: Date: Wed, 1 Mar 2023 15:37:00 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH RFC v8 48/56] KVM: SVM: Add SNP-specific handling for memory attribute updates Content-Language: en-US To: Michael Roth , kvm@vger.kernel.org Cc: 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, seanjc@google.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, marcorr@google.com, sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com, dgilbert@redhat.com, jarkko@kernel.org, ashish.kalra@amd.com, nikunj.dadhania@amd.com References: <20230220183847.59159-1-michael.roth@amd.com> <20230220183847.59159-49-michael.roth@amd.com> From: Dave Hansen In-Reply-To: <20230220183847.59159-49-michael.roth@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 51DA5A001A X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: 51ct97fuqdj7ccdur1ki8s8fknaocyqb X-HE-Tag: 1677713846-128439 X-HE-Meta: U2FsdGVkX1/tMYiJ9d1tSGD/Gs7eF2sYxiA86d506xcIDr9gTCNmEKpiS2U25hL/5PAOSxJZRvufOSKaGEGJ2QdR8KZwt3yjrjCv06dF46t4+EzFQV1n/AVFliFtNI/nO5NpkuevDzCk5hM2qIT+UIM81kY1RzrFbY9F//eoDMNQkKQ3edi4D7MdveT2GB1AH+Z+ZuI+rUZY2sIky7vLOk23ySewyofuBvxhQXPKf7CAsjeELgbRqmpm0Ekxy8Y/vJTS4xrJDhG/fmyOujfB7xryd3hh3iYQU4Wqiw5Luhso+4b1Cswad6ZuO0uYZeosJwuGXUWywSpK+3ZqTnODnaccWlgeqSNNgaXykeUCySSIM1npotszTiRua9Iy7VhyBqC6XGtjyNUM4ha+WU7J+J05FDc/QfEk6Dl5KusNpAhT/Us7sY827dFU7/YA+lKiIxnMde9xlE7dIQV1Da0YiQJiJdLEBj0Cvj/6+0TpiZL7efCcaXFzXU3wUc/b5L4dS0e9BQewG6xtoZdfTMlMkHUjcrkdAHH55dmrPhTGvbWPmEAUNc560WjdpGup5fMrUlwvoTLUNiZHdKHEG6pUye2s31ci6oQBF/FZ/9IQIPzC4juAMA/svVNcYdaIvfHcLOIATO2lyiCc/nw56FLJVubpdnM7ihFNZiD/RAioDHTiAOhBLyM2MDBHxKd6ssaGGkcCI8Y2ctVsi/dEXm/ZBuEOzAzUnYp9gQfSjAKVl58KtLzNNhj9IxLhor9/FYTzKCKOYCzlds5C6xdVtJCgTDkajpUa+hjdPi/6XhokpJvv6LDmBzV8lxABzh/12YBvcBw9GEbtZYT+WDiaUEt4EA3ILIXIrH/jsfYeGP1EmxkX6OcDwXbuE0x9HKSNl7YwrUNoEfp9Lteaz1X23Ei3HGvv/1RAXm3DQ1ncdG51X7xTIbQUpd9UObb6Rn+07eiqoK/VyEJJsqTFQfsJwN5 We7ZMyTO I0swQ7dPRovVWCHpowCCl/HARK1BTitYnhPAOF/SkUlyuK13eU8b+SgT1cvNKIP84UVStOVVywDksZ3kn8C1Y7uU2+e4QfSi/onqa/RBIcQJLXh/sBqTP/+Q6zqXNYz52rGp2vsYAScCgNQkV4ICnQLMchCLkwb6SAk+MDoC2lGVrPD5ethJhiInsWw== 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 2/20/23 10:38, Michael Roth wrote: > + /* > + * TODO: The RMP entry's hugepage bit is ignored for > + * shared/unassigned pages. Either handle looping through each > + * sub-page as part of snp_make_page_shared(), or remove the > + * level argument. > + */ > + if (op == SNP_PAGE_STATE_PRIVATE && order && > + IS_ALIGNED(gfn, 1 << order) && (gfn + (1 << order)) <= end) { > + level = order_to_level(order); > + npages = 1 << order; > + } That's a wee bit obtuse. First of all, I assume that the 'RFC' is because of these TODOs and they won't survive to the point when you ask for this to be merged. BTW, what keeps the restrictedmem_get_page() offset and the gfn aligned? Let's start with this: > +static inline u8 order_to_level(int order) > +{ > + BUILD_BUG_ON(KVM_MAX_HUGEPAGE_LEVEL > PG_LEVEL_1G); > + > + if (order >= KVM_HPAGE_GFN_SHIFT(PG_LEVEL_1G)) > + return PG_LEVEL_1G; > + > + if (order >= KVM_HPAGE_GFN_SHIFT(PG_LEVEL_2M)) > + return PG_LEVEL_2M; > + > + return PG_LEVEL_4K; > +} Right now, 'order' comes only from restrictedmem_get_page(), which I dug out of: > https://github.com/mdroth/linux/commit/c6792672cd11737fd255dff10b2d5b6bccc626a8 That order is *only* filled in by THPs. That makes the PG_LEVEL_1G stuff here kinda silly. I guess it might be seen as thorough, but it's dead code. I'd probably just make this work on order==9 || order==0 and warn on anything else. I'd also highly recommend some comments about how racy this all is. I guess it probably works, but it would be good to add some comments about page splits and collapsing. It's also not obvious why this only cares about private pages. Anyway, this is the exact kind of thing where I really like a well-commented helper: bool can_install_large_rmp_entry(gfn, order) { // small pages, blah blah if (!order) return false; // The region being updated must be aligned if (!IS_ALIGNED(gfn, 1 << order)) return false; // ... and fit if (gfn + (1 << order)) > end) return false; return true; } Which gets used like this: if (op == SNP_PAGE_STATE_PRIVATE && can_install_large_rmp_entry(gfn, order)) { level = ... }