Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Fri, 15 Sep 2023 14:32:06 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>, Marc Zyngier <maz@kernel.org>,
        Oliver Upton <oliver.upton@linux.dev>,
        Huacai Chen <chenhuacai@kernel.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Anup Patel <anup@brainfault.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Paul Moore <paul@paul-moore.com>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>, <kvm@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <kvmarm@lists.linux.dev>,
        <linux-mips@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>,
        <kvm-riscv@lists.infradead.org>, <linux-riscv@lists.infradead.org>,
        <linux-fsdevel@vger.kernel.org>, <linux-mm@kvack.org>,
        <linux-security-module@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        Chao Peng <chao.p.peng@linux.intel.com>,
        Fuad Tabba <tabba@google.com>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        Anish Moorthy <amoorthy@google.com>,
        Yu Zhang <yu.c.zhang@linux.intel.com>,
        Isaku Yamahata <isaku.yamahata@intel.com>,
        Xu Yilun <yilun.xu@intel.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Vishal Annapurve <vannapurve@google.com>,
        Ackerley Tng <ackerleytng@google.com>,
        Maciej Szmigiero <mail@maciej.szmigiero.name>,
        David Hildenbrand <david@redhat.com>,
        Quentin Perret <qperret@google.com>,
        Michael Roth <michael.roth@amd.com>,
        Wang <wei.w.wang@intel.com>,
        "Liam Merwick" <liam.merwick@oracle.com>,
        Isaku Yamahata <isaku.yamahata@gmail.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [RFC PATCH v12 11/33] KVM: Introduce per-page memory attributes
Message-ID: <ZQP6ZqXH81V24Lj/@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230914015531.1419405-1-seanjc@google.com>
 <20230914015531.1419405-12-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230914015531.1419405-12-seanjc@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?cJ/xVeZqbZbggo9Tjs7RGGngV/kc34E/K19QCIH+UGGk5dpgfayBtzcjmJ4J?=
 =?us-ascii?Q?hRi1GfByQ1/sMkhg6cmDG+BTAKhq8vW91KaYutCDfTswfsJnddx9VO77Hh7t?=
 =?us-ascii?Q?QyPHP4ZWHpibXQOyXYjzbDsrYANxr4chxgM5mfpE30OJgDRI90dduk/RLCKi?=
 =?us-ascii?Q?IkQbRUVAcM8gADdin2olj3dNhwsajzgipHF73IzcxqR20IheDX9X2GXwbwuO?=
 =?us-ascii?Q?i3hBEE4XgUmrM8WReHqBvGkd6X9nCVocugi3lvwzIxvjvONUNa/40rafY5S9?=
 =?us-ascii?Q?SalJlqw8/P5romUexK1UmWcDiA2swyUJcHteFRYZ4d9abFEe7lXgSV13Zes9?=
 =?us-ascii?Q?zTZp6j/6+CHpD2UqT+bAkNYty0YsD7B6R1my65uOvawNRuGOIwNycd2F3kAq?=
 =?us-ascii?Q?rq0QGiBFO9l8iA+OQq3F3UNwctlU9VDclNGbbqhT9/Uv1ZvtPA7z4xIEVnQ9?=
 =?us-ascii?Q?A8eKLE0qxdY2tKmGvyc542vKf09m7/G+wBz++BUIYnwAPnUOh+CDV/ztT9u+?=
 =?us-ascii?Q?l5DTQlJEuJkxVSuNQrHbPPlYfybEFFfCgLyC/s7LuH6sqcqyxGdFdyLUxAIZ?=
 =?us-ascii?Q?XPDP8WXodLSJe0BglSVXBKpy3FZqwLO2HWneojBUqi15LOuC+iPytv8m880I?=
 =?us-ascii?Q?rKQbv8icrfRO7oy+7d9WIBGQq8TPuLc5q0FsjijQdkHaPWgIZoNFhfbgI0Ky?=
 =?us-ascii?Q?HP6npdAskBi2gWlWN/q2Im0l/zJGcAKx5PwoUapGiPE2eX65o4cEZ11Xa0fB?=
 =?us-ascii?Q?Xsb1ezRkkBK707Joek3uTdslbvmwDcpizUusZrWkptrfZd19eaz9opUTLOYO?=
 =?us-ascii?Q?rij46Yftz0JgKKZJECHYZmz7KX8GaLJHylr7dMnD7gjt6C/SGBEWEZHMy2hR?=
 =?us-ascii?Q?dIv8TCQaT0P3yd9aXo2rIbQOSA+P29YuKdHdRfwJZrti+ljZk21qT9tm2ENe?=
 =?us-ascii?Q?xvP1TD+XYgaRR+QzwzgLOxeL7yQiPf17bddTXGT/Ca8CNpJvNzKCcoNZoWyy?=
 =?us-ascii?Q?VhV00fbREkQO0JeRGYGF1j8lxV/UGA+w73Kjq0mmNFK4NPC1witSsDXCFPHS?=
 =?us-ascii?Q?7uiM8h7EEGODBbI7ZPPp0ZazgSOx30I+5d629+ArKktv3epC+Ik+pNpsoeNA?=
 =?us-ascii?Q?lgM32FbKW2W39nFBpDJoiTuIp+P1WSyzFMEm5UeDv5cSi20iNlfs4h7eOutX?=
 =?us-ascii?Q?eM8ysPAGfz8BoLkxfy9LYzIk477LKR+clYWXEeWZiZBfBh/VfuYnbgAnFuke?=
 =?us-ascii?Q?V6ZrlBvq4xsUkiPmhFcuF8WY4J61Z8IEUGp2ySvOguTd/+gWHTTryyFUNR9n?=
 =?us-ascii?Q?WaoEA8wikUm3FBgNXEeSAOOD4mgt04s9dZnjJpMQamfQXDn0d63XigeZGCwn?=
 =?us-ascii?Q?Qk8RwMbE/ZOEoOYW9NvF+/wUlJ9pWXAukTa7aaqxiW+Cbfhv8/JljPma2pdC?=
 =?us-ascii?Q?e4e/vL/QRFuhRXYVtqUdYCNaFyFd+UoNs7zqQTkIxv7C8vnb4TTD7JgNi5z2?=
 =?us-ascii?Q?zvwtr1g1lY/WgjCIRd47Hz6HxzGvuvJmKUnVHM4v9k7J1wXoTBFA9iNKJd0v?=
 =?us-ascii?Q?lgCVoW/Wo06h+F5JzTn96s4nfYvQFOAV48BiBITy?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4301786c-6c79-4955-7eee-08dbb5b9653e
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Sep 2023 07:00:07.6713
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: m06sK2+7ALEC7pkBv7zLGGHvlo71jRH/OwYBkf30C9UZOGi4hGOe5o2sCizMV6ce4HTBpLdB/ZGyMmYQnsLmJw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6181
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Fri, 15 Sep 2023 00:00:35 -0700 (PDT)

On Wed, Sep 13, 2023 at 06:55:09PM -0700, Sean Christopherson wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
> 
> In confidential computing usages, whether a page is private or shared is
> necessary information for KVM to perform operations like page fault
> handling, page zapping etc. There are other potential use cases for
> per-page memory attributes, e.g. to make memory read-only (or no-exec,
> or exec-only, etc.) without having to modify memslots.
> 
...
>> +bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> +				     unsigned long attrs)
> +{
> +	XA_STATE(xas, &kvm->mem_attr_array, start);
> +	unsigned long index;
> +	bool has_attrs;
> +	void *entry;
> +
> +	rcu_read_lock();
> +
> +	if (!attrs) {
> +		has_attrs = !xas_find(&xas, end);
> +		goto out;
> +	}
> +
> +	has_attrs = true;
> +	for (index = start; index < end; index++) {
> +		do {
> +			entry = xas_next(&xas);
> +		} while (xas_retry(&xas, entry));
> +
> +		if (xas.xa_index != index || xa_to_value(entry) != attrs) {
Should "xa_to_value(entry) != attrs" be "!(xa_to_value(entry) & attrs)" ?

> +			has_attrs = false;
> +			break;
> +		}
> +	}
> +
> +out:
> +	rcu_read_unlock();
> +	return has_attrs;
> +}
> +
...
> +/* Set @attributes for the gfn range [@start, @end). */
> +static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> +				     unsigned long attributes)
> +{
> +	struct kvm_mmu_notifier_range pre_set_range = {
> +		.start = start,
> +		.end = end,
> +		.handler = kvm_arch_pre_set_memory_attributes,
> +		.on_lock = kvm_mmu_invalidate_begin,
> +		.flush_on_ret = true,
> +		.may_block = true,
> +	};
> +	struct kvm_mmu_notifier_range post_set_range = {
> +		.start = start,
> +		.end = end,
> +		.arg.attributes = attributes,
> +		.handler = kvm_arch_post_set_memory_attributes,
> +		.on_lock = kvm_mmu_invalidate_end,
> +		.may_block = true,
> +	};
> +	unsigned long i;
> +	void *entry;
> +	int r = 0;
> +
> +	entry = attributes ? xa_mk_value(attributes) : NULL;
Also here, do we need to get existing attributes of a GFN first ?

> +	mutex_lock(&kvm->slots_lock);
> +
> +	/* Nothing to do if the entire range as the desired attributes. */
> +	if (kvm_range_has_memory_attributes(kvm, start, end, attributes))
> +		goto out_unlock;
> +
> +	/*
> +	 * Reserve memory ahead of time to avoid having to deal with failures
> +	 * partway through setting the new attributes.
> +	 */
> +	for (i = start; i < end; i++) {
> +		r = xa_reserve(&kvm->mem_attr_array, i, GFP_KERNEL_ACCOUNT);
> +		if (r)
> +			goto out_unlock;
> +	}
> +
> +	kvm_handle_gfn_range(kvm, &pre_set_range);
> +
> +	for (i = start; i < end; i++) {
> +		r = xa_err(xa_store(&kvm->mem_attr_array, i, entry,
> +				    GFP_KERNEL_ACCOUNT));
> +		KVM_BUG_ON(r, kvm);
> +	}
> +
> +	kvm_handle_gfn_range(kvm, &post_set_range);
> +
> +out_unlock:
> +	mutex_unlock(&kvm->slots_lock);
> +
> +	return r;
> +}