linux-kernel - Re: [PATCH 1/2] KVM: x86/mmu: Check yielded_gfn for forward progress iff resched is needed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADrL8HUwkxGmud5hVYPC6ibsA=ee8SePbzz23Vrqrex+AOtmTQ@mail.gmail.com>
Date: Thu, 31 Oct 2024 11:48:18 -0700
From: James Houghton <jthoughton@...gle.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	David Matlack <dmatlack@...gle.com>
Subject: Re: [PATCH 1/2] KVM: x86/mmu: Check yielded_gfn for forward progress
 iff resched is needed

On Thu, Oct 31, 2024 at 10:07 AM Sean Christopherson <seanjc@...gle.com> wrote:
>
> Swap the order of the checks in tdp_mmu_iter_cond_resched() so that KVM
> checks to see if a resched is needed _before_ checking to see if yielding
> must be disallowed to guarantee forward progress.  Iterating over TDP MMU
> SPTEs is a hot path, e.g. tearing down a root can touch millions of SPTEs,
> and not needing to reschedule is by far the common case.  On the other
> hand, disallowing yielding because forward progress has not been made is a
> very rare case.
>
> Returning early for the common case (no resched), effectively reduces the
> number of checks from 2 to 1 for the common case, and should make the code
> slightly more predictable for the CPU.
>
> To resolve a weird conundrum where the forward progress check currently
> returns false, but the need resched check subtly returns iter->yielded,
> which _should_ be false (enforced by a WARN), return false unconditionally
> (which might also help make the sequence more predictable).  If KVM has a
> bug where iter->yielded is left danging, continuing to yield is neither
> right nor wrong, it was simply an artifact of how the original code was
> written.
>
> Unconditionally returning false when yielding is unnecessary or unwanted
> will also allow extracting the "should resched" logic to a separate helper
> in a future patch.
>
> Cc: David Matlack <dmatlack@...gle.com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Feel free to add:

Reviewed-by: James Houghton <jthoughton@...gle.com>