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 4E6A9C3DA78 for ; Fri, 13 Jan 2023 22:50:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9A2F28E0005; Fri, 13 Jan 2023 17:50:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 951A28E0001; Fri, 13 Jan 2023 17:50:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 81A238E0005; Fri, 13 Jan 2023 17:50:30 -0500 (EST) 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 71F348E0001 for ; Fri, 13 Jan 2023 17:50:30 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 3F9A11408B4 for ; Fri, 13 Jan 2023 22:50:30 +0000 (UTC) X-FDA: 80351271420.24.7E08689 Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by imf22.hostedemail.com (Postfix) with ESMTP id 90BF7C000E for ; Fri, 13 Jan 2023 22:50:28 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=X76rLWdZ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf22.hostedemail.com: domain of seanjc@google.com designates 209.85.216.47 as permitted sender) smtp.mailfrom=seanjc@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673650228; 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=yQZCEBEaghsOR+zhl/QRIXPVX+uCKPvU91VQuy/V94s=; b=GaJi81ByQ4eqbyGytUcex4l7bGZhKUqxM+KMJIkarWK4N0aBOkJiyN8UvQhAGaiI7L1dJJ /qg5PP43Juk1uaB6rhOo7UiH69hGvm82faXBxFWB030ZFmcaHFTRPqQ0Hcv5YXQD4pzMmh nQKmAxizKknJh8UEC49EvWBepW7mqLc= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=X76rLWdZ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf22.hostedemail.com: domain of seanjc@google.com designates 209.85.216.47 as permitted sender) smtp.mailfrom=seanjc@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673650228; a=rsa-sha256; cv=none; b=4nmAiGz4pWOaLyQzya4QDDBkU2yLUT17pBcsGcHj7lCrmLhC5R6EnkRLjn41tTkmD8Sqeq blymUmJsfF4NG8LEWwy4uOESBxYwtJX/9WPp4rTmNymA7ahcR7+fLHm5/Czc4glNI/2H+i lykJQTjsNxbyVqsBmOJJVv5vrFxgu0E= Received: by mail-pj1-f47.google.com with SMTP id o7-20020a17090a0a0700b00226c9b82c3aso25817777pjo.3 for ; Fri, 13 Jan 2023 14:50:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=yQZCEBEaghsOR+zhl/QRIXPVX+uCKPvU91VQuy/V94s=; b=X76rLWdZrm8cdm781bGE/NuVpMIrvPOi9lGhvncTfFs81VVxsQB/QGGTWKxHh1GL1W d/AoRzt/BgZEnefw0frAVABgAhcfb0NjbvQQ6PThCX1YXedTBAqOobICTThGoiP6om5q O3KMRfb6M32Bg7CVQSQvIy3h9U9bkySatLBmzLp/OFGTsGTVJq+w1gHMbDBjfBgq7rHh ADYr+PPp2TF3nGbuwOKFE5/4euBQA3ZQaQSM4gqqLZGzGNJeHa+tfxiLoUS2z+t4YmPi kKxrLU/4iehY+TcpZNW1WLdBUrSh7cY4mTcXmXq7RCEGkbTb53lk/EI3QzzxovpqZqg9 0htA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=yQZCEBEaghsOR+zhl/QRIXPVX+uCKPvU91VQuy/V94s=; b=cTmCv3kZcwR1JHESEjURmbndnXyiSELh5ahYEufz95bwCjhE2j7X55zlzJjedTUdsX 75DW9zXKTEOkbuZ+/zbTgBsmNms5JE+js6yV4+NfU4O/yzyVQ4R6snuP5axq8aHecHc+ t9pGma7d9KvKGY80DkeFmhDVUhiHnzBqSBECAbXtDY4DUjNK76pzypMACPeYZEBFCZC5 B7QvOkBvWQnd4BrqARgrJHYfptp/M0JwrTTgEdVYBdXIh5aQlIGQUMB0k12erCDPTRs3 rpwJJ5K7nbmiVup39GK//vmLXJmcmpKIFqgOC5Ggm5TKAknZCVcVPsKC1rYbnBowsCrq nikA== X-Gm-Message-State: AFqh2kpB9cMUPT2c9DvqSKkoRMNdCxQzW/m/szHmKQOZbe7qbeWcB9G/ DlTG7vN6OAxwTZwgv5RL1SXfQA== X-Google-Smtp-Source: AMrXdXuS4zXTytfOZ5xIh4bs+rkmeZmK+QXoBwGpkC3p2EnL8ac6bXz6nz3yQ+yKSuiIei2pYWQMCA== X-Received: by 2002:a17:902:b10e:b0:191:4367:7fde with SMTP id q14-20020a170902b10e00b0019143677fdemr1360463plr.0.1673650227268; Fri, 13 Jan 2023 14:50:27 -0800 (PST) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id 13-20020a170902c24d00b0019460ac7c6asm3819704plg.283.2023.01.13.14.50.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Jan 2023 14:50:26 -0800 (PST) Date: Fri, 13 Jan 2023 22:50:22 +0000 From: Sean Christopherson To: Chao Peng Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-api@vger.kernel.org, linux-doc@vger.kernel.org, qemu-devel@nongnu.org, Paolo Bonzini , Jonathan Corbet , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Arnd Bergmann , Naoya Horiguchi , Miaohe Lin , x86@kernel.org, "H . Peter Anvin" , Hugh Dickins , Jeff Layton , "J . Bruce Fields" , Andrew Morton , Shuah Khan , Mike Rapoport , Steven Price , "Maciej S . Szmigiero" , Vlastimil Babka , Vishal Annapurve , Yu Zhang , "Kirill A . Shutemov" , luto@kernel.org, jun.nakajima@intel.com, dave.hansen@intel.com, ak@linux.intel.com, david@redhat.com, aarcange@redhat.com, ddutile@redhat.com, dhildenb@redhat.com, Quentin Perret , tabba@google.com, Michael Roth , mhocko@suse.com, wei.w.wang@intel.com Subject: Re: [PATCH v10 6/9] KVM: Unmap existing mappings when change the memory attributes Message-ID: References: <20221202061347.1070246-1-chao.p.peng@linux.intel.com> <20221202061347.1070246-7-chao.p.peng@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221202061347.1070246-7-chao.p.peng@linux.intel.com> X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 90BF7C000E X-Stat-Signature: xzsxhst69ahoatmytomyyuh3x8qd6ci3 X-HE-Tag: 1673650228-804945 X-HE-Meta: U2FsdGVkX19mu8fbpEzn2P2hHvHk9WZGXDa5vkdz0ORPWS7D2VVuFNGaGqhA3+nywUnUElKp/+FL3mPRyIingllKS7Kl4KwCQU+CbuacM/XW6Yy6BCbSUYte4H5vfTRwfmobhv3Nuvub8zLnI9Qrt0rjZTms9TxV7MSxM55RQCkKh2XQ+zKnhVWVVJ1GqyZxsSWliwJGaan096EsKvdaCTiFJn6Jq1DjOe4M3MZI3H94AzrmIxfbBAoNaIKuYcM2NmC9/8SoUexYEOI7NUVWioEv/dkt26WcdtiNZJHyHAWEcZrTgRC8hFtPi+AfXitnhfkTaZHoXQCretygZKtoYc1nnGZGs+HFpD8LdyGGLui9fcNom2gVuvaQPLTMl3wNOIcLaMIFiweNodxRLILqiIFgqQQTM3THN9W2HSkXk/MW1r/sEo3rDc+Z8GrqxB8UchvXAmR8Ts+SYJA5Vkq0rje7CczoiMt4/8wTVQIKEBzoqjFdmlWh2apr5Y8d7IdkJafuJKTrNyZ8vTv4AGCfvnrH/Mg9n/yN2ko6OCnrA5WMwiqX4e11NvFBgig1YW+Do8jW9j8p/wXf8UgBWCEVnbw8q9ns7OigikiCy3qeUavvwyPOgSkDADfTmvsD9AwwbgbeTtnFA7iWR0eDeND8oaa4oK86YcgN/+XOIDWhRPKlGd1OSXdoN8Ydf6BguT64YqyYdCR7z6/PLxik8zIJ75lDM9rEjKBOo8THB7GKtTldfse/3LDdC3LvwjUNrhgKxNpE67HpeeMNHkSoXj7DMwMFT9HuJbvRAKzhjjZdtkbgG/KGvxH5exl5aGd1CwMuGZxGzZDv54FEZ9l/dg2yFrvqYZGEW7lc5UXiKEXNzDFAr2CJoSXyttg+ydGwwbuU3LpxxPsKlt3zmHYqJ8Pm/ZhCsnUTv1WM8TguZCxZwWfyFzSnqFsrRJ5QlCXtkzaD6LGPcEcEMRB1a/ZtnBA ndef4zNY 5UzeUwIr+krcmH7Qy7y9wFKkh7uykATJjdXRbSlhseIX8gGZl2FtSo96FMJ486aYlasGnAq5cHuOgQGr6lj9tcEW58aUxGQcYpk50 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 Fri, Dec 02, 2022, Chao Peng wrote: > @@ -785,11 +786,12 @@ struct kvm { > > #if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER) > struct mmu_notifier mmu_notifier; > +#endif > unsigned long mmu_invalidate_seq; > long mmu_invalidate_in_progress; > gfn_t mmu_invalidate_range_start; > gfn_t mmu_invalidate_range_end; > -#endif Blech. The existing code is a bit ugly, and trying to extend for this use case makes things even worse. Rather than use the base MMU_NOTIFIER Kconfig and an arbitrary define, I think we should first add a proper Kconfig, e.g. KVM_GENERIC_MMU_NOTIFIER, to replace the combination. E.g config KVM_GENERIC_MMU_NOTIFIER select MMU_NOTIFIER bool and then all architectures that currently #define KVM_ARCH_WANT_MMU_NOTIFIER can simply select the Kconfig, which is everything except s390. "GENERIC" again because s390 does select MMU_NOTIFER and actually registers its own notifier for s390's version of protected VMs (at least, I think that's what its "pv" stands for). And then later down the line in this series, when the attributes and private mem needs to tie into the notifiers, we can do: config KVM_GENERIC_MEMORY_ATTRIBUTES select KVM_GENERIC_MMU_NOTIFIER bool I.e. that way this patch doesn't need to partially expose KVM's notifier stuff and can instead just keep the soon-to-be-existing KVM_GENERIC_MMU_NOTIFIER. Taking a depending on KVM_GENERIC_MMU_NOTIFIER for KVM_GENERIC_MEMORY_ATTRIBUTES makes sense, because AFAICT, changing any type of attribute, e.g. RWX bits, is going to necessitate unmapping the affected gfn range. > struct list_head devices; > u64 manual_dirty_log_protect; > struct dentry *debugfs_dentry; > @@ -1480,6 +1482,7 @@ bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu); > int kvm_arch_post_init_vm(struct kvm *kvm); > void kvm_arch_pre_destroy_vm(struct kvm *kvm); > int kvm_arch_create_vm_debugfs(struct kvm *kvm); > +bool kvm_arch_has_private_mem(struct kvm *kvm); The reference to private memory belongs in a later patch. More below. > +static void kvm_unmap_mem_range(struct kvm *kvm, gfn_t start, gfn_t end) > +{ > + struct kvm_gfn_range gfn_range; > + struct kvm_memory_slot *slot; > + struct kvm_memslots *slots; > + struct kvm_memslot_iter iter; > + int i; > + int r = 0; The return from kvm_unmap_gfn_range() is a bool, this should be: bool flush = false; > + > + gfn_range.pte = __pte(0); > + gfn_range.may_block = true; > + > + for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) { > + slots = __kvm_memslots(kvm, i); > + > + kvm_for_each_memslot_in_gfn_range(&iter, slots, start, end) { > + slot = iter.slot; > + gfn_range.start = max(start, slot->base_gfn); > + gfn_range.end = min(end, slot->base_gfn + slot->npages); > + if (gfn_range.start >= gfn_range.end) > + continue; > + gfn_range.slot = slot; > + > + r |= kvm_unmap_gfn_range(kvm, &gfn_range); > + } > + } > + > + if (r) > + kvm_flush_remote_tlbs(kvm); > +} > + > static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm, > struct kvm_memory_attributes *attrs) > { > gfn_t start, end; > unsigned long i; > void *entry; > + int idx; > u64 supported_attrs = kvm_supported_mem_attributes(kvm); > > - /* flags is currently not used. */ > + /* 'flags' is currently not used. */ Kind of a spurious change. > if (attrs->flags) > return -EINVAL; > if (attrs->attributes & ~supported_attrs) > @@ -2372,6 +2409,13 @@ static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm, > > entry = attrs->attributes ? xa_mk_value(attrs->attributes) : NULL; > > + if (kvm_arch_has_private_mem(kvm)) { I think we should assume that any future attributes will necessitate unmapping and invalidation, i.e. drop the private mem check. That allows introducing kvm_arch_has_private_mem() in a later patch that is more directly related to private memory. > + KVM_MMU_LOCK(kvm); > + kvm_mmu_invalidate_begin(kvm); > + kvm_mmu_invalidate_range_add(kvm, start, end); > + KVM_MMU_UNLOCK(kvm); > + } > + > mutex_lock(&kvm->lock); > for (i = start; i < end; i++) > if (xa_err(xa_store(&kvm->mem_attr_array, i, entry, > @@ -2379,6 +2423,16 @@ static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm, > break; > mutex_unlock(&kvm->lock); > > + if (kvm_arch_has_private_mem(kvm)) { > + idx = srcu_read_lock(&kvm->srcu); Mostly for reference, this goes away if slots_lock is used instead of kvm->lock. > + KVM_MMU_LOCK(kvm); > + if (i > start) > + kvm_unmap_mem_range(kvm, start, i); > + kvm_mmu_invalidate_end(kvm); > + KVM_MMU_UNLOCK(kvm); > + srcu_read_unlock(&kvm->srcu, idx); > + } > + > attrs->address = i << PAGE_SHIFT; > attrs->size = (end - i) << PAGE_SHIFT; > > -- > 2.25.1 >