linux-kernel - Re: [RFC PATCH 0/4] KVM: x86/mmu: Rework marking folios dirty/accessed

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] [day] [month] [year] [list]

Message-ID: <9b0f5285-bbce-41a1-9c74-74051ca3f9b1@redhat.com>
Date: Fri, 5 Apr 2024 16:06:33 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>,
 David Hildenbrand <david@...hat.com>
Cc: David Matlack <dmatlack@...gle.com>, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, David Stevens <stevensd@...omium.org>,
 Matthew Wilcox <willy@...radead.org>
Subject: Re: [RFC PATCH 0/4] KVM: x86/mmu: Rework marking folios
 dirty/accessed

On 4/5/24 15:59, Sean Christopherson wrote:
> Ya, from commit c13fda237f08 ("KVM: Assert that notifier count is elevated in
> .change_pte()"):
> 
>      x86 and MIPS are clearcut nops if the old SPTE is not-present, and that
>      is guaranteed due to the prior invalidation.  PPC simply unmaps the SPTE,
>      which again should be a nop due to the invalidation.  arm64 is a bit
>      murky, but it's also likely a nop because kvm_pgtable_stage2_map() is
>      called without a cache pointer, which means it will map an entry if and
>      only if an existing PTE was found.
> 
> I'm 100% in favor of removing .change_pte().  As I've said multiple times, the
> only reason I haven't sent a patch is because I didn't want it to prompt someone
> into resurrecting the original behavior. 🙂

Ah, this indeed reminded me of discussions we had in the past.  Patches 
sent now:

https://lore.kernel.org/kvm/20240405115815.3226315-1-pbonzini@redhat.com/

and I don't think anyone would try to resurrect the original behavior. 
It made some sense when there was an .invalidate_page() callback as 
well, but the whole thing started to crumble when .invalidate_page() was 
made sleepable, plus as David noticed it's very poorly documented where 
to use set_pte_at_notify() vs. set_pte_at().

As I wrote in the cover letter above, the only reason why it has never 
caused a bug is because it was doing nothing...

Paolo