From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4872DC4167B for ; Wed, 14 Dec 2022 19:09:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238450AbiLNTJN (ORCPT ); Wed, 14 Dec 2022 14:09:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37460 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237321AbiLNTJK (ORCPT ); Wed, 14 Dec 2022 14:09:10 -0500 Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 41AFEE80 for ; Wed, 14 Dec 2022 11:09:09 -0800 (PST) Received: by mail-pj1-x102f.google.com with SMTP id 3-20020a17090a098300b00219041dcbe9so189693pjo.3 for ; Wed, 14 Dec 2022 11:09:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=FIlNxfH2N14uQxL9gQjMu+4jFBzMKkxQj54k3fDEAoM=; b=ki7KXMuMgFSwbFlvvSqSuOQhI4nnWJpLKfKstanOkB/onj5nUHoVz525QhfZ8nokKB VXeBAUiQXL7JxbcSrDvQSoa4lSwXeMjbkvKivBYSZ0TEpYbXer0eJBuO5rSLLr9It5WK fvs+dp4wbuKBP0yy+S1uOP84czNeK8nhe/1Uifui5WzzQ+tKUtXjnaV2x0inbQFe8dNk qOUQLOFp3Ks0/MP/nhesJP/lViUPuMv+FypbFBL4U08YgIe7ET0AWrPZN0TUa7vtcmVX 4UPcD3345eJMKynVDVcjhWoPJZZ1z1i5AX6S+YpLVZ/1/YeT1xAjXv5yV02NAAay6VtC Kq4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=FIlNxfH2N14uQxL9gQjMu+4jFBzMKkxQj54k3fDEAoM=; b=LXbfZW1Pg0lCpetZQY53Xwbatrz1UBDLK0nZ+2+b7rhib/MD8MWbnYSiHvP1FjG6uz IE+sNFI3bZoCCPBgXHPMcjOfc8I+adZZmvKEBoqw0p8UyToW/gvqabkd5ogE1RxgIAvP bcmC6fQq46gr6jdRmveGMmA+YUN6spWjnKhvH1c8E/C5yVCdVKE9OFobzZxvd3Ps34rC 80VAuPr6xyHxkjl3gxzH+xFIFzZ7nIYpT5pPFAfAVJTCieuiXQB7N+THRGVlncs3wcyL 5Nt/oNLOn6apBp0CunWHaUp/Zw1JLfTdAUsB/25QCeFZCGyz7hXHoVT9y0EBLwBD6p45 zCwg== X-Gm-Message-State: ANoB5pla/Ks5HmPP2CBtqq5ck8AV0knr4zghUYrc5YjE63RoLyoYRmw9 KDZoigzf4KsL8UFI5klw45T6AA== X-Google-Smtp-Source: AA0mqf7VDwtO7bD7iDYUg1CUV8sCconizPsDQAtMy4jK4dIm4X3K9Vibe1wrX4pKJoLBdYIM/pjdjg== X-Received: by 2002:a05:6a21:2d8f:b0:a7:882e:3a18 with SMTP id ty15-20020a056a212d8f00b000a7882e3a18mr671791pzb.1.1671044948611; Wed, 14 Dec 2022 11:09:08 -0800 (PST) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id e28-20020aa7981c000000b00575b6f8b84esm234502pfl.26.2022.12.14.11.09.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Dec 2022 11:09:06 -0800 (PST) Date: Wed, 14 Dec 2022 19:09:02 +0000 From: Sean Christopherson To: Lai Jiangshan Cc: linux-kernel@vger.kernel.org, Paolo Bonzini , Lai Jiangshan , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , kvm@vger.kernel.org Subject: Re: [PATCH 1/2] kvm: x86/mmu: Reduce the update to the spte in FNAME(sync_page) Message-ID: References: <20221212153205.3360-1-jiangshanlai@gmail.com> <20221212153205.3360-2-jiangshanlai@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 14, 2022, Lai Jiangshan wrote: > Hello Sean, > > On Wed, Dec 14, 2022 at 2:12 AM Sean Christopherson wrote: > > > > On Mon, Dec 12, 2022, Lai Jiangshan wrote: > > > From: Lai Jiangshan > > > > > > Sometimes when the guest updates its pagetable, it adds only new gptes > > > to it without changing any existed one, so there is no point to update > > > the sptes for these existed gptes. > > > > > > Also when the sptes for these unchanged gptes are updated, the AD > > > bits are also removed since make_spte() is called with prefetch=true > > > which might result unneeded TLB flushing. > > > > If either of the proposed changes is kept, please move this to a separate patch. > > Skipping updates for PTEs with the same protections is separate logical change > > from skipping updates when making the SPTE writable. > > > > Actually, can't we just pass @prefetch=false to make_spte()? FNAME(prefetch_invalid_gpte) > > has already verified the Accessed bit is set in the GPTE, so at least for guest > > correctness there's no need to access-track the SPTE. Host page aging is already > > fuzzy so I don't think there are problems there. > > FNAME(prefetch_invalid_gpte) has already verified the Accessed bit is set > in the GPTE and FNAME(protect_clean_gpte) has already verified the Dirty > bit is set in the GPTE. These are only for guest AD bits. > > And I don't think it is a good idea to pass @prefetch=false to make_spte(), > since the host might have cleared AD bit in the spte for aging or dirty-log, > The AD bits in the spte are better to be kept as before. Drat, I was thinking KVM never flushes when aging SPTEs, but forgot about clear_flush_young(). Rather than skipping if the Accessed bit is the only thing that's changing, what about simply preserving the Accessed bit? And s/prefetch/accessed in make_spte() so that future changes to make_spte() don't make incorrect assumptions about the meaning of "prefetch". Another alternative would be to conditionally preserve the Accessed bit, i.e. clear it if a flush is needed anyways, but that seems unnecessarily complex. > Though passing @prefetch=false would not cause any correctness problem > in the view of maintaining guest AD bits. > > > > > > Do nothing if the permissions are unchanged or only write-access is > > > being added. > > > > I'm pretty sure skipping the "make writable" case is architecturally wrong. On a > > #PF, any TLB entries for the faulting virtual address are required to be removed. > > That means KVM _must_ refresh the SPTE if a vCPU takes a !WRITABLE fault on an > > unsync page. E.g. see kvm_inject_emulated_page_fault(). > > I might misunderstand what you meant or I failed to connect it with > the SDM properly. > > I think there is no #PF here. > > And even if the guest is requesting writable, the hypervisor is allowed to > set it non-writable and prepared to handle it in the ensuing write-fault. Yeah, you're right. The host will see the "spurious" page fault but it will never get injected into the guest. > Skipping to make it writable is a kind of lazy operation and considered > to be "the hypervisor doesn't grant the writable permission for a period > before next write-fault". But that raises the question of why? No TLB flush is needed precisely because any !WRITABLE fault will be treated as a spurious fault. The cost of writing the SPTE is minimal. So why skip? Skipping just to reclaim a low SPTE bit doesn't seem like a good tradeoff, especially without a concrete use case for the SPTE bit. E.g. on pre-Nehalem Intel CPUs, i.e. CPUs that don't support EPT and thus have to use shadow paging, the CPU automatically retries accesses after the TLB flush on permission faults. The lazy approach might introduce a noticeable performance regression on such CPUs due to causing more #PF VM-Exits than the current approach.