Date:   Fri, 17 Mar 2023 15:29:12 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>,
        Zhenyu Wang <zhenyuw@linux.intel.com>,
        Zhi Wang <zhi.a.wang@intel.com>, <kvm@vger.kernel.org>,
        <intel-gvt-dev@lists.freedesktop.org>,
        <intel-gfx@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
        Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v2 14/27] KVM: x86: Reject memslot MOVE operations if
 KVMGT is attached
Message-ID: <ZBQWyPF7Py2RTGAH@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230311002258.852397-1-seanjc@google.com>
 <20230311002258.852397-15-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230311002258.852397-15-seanjc@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?9pY2aUutJ7pq0Vt5v5kEcRiUUJSCoBTw+dBmrEUlJ2woQzc2lU/9KnkHEOKk?=
 =?us-ascii?Q?Lf4Ot80A1HzYxRZxzXRzYx5ZYmITAgemaDGwzABzoRw1MTwAXIJFUBsGGmho?=
 =?us-ascii?Q?MFD4UfBfVrtjoqoMK0zu3wyN49nMzz7Apyvwr18OKWXitcUUtgbKNldYjW7h?=
 =?us-ascii?Q?zhuPdXqjhYgYDhGn+oKnYdki2WmernGkPDv8W7AfR11oNJcPEXqtv5KTmeab?=
 =?us-ascii?Q?FanCB5FRATQPY2vF7Pbs0CWcAJdTRDieELnyGSvwLlwiNT+cYIE70tHD24Bq?=
 =?us-ascii?Q?JB7xnwmLrgLKPgFmn6A55XoV1P/zefzlupHoY6GnhGZrBRJjaT75/ZyZIO7a?=
 =?us-ascii?Q?yR4zZfKQxZD1EDw/KttXKq2A0WDow9UVNGkwvmFxGefa4RgLUHF0R2l+3YX4?=
 =?us-ascii?Q?AkaTq6qURRbALK6oh47ut6uryeQSw4aD4cR+e1DGAQ6ikHtTgJveLNvzMr2v?=
 =?us-ascii?Q?/PhTuETtad4wj0qyHrXAHYKCqcnL6n3oHo2cslvkRnOG5eMlSTcjvR8t/l87?=
 =?us-ascii?Q?lj89IlrqWVdPfdwxz22SwHXnBq1W3bfsRsTkJ/BZw1LVEYRUdsvTQBo5cGLe?=
 =?us-ascii?Q?3IUhc/I/Gi+aolBN2pjsgNR+hZ5ZNvkzLg55IkRAAdlaxv08IHll8sho1O9U?=
 =?us-ascii?Q?n5eOTKeTf4i2sWlallQ+RUy1tOpB4Q4ewoT/7SU7z804u854QlINoyzQzQDb?=
 =?us-ascii?Q?Q/jlTXl3aWSholh30L24CSmksE/ozJYgx6YpOhijNXHHJwDzr2UTI1JFuFCp?=
 =?us-ascii?Q?Pncl2TYkdw21vMERsnYjdYxn2BwT9/wMi+Rv9/o9tv1pZVK+D7qTek+HTzAu?=
 =?us-ascii?Q?YykLM5RL/o7aBRCyIh/cZGbS79MLZ4bJz5kZWoZysyx8EiN4Dv8ZGREOKN3K?=
 =?us-ascii?Q?RB068y8McvVjBCWmURELBMOsNPnmc8sS2AHliVN89zV+N9nIQBVKrzIuFq3m?=
 =?us-ascii?Q?jjUYKWtF7ix9lHbIdWXOhu8UcMuMRqgvgIxDvvCgnnfiaWvf+lNny4eifxL5?=
 =?us-ascii?Q?bNLPJohSjdMoA7E8aVfkjdZdQH0wCvfuV532wTxfmtpC3Rvp7YbBssrOnkLn?=
 =?us-ascii?Q?ZF3YL9FvSxSLYrfYaE98aQY7LGxJmIOJlN8UF508fEp5nzE0atJNRVPZsQmf?=
 =?us-ascii?Q?4nGg+iKoqm0fO6KjYg/g7msciUy/bB/wV9vuUffSiJmro+77p5adyI7/QIdo?=
 =?us-ascii?Q?h8ACwyFpMgny+960H6FolqMeTxPsL7qCxwm/fwoV/5u8vpKcO4QCm9AnYQ47?=
 =?us-ascii?Q?VJeB/ykfj3haTAynH7aUht3Dc1jsVx7ylEWhKHCzTeceKtP2cEhRvhinZFpS?=
 =?us-ascii?Q?pZtRIEoge6r3dwQXm3NZzPsrfIScHsyKvtG+WLYwwOhs3AhzQEOq7LI3PT1U?=
 =?us-ascii?Q?mGSG65Ms4zLajz7Wa4jmcXTFizz6nKWKXPJqYCiPsfvWif8rg200qcdCyQ/z?=
 =?us-ascii?Q?vErhAlLKj/ZWW0gjizpJsAAZhGd61+LnW2p3+A8KjgrdsYLP64abhUw0j8Ax?=
 =?us-ascii?Q?YNVJYfat6JU79Giy2H+FCQnev8ZHUR3nQRKy98so5Nntpv+cLKgQiEEwQeVX?=
 =?us-ascii?Q?2/MkIQDanXNlFsU3yWyMXoPiVannmeo5VXLfBSptYFrX+00hLCmte88zpXTq?=
 =?us-ascii?Q?ng=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4c44817e-66c8-4c2a-18e4-08db26bcbf70
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Mar 2023 07:53:51.2256
 (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: tkKcjR6MynEBL7JwcCKOL9M+tIhxv4e4vJAQpsFqJd6sNXoGJ4lQiM86IlZ0PttJ+u+g3PUkOSw/MRUNDbqg/w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB6272
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>

On Fri, Mar 10, 2023 at 04:22:45PM -0800, Sean Christopherson wrote:
> Disallow moving memslots if the VM has external page-track users, i.e. if
> KVMGT is being used to expose a virtual GPU to the guest, as KVM doesn't
> correctly handle moving memory regions.
> 
> Note, this is potential ABI breakage!  E.g. userspace could move regions
> that aren't shadowed by KVMGT without harming the guest.  However, the
> only known user of KVMGT is QEMU, and QEMU doesn't move generic memory
> regions.  KVM's own support for moving memory regions was also broken for
> multiple years (albeit for an edge case, but arguably moving RAM is
> itself an edge case), e.g. see commit edd4fa37baa6 ("KVM: x86: Allocate
> new rmap and large page tracking when moving memslot").
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/kvm_page_track.h | 3 +++
>  arch/x86/kvm/mmu/page_track.c         | 5 +++++
>  arch/x86/kvm/x86.c                    | 7 +++++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/arch/x86/include/asm/kvm_page_track.h b/arch/x86/include/asm/kvm_page_track.h
> index 0d65ae203fd6..6a287bcbe8a9 100644
> --- a/arch/x86/include/asm/kvm_page_track.h
> +++ b/arch/x86/include/asm/kvm_page_track.h
> @@ -77,4 +77,7 @@ kvm_page_track_unregister_notifier(struct kvm *kvm,
>  void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
>  			  int bytes);
>  void kvm_page_track_flush_slot(struct kvm *kvm, struct kvm_memory_slot *slot);
> +
> +bool kvm_page_track_has_external_user(struct kvm *kvm);
> +
>  #endif
> diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
> index 39a0863af8b4..1cfc0a0ccc23 100644
> --- a/arch/x86/kvm/mmu/page_track.c
> +++ b/arch/x86/kvm/mmu/page_track.c
> @@ -321,3 +321,8 @@ enum pg_level kvm_page_track_max_mapping_level(struct kvm *kvm, gfn_t gfn,
>  	return max_level;
>  }
>  EXPORT_SYMBOL_GPL(kvm_page_track_max_mapping_level);
> +
> +bool kvm_page_track_has_external_user(struct kvm *kvm)
> +{
> +	return hlist_empty(&kvm->arch.track_notifier_head.track_notifier_list);
> +}
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 29dd6c97d145..47ac9291cd43 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -12484,6 +12484,13 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
>  				   struct kvm_memory_slot *new,
>  				   enum kvm_mr_change change)
>  {
> +	/*
> +	 * KVM doesn't support moving memslots when there are external page
> +	 * trackers attached to the VM, i.e. if KVMGT is in use.
> +	 */
> +	if (change == KVM_MR_MOVE && kvm_page_track_has_external_user(kvm))
> +		return -EINVAL;
> +
>  	if (change == KVM_MR_CREATE || change == KVM_MR_MOVE) {
>  		if ((new->base_gfn + new->npages - 1) > kvm_mmu_max_gfn())
>  			return -EINVAL;
> -- 
> 2.40.0.rc1.284.g88254d51c5-goog
>