Date:   Fri, 17 Mar 2023 15:45:26 +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 17/27] drm/i915/gvt: switch from ->track_flush_slot()
 to ->track_remove_region()
Message-ID: <ZBQalmUGAHyRQ2sg@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230311002258.852397-1-seanjc@google.com>
 <20230311002258.852397-18-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230311002258.852397-18-seanjc@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mDkNj95p5jcHt4f/1sjxv8smczwCH7zHJnIyiK+9yAq8OlNeK7P4A84a4rki?=
 =?us-ascii?Q?12wDEqy1bzO16pSk9YG4mi2eGaTyBDxdGIKGbTnwjpVvzDUOwDi5Gf9o8CVi?=
 =?us-ascii?Q?s/ej7ELXzq6yxKtik0NJ670v7nKHEcKJVEAz8wdzQ1l3N9rqkF24KfSYY/TM?=
 =?us-ascii?Q?0JQycvJNHky2aRn7piUAZM1ND3ICIG1qJfp98p9bj4RbdqdFMV9+/gW+sOpU?=
 =?us-ascii?Q?nfAaa/tGnr+KfVoKXUVnyVNKaCjBHzRfZs9LXj5LZCeHBfCWZLM9DeEpNBVj?=
 =?us-ascii?Q?yREJLuRtmKgv2DG+7IX39Bi8rseivICpU45A8/K+hX6la7Rx3fEQPJAnlL1/?=
 =?us-ascii?Q?OuTqZCcGg+BxjXmj7f7H3lleRbBqLhHate+XndxASPF2oVcYC7oznw0GbLEz?=
 =?us-ascii?Q?kxOa/9mhxh/cuc8UDoykQ9iRQP1/lk3mm0LNF/QLE48znLnZweqouHJ5QYg6?=
 =?us-ascii?Q?5l4aNd7BV7t61CMjiX+2SMWW7BGQN6qiEDdPxv4OEKg7Fow3EPNsk6waAM2I?=
 =?us-ascii?Q?fCnkODDa0vbWvhUX2l2AiEp4QdBjrTfw2jqOrp6u/E6V4cKsS3ZbP4DRZBvC?=
 =?us-ascii?Q?wGuLTjst5jq7KEvBZQnxyob7m2P2oX2K8DnPHeXj5PnhF9s3vAcPb0K6LbwA?=
 =?us-ascii?Q?vCzgBN4hLev2EclLEv5/su/TeYwSqxs/bDLsZX8wcj14mkdsAbx+o7XZEMYl?=
 =?us-ascii?Q?bUWIP+0u3sb+cvZBB1PX0G2qNYnI3E7F7cKr5cSamR+IrYOVVdIMS46TwZ6q?=
 =?us-ascii?Q?I8M/OVZRC/m24nubVZRdTKLBYjxHAx3KVuGtn1Ruzh4OnfU+KAtJXIGgSAm7?=
 =?us-ascii?Q?h/7lpxUzu66Y9TTHg8esh6DG+0WVz5rZrON//jqELuYel4NR60eU0fKx+sk/?=
 =?us-ascii?Q?6jo8phWchulQYVC6OCpV7w+8S+4FtH6ohtWRL39gnlFaOJuSSU8Gr0jlbtwJ?=
 =?us-ascii?Q?pfUTTnuglrwTPuKQTPyVsm49yjvTHs2ye97/hcYWcALS7G6xn1ISKUusjDn0?=
 =?us-ascii?Q?OjpT/O8eYroCU/G5wA5MgW/57bvFlsQCRfrS3Hx2aj92NdhugR44L82KwRMu?=
 =?us-ascii?Q?i9X7i9aD2nkijd9zTJDcW9xjSCoCL0VgzimO3cHYnTbhmTYIPAPaf6DpFa3R?=
 =?us-ascii?Q?Hr4hnomYzKuzVe4EDn6dJGOgr04lpFHbYup88glXVCe8JiwMnYQA1dtPq4vp?=
 =?us-ascii?Q?DrcQbFa96IkwHyAn0bnP+EVAOhVcB7cCuBXQIfBWK5ncX2sbQnuj15OUalUm?=
 =?us-ascii?Q?sKSnHeIJA8zjnaJ0HIQ3ehZtitBjISm1wH4rcxA2Ms2Eul5kKfbW1N0CX3F2?=
 =?us-ascii?Q?4EKSxBIldU6TtG6HN0CYPrhFyRPsYYtn3gZLOZWExwn5Ogyh0PsEJgqj/qQK?=
 =?us-ascii?Q?FwVBD9pLqkKBmZ+vZHNQIL5ZIWoCeSInv2sNiZnNqmRH+fh1Mm82TSBAQ5Gd?=
 =?us-ascii?Q?5wcQ0FCS7wdhL8L7W6r62qTpOtjeJFZf9MbTtiwDLwNN48Xkl0Z4IOUBTTxf?=
 =?us-ascii?Q?ITGwyz1M9XgkEXpNANg9GfPw8r17atHHwEXq/b7S4//MVx6W5QEtkAIadyFc?=
 =?us-ascii?Q?gVkw7cL3CuFgj84DmTnlKQpK3j2+NBTE2pmCs5Ou09Z77WEhzyDR1sbPPTQo?=
 =?us-ascii?Q?uQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8e386596-4d6d-4b74-d9db-08db26bf0321
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Mar 2023 08:10:03.8103
 (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: EjMsRovTQWgxkx/iGWSYe1YWr16RCgQlvlfOy0HYfxAEdm/tnHlNNbRUEt6JL2KL+fB17TTDmLu0KJtZgRi7Eg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6663
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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

On Fri, Mar 10, 2023 at 04:22:48PM -0800, Sean Christopherson wrote:
> From: Yan Zhao <yan.y.zhao@intel.com>
> 
> Switch from the poorly named and flawed ->track_flush_slot() to the newly
> introduced ->track_remove_region().  From KVMGT's perspective, the two
> hooks are functionally equivalent, the only difference being that
> ->track_remove_region() is called only when KVM is 100% certain the
> memory region will be removed, i.e. is invoked slightly later in KVM's
> memslot modification flow.
> 
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> [sean: handle name change, massage changelog, rebase]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  drivers/gpu/drm/i915/gvt/kvmgt.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 577712ea4893..9f188b6c3edf 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -108,9 +108,8 @@ struct gvt_dma {
>  
>  static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
>  				   struct kvm_page_track_notifier_node *node);
> -static void kvmgt_page_track_flush_slot(struct kvm *kvm,
> -		struct kvm_memory_slot *slot,
> -		struct kvm_page_track_notifier_node *node);
> +static void kvmgt_page_track_remove_region(gfn_t gfn, unsigned long nr_pages,
> +					   struct kvm_page_track_notifier_node *node);
>  
>  static ssize_t intel_vgpu_show_description(struct mdev_type *mtype, char *buf)
>  {
> @@ -680,7 +679,7 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev)
>  		return -EEXIST;
>  
>  	vgpu->track_node.track_write = kvmgt_page_track_write;
> -	vgpu->track_node.track_flush_slot = kvmgt_page_track_flush_slot;
> +	vgpu->track_node.track_remove_region = kvmgt_page_track_remove_region;
>  	kvm_get_kvm(vgpu->vfio_device.kvm);
>  	kvm_page_track_register_notifier(vgpu->vfio_device.kvm,
>  					 &vgpu->track_node);
> @@ -1631,22 +1630,20 @@ static void kvmgt_page_track_write(gpa_t gpa, const u8 *val, int len,
>  	mutex_unlock(&info->vgpu_lock);
>  }
>  
> -static void kvmgt_page_track_flush_slot(struct kvm *kvm,
> -		struct kvm_memory_slot *slot,
> -		struct kvm_page_track_notifier_node *node)
> +static void kvmgt_page_track_remove_region(gfn_t gfn, unsigned long nr_pages,
> +					   struct kvm_page_track_notifier_node *node)
>  {
>  	unsigned long i;
> -	gfn_t gfn;
>  	struct intel_vgpu *info =
>  		container_of(node, struct intel_vgpu, track_node);
>  
>  	mutex_lock(&info->vgpu_lock);
>  
> -	for (i = 0; i < slot->npages; i++) {
> -		gfn = slot->base_gfn + i;
> -		if (kvmgt_gfn_is_write_protected(info, gfn))
> -			kvmgt_protect_table_del(info, gfn);
> +	for (i = 0; i < nr_pages; i++) {
> +		if (kvmgt_gfn_is_write_protected(info, gfn + i))
> +			kvmgt_protect_table_del(info, gfn + i);
>  	}
> +
>  	mutex_unlock(&info->vgpu_lock);
>  }
>  
> -- 
> 2.40.0.rc1.284.g88254d51c5-goog
>