From: Sean Christopherson <seanjc@google.com>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: linux-kernel@vger.kernel.org,
Lai Jiangshan <jiangshan.ljs@antgroup.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
kvm@vger.kernel.org
Subject: Re: [PATCH 1/2] KVM: x86/mmu: Don't check old SPTE permissions when trying to unsync
Date: Thu, 12 Mar 2026 10:35:46 -0700 [thread overview]
Message-ID: <abL5ckz92pGbZ5uc@google.com> (raw)
In-Reply-To: <20260123090304.32286-1-jiangshanlai@gmail.com>
On Fri, Jan 23, 2026, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
>
> Commit ecc5589f19a5 ("KVM: MMU: optimize set_spte for page sync") added
> a writable permission check on the old SPTE to avoid unnecessary calls
> to mmu_try_to_unsync_pages() when syncing SPTEs.
>
> Later, commit e6722d9211b2 ("KVM: x86/mmu: Reduce the update to the spte
> in FNAME(sync_spte)") indirectly achieves it by avoiding some SPTE
> updates altogether, which makes the writable permission check in
> make_spte() much less useful.
>
> Remove the old-SPTE writable permission check from make_spte() to
> simplify the code.
>
> This may cause mmu_try_to_unsync_pages() to be called in a few
> additional cases not covered by commit e6722d9211b2, such as when the
> guest toggles the execute bit, which is expected to be rare.
Hmm, but it would also apply to spurious faults. The TDP MMU largely guards
against that behavior thanks to commit 386d69f9f29b ("KVM: x86/mmu: Treat TDP MMU
faults as spurious if access is already allowed"), but the shadow MMU does not.
Booting a 24 vCPU VM with shadowing paging gets ~3000 hits on the optimizations,
which isn't a ton, but it's definitely not 0 either. And while I'm generally all
about simplifying code, I'm also generally very hesitant to tweak shadow paging
optimizations without strong evidence for doing so.
Is there an ulterior motive to this change, e.g. does it allow for additional
cleanups, or is simplifying the code the main goal?
prev parent reply other threads:[~2026-03-12 17:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-23 9:03 [PATCH 1/2] KVM: x86/mmu: Don't check old SPTE permissions when trying to unsync Lai Jiangshan
2026-01-23 9:03 ` [PATCH 2/2] KVM: x86/mmu: KVM: x86/mmu: Skip unsync when large pages are allowed Lai Jiangshan
2026-03-12 17:07 ` Sean Christopherson
2026-03-12 17:22 ` Sean Christopherson
2026-03-12 17:35 ` Sean Christopherson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=abL5ckz92pGbZ5uc@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jiangshan.ljs@antgroup.com \
--cc=jiangshanlai@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=tglx@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox