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:   Fri, 29 Jul 2022 09:02:51 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>,
        Yosry Ahmed <yosryahmed@google.com>,
        "Mingwei Zhang" <mizhang@google.com>,
        Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v2 4/6] KVM: x86/mmu: Track the number of TDP MMU pages,
 but not the actual pages
Message-ID: <YuMxuxgFpQdpmBBN@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20220723012325.1715714-1-seanjc@google.com>
 <20220723012325.1715714-5-seanjc@google.com>
 <YuCl48wyA1XkqMan@yzhao56-desk.sh.intel.com>
 <YuGMQ/JJjuWxaUSu@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YuGMQ/JJjuWxaUSu@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?x5x8s4qBWEH+zcQO+K1Vjwz/hW9ettyKUxBg7qP1Jhi+OjEyGjpkTLZZFa3F?=
 =?us-ascii?Q?u/tUECVWXb2IXd4dT+4qY1pRN18/ejkVrOEjkeIj8ktRwPTohvGpzHvWU/x4?=
 =?us-ascii?Q?0DF12sgWBo0q6i5DwopJaeANQuiuHzWSToJveeroabTwY2Q+4TzMCIVXXSqx?=
 =?us-ascii?Q?C1TDOEWIbNbZLJGY7Cepi/rr8wblNG5fL9ym8ly9luhaQhZhEgkd359NeHQ7?=
 =?us-ascii?Q?dhw61vBTnOhQ4CtLlOwkOURCdG6ORnpV/4zNre2ySCcGqm5kQHCkaNRVfaqQ?=
 =?us-ascii?Q?ZJBSdrV6HJFL7G3IuTuei1AgeAnk8429+oK0/+gaGe+Tx89rZ8jCE8+ml6Ht?=
 =?us-ascii?Q?MWef3azb5iR6epn3lsmHtolC9qYO3fXY40xeFNtGRAo9aLxAWr4MaVQ197lm?=
 =?us-ascii?Q?/DLhyTHN2qoikHMQbmqKCMtsSrORjC5N+Oz8tShyfLUbMc0WVqE+hIpOeFCk?=
 =?us-ascii?Q?rxaxkQP3sIvv2Biy3Nh4NgF46BHQsjV8TpEdn/HKqUP/pXDPMbnJBiCbBWKZ?=
 =?us-ascii?Q?tr8EuFbRQe+I1Wp7xxoZcI6NxlQR0JIEWRGmUps8XP4lRZpT70/ictmcXWq8?=
 =?us-ascii?Q?ZuazsrgrHGLTHHbJkVlyQPcsOaqdvfiGWTZ5+ySuC8YFo2vkNsfH3T9nfs/q?=
 =?us-ascii?Q?rKJTvkoDbSO0HjqtzMTZR7iFpFzOWDTlifI1OcyaeTSev3xQADN1jpuP7xJ8?=
 =?us-ascii?Q?PHNzmj/3TvFd3eJzxQvaiU/KfCgpwr8uBwpPyXOD+nGL/qbrIXR8ogEgKVJQ?=
 =?us-ascii?Q?l0KeewaU5vQCMKAUpkek0yhejCWwa58sJl079FHzysMraluprz2nsQsVe92V?=
 =?us-ascii?Q?GcFqJQI5nn+XaqG0fmoC93pUgzbKMOkOIWZvKWYqGw0uC1IVWBy+GpJ4vYNg?=
 =?us-ascii?Q?kQ0KNFDCITcWXGx1WbQqfm87uBrmD9z7iXfTkOeQzCtGr+apOliEez9ul5Ge?=
 =?us-ascii?Q?jfg+PaK4j2IvoXFTX1TdkvCn7aW2PfL5Fg+8VuM9YzZpenSarqpsa1DSlWsr?=
 =?us-ascii?Q?b1KH02UehTi4XP6yq0TUc3rNtoMgYEDoeYnVimpaqJpKqVZuZCiKKkraI36e?=
 =?us-ascii?Q?kYIJtuIsVZAMfMV1uC7MzL/oE5AmtxvS6063riZ7qKrhqmmtC9UNraWNtyWK?=
 =?us-ascii?Q?S2Pmeb9kz/JK4NqJcauUDJPegbzhiAW/1W2ix5FB/1CyUcOh4yjwZxlMPsEZ?=
 =?us-ascii?Q?ubN5nmKlbvyaiV4ibMRy10nVaQuxZFWhsJXAZ/8pXEvS6YeMj25gtLMvfN28?=
 =?us-ascii?Q?WikQuKzoXKCl+b7YdmH+MH899axsXl2C+0SzkPun6Z/x3WL9D7DI+3X8Hnbf?=
 =?us-ascii?Q?nY3p8SHoS5a8AbNqVdDrDU551Q4OkBzzLIeljCycWlZwk5hx4qTIzXX238Gq?=
 =?us-ascii?Q?LpxyS2motjYXL4shz/tl2upPJpLe3xTXOmAklgLrE3aQSRrNrdY0lfDUwEPo?=
 =?us-ascii?Q?MF3FaGkeUw8Pk8uF/bidTCODj47S4pHN9sVJnYNShRjNJNqwASBNgOJqs980?=
 =?us-ascii?Q?Sj8QQmiaOYzfw9G1AaCFU/lt/qBmQQygbLqFIE2+uSpUwcapWqbZg8sq39lL?=
 =?us-ascii?Q?P/GvBrvRAjGPwWC+j7IgQNmXZFxsMGLeRn77VB4D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6e867979-9387-4b08-24e8-08da7100da80
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2022 01:22:50.8928
 (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: IYn+NAr0F8E3dZfbpOnxkFrN/4mcFGE0Doz//htt9/ETdc4/0+Tpjs9NNulIK+Bzd1Gsnb/vcPqoTOTd6jAGhg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR11MB3521
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE autolearn=ham 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 Wed, Jul 27, 2022 at 07:04:35PM +0000, Sean Christopherson wrote:
> On Wed, Jul 27, 2022, Yan Zhao wrote:
> > On Sat, Jul 23, 2022 at 01:23:23AM +0000, Sean Christopherson wrote:
> > 
> > <snip>
> > 
> > > @@ -386,16 +385,18 @@ static void handle_changed_spte_dirty_log(struct kvm *kvm, int as_id, gfn_t gfn,
> > >  static void tdp_mmu_unlink_sp(struct kvm *kvm, struct kvm_mmu_page *sp,
> > >  			      bool shared)
> > >  {
> > > +	atomic64_dec(&kvm->arch.tdp_mmu_pages);
> > > +
> > > +	if (!sp->nx_huge_page_disallowed)
> > > +		return;
> > > +
> > Does this read of sp->nx_huge_page_disallowed also need to be protected by
> > tdp_mmu_pages_lock in shared path?
> 
> 
> No, because only one CPU can call tdp_mmu_unlink_sp() for a shadow page.  E.g. in
> a shared walk, the SPTE is zapped atomically and only the CPU that "wins" gets to
> unlink the s[.  The extra lock is needed to prevent list corruption, but the
> sp itself is thread safe.
> 
> FWIW, even if that guarantee didn't hold, checking the flag outside of tdp_mmu_pages_lock
> is safe because false positives are ok.  untrack_possible_nx_huge_page() checks that
> the shadow page is actually on the list, i.e. it's a nop if a different task unlinks
> the page first.
> 
> False negatives need to be avoided, but nx_huge_page_disallowed is cleared only
> when untrack_possible_nx_huge_page() is guaranteed to be called, i.e. true false
> negatives can't occur.
> 
> Hmm, but I think there's a missing smp_rmb(), which is needed to ensure
> nx_huge_page_disallowed is read after observing the shadow-present SPTE (that's
> being unlinked).  I'll add that in the next version.

It makes sense. Thanks for such detailed explanation!

Thanks
Yan