Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 23 Mar 2023 16:50:24 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Sean Christopherson <seanjc@google.com>, <kvm@vger.kernel.org>,
        <intel-gfx@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
        "Zhenyu Wang" <zhenyuw@linux.intel.com>,
        Ben Gardon <bgardon@google.com>,
        "Paolo Bonzini" <pbonzini@redhat.com>,
        <intel-gvt-dev@lists.freedesktop.org>,
        "Zhi Wang" <zhi.a.wang@intel.com>
Subject: Re: [PATCH v2 25/27] KVM: x86/mmu: Drop @slot param from
 exported/external page-track APIs
Message-ID: <ZBwS0DNOwMf7OVmV@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230311002258.852397-1-seanjc@google.com>
 <20230311002258.852397-26-seanjc@google.com>
 <ZBQkyB3KJP34D9/h@yzhao56-desk.sh.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZBQkyB3KJP34D9/h@yzhao56-desk.sh.intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?s8RoL6UXcxtKTGPeWIZ/Y1I/cYlYEkKv2C3DkVFPKaS/kuMWQrBN5HfB2BUO?=
 =?us-ascii?Q?g0L4y3/UTx2rbEW3TQayZeSy9V10Cx1alei47BAefJNCuw/qo56qcuDUGG83?=
 =?us-ascii?Q?s7hys9Ad0bZPUsKL9XFukRi0Sj4x+caSkplPH0B03Md098f9B8MLWTk8iKs0?=
 =?us-ascii?Q?pL6pXvQQnwvIFqJF6M4Yz2+cnI//mnu4ZS4k9xL7KLl2fs5UFIcbkw7MZdTR?=
 =?us-ascii?Q?q3Z+QGRvNNJq1vJUdqMbKLRjnl5Gv1jbv5ZbYF1QPme+3jc4B1sMopqC/VvC?=
 =?us-ascii?Q?Ewxn7Bk3BH7LlQ0ezFmnEFSulkmqikRCpohKjcEoQaJOiDn+4P1GmHDcr8/T?=
 =?us-ascii?Q?VcPfazRoH8MUs35UeZcnBWw3fiQn6bI/GUvO/OorN3bH3cowiBNGQmAMelSw?=
 =?us-ascii?Q?7TeqthkIx1S01aNvLrppdj6Xx5wSBfm7H3jy0VkgIOMnKkXXSpFYTKkK8wVj?=
 =?us-ascii?Q?2ToGHtvOlwdb9+ZuFR9Lqh01WSaA8pOSl9lNpwH2Cb2anWeY2wYEYpGtDBRY?=
 =?us-ascii?Q?su1gvNoIt9OVG8pCygiXbj0lvl8nOh8EqYEtt9skI96XF96mlnultf+UuQjX?=
 =?us-ascii?Q?IiznzspTgd4GBZZTniDSjQ1VFY84lXQ5B5opU4QdUh5gInuEWg32HNuoQtLh?=
 =?us-ascii?Q?oF4GOMVdsc3KWS3+K3EpkU7O8DxdYcAl9O5bkdTFdDqP1SJdOtyTyX7Ndg7Y?=
 =?us-ascii?Q?ZvxSHkg7TRbcQcs/OnY815cgD5S30RhDQZbDU5D3HWPU6vaQ6xp8VOTi1H1Q?=
 =?us-ascii?Q?uy0u40uynCZGdpCMCvtOxFBpyalR+tWGyECkEKk2PKxtlBBJTZ3m6A0hcvHf?=
 =?us-ascii?Q?AP26sKSlhyuda3dvXJ65oUfii+nZaA1wxEcQ/H3zVzWF9J8ByNYBx9KKNudN?=
 =?us-ascii?Q?KA/iqWrSlhId9K1U8H39yt8dbt6Sunu9zPooqNlwEkKOidXOaUqRVDXZXKml?=
 =?us-ascii?Q?uxAxS3HkV7AvH5rzFgYn2RkhY9CY/FGIDao5bTgRkPTXyCH5Elp9rWLNaxih?=
 =?us-ascii?Q?zkpbhLf+BCC4LIrNytwGWy4ul5RQ3HR/nxxH79zN5Annr+tEUheOiPKc0/Bz?=
 =?us-ascii?Q?ODRrCvQXct5RzZT6RlgLtIYmgLVkzv+1M6B5PP4MrahHxHi8hyrKYBbQ4l34?=
 =?us-ascii?Q?Jolk5a5La2tZTWW1+fiImjfdV6HHjASKGu9XBFuU6213a3zjWd54Aw6eTqUZ?=
 =?us-ascii?Q?FqvJMW/8IMmJado9qKChQHE8tc77rBNNF8/0jqRf1yIwAwNUNp3lKTt7RGec?=
 =?us-ascii?Q?iXT+d4toBFzyjhv3Or/tDRSei6RG8eukWonBSnma8BRtmS4MOWOCRXz4Eybu?=
 =?us-ascii?Q?qtCWHFlb5Vfj/FMIjfmDpIrZdYmJTUsCeBK0JeilQg+9EBQACMTp38sg4Qh8?=
 =?us-ascii?Q?eYD9FC33CUOU3bnY5WvSP3HR4HKVC0chmlhtjhQ5XA9tn8HcygVtX6BkgbiV?=
 =?us-ascii?Q?xudalaGU2A4B8udqnVnyV7Tx8QDclsWVgXPOMD9M32nCvVCnSd5+YIbfLs2Y?=
 =?us-ascii?Q?WwPiM6ppglb8/5kK2M0AzNxWnrfRbVe0OgxuJGUzSmRNvNMytnJP/cnmkDBC?=
 =?us-ascii?Q?gzmlP+6nZqWw+OYBjBWNxf1pL5rPhrWPyjjF9YSMfv8MmqSCDRLLinpcuIQI?=
 =?us-ascii?Q?KA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3f99932a-e2db-4b67-79b6-08db2b7f1ca8
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Mar 2023 09:15:14.6744
 (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: avj5XMDXIoZvuJhDjhlTV2eY4Esd+RV1XywZ79pdkMYau6HaIJsM7tvnnm77rPQtVaUzKfvQtmNykQyHO31kEQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB7130
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Mar 17, 2023 at 04:28:56PM +0800, Yan Zhao wrote:
> On Fri, Mar 10, 2023 at 04:22:56PM -0800, Sean Christopherson wrote:
> ...
> > +int kvm_write_track_add_gfn(struct kvm *kvm, gfn_t gfn)
> > +{
> > +	struct kvm_memory_slot *slot;
> > +	int idx;
> > +
> > +	idx = srcu_read_lock(&kvm->srcu);
> > +
> > +	slot = gfn_to_memslot(kvm, gfn);
> > +	if (!slot) {
> > +		srcu_read_unlock(&kvm->srcu, idx);
> > +		return -EINVAL;
> > +	}
> > +
> Also fail if slot->flags & KVM_MEMSLOT_INVALID is true?
> There should exist a window for external users to see an invalid slot
> when a slot is about to get deleted/moved.
> (It happens before MOVE is rejected in kvm_arch_prepare_memory_region()).

Or using
        if (!kvm_is_visible_memslot(slot)) {
		srcu_read_unlock(&kvm->srcu, idx);
		return -EINVAL;
	}

> 
> > +	write_lock(&kvm->mmu_lock);
> > +	__kvm_write_track_add_gfn(kvm, slot, gfn);
> > +	write_unlock(&kvm->mmu_lock);
> > +
> > +	srcu_read_unlock(&kvm->srcu, idx);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(kvm_write_track_add_gfn);
> > +
> > +/*
> > + * remove the guest page from the tracking pool which stops the interception
> > + * of corresponding access on that page.
> > + *
> > + * @kvm: the guest instance we are interested in.
> > + * @gfn: the guest page.
> > + */
> > +int kvm_write_track_remove_gfn(struct kvm *kvm, gfn_t gfn)
> > +{
> > +	struct kvm_memory_slot *slot;
> > +	int idx;
> > +
> > +	idx = srcu_read_lock(&kvm->srcu);
> > +
> > +	slot = gfn_to_memslot(kvm, gfn);
> > +	if (!slot) {
> > +		srcu_read_unlock(&kvm->srcu, idx);
> > +		return -EINVAL;
> > +	}
> > +
> Ditto.
> 
> > +	write_lock(&kvm->mmu_lock);
> > +	__kvm_write_track_remove_gfn(kvm, slot, gfn);
> > +	write_unlock(&kvm->mmu_lock);
> > +
> > +	srcu_read_unlock(&kvm->srcu, idx);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(kvm_write_track_remove_gfn);
>