All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Lai Jiangshan <laijs@linux.alibaba.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH V2] KVM: X86: Move PTE present check from loop body to __shadow_walk_next()
Date: Thu, 2 Sep 2021 20:43:25 +0000	[thread overview]
Message-ID: <YTE3bRcZv2BiVxzH@google.com> (raw)
In-Reply-To: <20210813031629.78670-1-jiangshanlai@gmail.com>

On Fri, Aug 13, 2021, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@linux.alibaba.com>
> 
> So far, the loop bodies already ensure the PTE is present before calling
> __shadow_walk_next():  Some loop bodies simply exit with a !PRESENT
> directly and some other loop bodies, i.e. FNAME(fetch) and __direct_map()
> do not currently terminate their walks with a !PRESENT, but they get away
> with it because they install present non-leaf SPTEs in the loop itself.
> 
> But checking pte present in __shadow_walk_next() is a more prudent way of
> programing and loop bodies will not need to always check it. It allows us
> removing unneded is_shadow_present_pte() in the loop bodies.
           ^^^^^^^
	   unneeded

> 
> Terminating on !is_shadow_present_pte() is 100% the correct behavior, as
> walking past a !PRESENT SPTE would lead to attempting to read a the next
> level SPTE from a garbage iter->shadow_addr.  Even some paths that do _not_
> currently have a !is_shadow_present_pte() in the loop body is Ok since
> they will install present non-leaf SPTEs and the additinal present check
                                                   ^^^^^^^^^
						   additional
> is just an NOP.
> 
> The checking result in __shadow_walk_next() will be propagated to
> shadow_walk_okay() for being used in any for(;;) loop.
> 
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---

Nits aside,

Reviewed-by: Sean Christopherson <seanjc@google.com>

  parent reply	other threads:[~2021-09-02 20:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12  4:36 [PATCH 1/2] KVM: X86: Check pte present first in __shadow_walk_next() Lai Jiangshan
2021-08-12  4:36 ` [PATCH 2/2] KVM: X86: Remove the present check from for_each_shadow_entry* loop body Lai Jiangshan
2021-08-12 16:03 ` [PATCH 1/2] KVM: X86: Check pte present first in __shadow_walk_next() Sean Christopherson
2021-08-13  3:16   ` [PATCH V2] KVM: X86: Move PTE present check from loop body to __shadow_walk_next() Lai Jiangshan
2021-08-24  8:30     ` Lai Jiangshan
2021-09-02 20:43     ` Sean Christopherson [this message]
2021-09-06 12:25       ` [PATCH V3 1/2] KVM: x86/mmu: Verify shadow walk doesn't terminate early in page faults Lai Jiangshan
2021-09-06 12:25         ` [PATCH V3 2/2] KVM: X86: Move PTE present check from loop body to __shadow_walk_next() Lai Jiangshan
2021-09-24  9:56           ` Paolo Bonzini
2021-09-24  9:33         ` [PATCH V3 1/2] KVM: x86/mmu: Verify shadow walk doesn't terminate early in page faults Paolo Bonzini
2021-08-14  9:47   ` [PATCH 1/2] KVM: X86: Check pte present first in __shadow_walk_next() Lai Jiangshan

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=YTE3bRcZv2BiVxzH@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jiangshanlai@gmail.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.