Date:   Fri, 17 Mar 2023 15:43:17 +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 16/27] KVM: x86: Add a new page-track hook to handle
 memslot deletion
Message-ID: <ZBQaFRdRawenuEan@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230311002258.852397-1-seanjc@google.com>
 <20230311002258.852397-17-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230311002258.852397-17-seanjc@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?8xi3HLCgQB55T8BMYhFOSU6Pl4wxqWLuGbCvLTa5ZAmKi+w2GEtjA9eVNfkM?=
 =?us-ascii?Q?qtukVX20kv/SMb5HNlAp7R1h6jMGerybg29cUa0arSuNJfz0yimdQRPrtpnf?=
 =?us-ascii?Q?IipRmNUKcHk8+RLqAHkwCsq2sZ0y3mjdhIeiD5bt18f7rlY0TRUc5TXg5dkg?=
 =?us-ascii?Q?DOTKQ7noG3FBUQ3ajG/mH9GycnZOnBCJ4PoBAcbSo/hhzcQUpgPJqi6zY55H?=
 =?us-ascii?Q?7ZZUTH/GdlohHkXBrkD6kPR4etrGCA0HLrk1qjuidXtWxKaUtiwpkWfuAApr?=
 =?us-ascii?Q?zcltooTbZxIeyClWxOjRPd68kcaPEMLrPC/dT1Y33JHgHeilsUXsTtJOhfRk?=
 =?us-ascii?Q?WfBrl/Ka/1iIbjI3e5ilwgIci+9T1hi4SsFXJXdQBEUguuo+jD3LCTo3d6N9?=
 =?us-ascii?Q?6BESWK+H02RXKRIvatDUXsdh7bF1ETubYNlQ53p3S9LH27AvyRV0gDTuN1Cy?=
 =?us-ascii?Q?Gg+zHK+H0ckYz646EsPa3Aass1fQsVDW6guMXVG33pKRlyqfg2xDLRNyL7+9?=
 =?us-ascii?Q?HrNKukt/DOvSLHvGFlnRZuG1lvMYUpFvq1IYwmkKuj1xUazAbau5ekCtqcOL?=
 =?us-ascii?Q?Gqn+fy/hRVxbcEx/7XDtaBazdCsuRHePXCelqlXCTzzmicE+XdO5RIlwtQpg?=
 =?us-ascii?Q?q5svgZsGVAKuTU7i5bwvQvCQwv03pwmT2sDOXZuyIzNOvzSgDbPedvVURJ/B?=
 =?us-ascii?Q?KkuZCnsq4WcqlNimCilMHuqprZnCtBDpljo2NSiHazBrPXxPH41rrDELXiYC?=
 =?us-ascii?Q?UrZe7OMhesT9dbx3LEYMbISPxf9+RhfqWMlx1ymepUZleqvbl+Nq760RGWeM?=
 =?us-ascii?Q?9CTbJLJYzemqSqSuzpsbJZii/6ykCKuLk5fqRFrsCs/K/Lxq2TP7AEvjwJ8p?=
 =?us-ascii?Q?I1ut6RK6C5o7S6NaHjOPY6Xovks9dyqSmSJQ9XBJRut8zDQ1e9wz4B+1JSB0?=
 =?us-ascii?Q?WL0DrHQfSS343P82R/OXb9ryArtzV3AnFGECUHZaBaJ3Ryyza1rwYr3Er0CQ?=
 =?us-ascii?Q?4ToVBhObgYOQlEIvnQSJceVmCG1MJWUJhnA1/Xs/mY9cI6R3I9J9TqNKa8gD?=
 =?us-ascii?Q?DY+BWvHpmUCYuC1z/se8jsq1ZynQL9Db0+CM1MlIZGQH7BsX5w5ehKP1eUr/?=
 =?us-ascii?Q?aolZzmuWOWuiinbsIN4vysXblDol2aRV4eoaS+802Q2qDI4iXIBbOcQfHfyr?=
 =?us-ascii?Q?TYrJ29XCIHUKvojbJSqI0W2nSOUXPrEJ1JI/7+FTVmSU5Pmh9XYDiRlOh5ud?=
 =?us-ascii?Q?H+MFws/vpy8jmdH4/7c0+xvkROZoBov2q2JVOz0mYbbfuBvxlYiVEf6RqoMi?=
 =?us-ascii?Q?V5bVznpAAAds9laJB3Xa6IrNaCrXHS4rgKsZdngUrHy9nvfnik/b29JWeaRJ?=
 =?us-ascii?Q?cXtoHcAgWffrZKaL4nC+72GerGRFPwl1Ysv4LjaCGnQ0m576cfUuDF1WqqFI?=
 =?us-ascii?Q?zSlki2tEXSnWho0sEkai60DX46/AT8Ozny/SmEnTQZxx+MKycYaAA2aba7Lq?=
 =?us-ascii?Q?NcFhFRMyqXtGZ/GvP4bKy9msBfBrYb26Hadj1Fmz3CKkkjiUTwwYZB3g66ZR?=
 =?us-ascii?Q?W+HuxiQO4LpLff0Fp8ZjpN1udGDbVJkG7Je0xoMkXYhNaPvZaWW8aPPH95wY?=
 =?us-ascii?Q?eg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a1487150-ef21-4933-5b5c-08db26beb68b
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:07:55.3641
 (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: wQ1KvjKijWt/0p7GMPZdROz6CHwsguyCXj/2epUHSlBLBMByi5eDSYe3nU6V13urq8WkqIfqmp6V4u1flLiiPQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB5827
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Mar 10, 2023 at 04:22:47PM -0800, Sean Christopherson wrote:
> From: Yan Zhao <yan.y.zhao@intel.com>
> 
> Add a new page-track hook, track_remove_region(), that is called when a
> memslot DELETE operation is about to be committed.  The "remove" hook
> will be used by KVMGT and will effectively replace the existing
> track_flush_slot() altogether now that KVM itself doesn't rely on the
> "flush" hook either.
> 
> The "flush" hook is flawed as it's invoked before the memslot operation
> is guaranteed to succeed, i.e. KVM might ultimately keep the existing
> memslot without notifying external page track users, a.k.a. KVMGT.  In
> practice, this can't currently happen on x86, but there are no guarantees
> that won't change in the future, not to mention that "flush" does a very
> poor job of describing what is happening.
> 
> Pass in the gfn+nr_pages instead of the slot itself so external users,
> i.e. KVMGT, don't need to exposed to KVM internals (memslots).  This will
> help set the stage for additional cleanups to the page-track APIs.
> 
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
...

> +void kvm_page_track_delete_slot(struct kvm *kvm, struct kvm_memory_slot *slot)
> +{
> +	struct kvm_page_track_notifier_head *head;
> +	struct kvm_page_track_notifier_node *n;
> +	int idx;
> +
> +	head = &kvm->arch.track_notifier_head;
> +
> +	if (hlist_empty(&head->track_notifier_list))
> +		return;
> +
> +	idx = srcu_read_lock(&head->track_srcu);
> +	hlist_for_each_entry_srcu(n, &head->track_notifier_list, node,
> +				srcu_read_lock_held(&head->track_srcu))
Sorry, not sure why the alignment here is not right.
Patchwork just sent me a mail to complain about it.
Would you mind helping fix it in the next version?

Thanks a lot!

> +		if (n->track_remove_region)
> +			n->track_remove_region(slot->base_gfn, slot->npages, n);
> +	srcu_read_unlock(&head->track_srcu, idx);
> +}
> +