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

Message-ID: <7a1c58d7-ddd9-40fc-a4ef-81c548de2b07@redhat.com>
Date: Fri, 5 Apr 2024 12:14:00 +0200
From: David Hildenbrand <david@...hat.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
 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 05.04.24 11:37, Paolo Bonzini wrote:
> On Fri, Apr 5, 2024 at 8:53 AM David Hildenbrand <david@...hat.com> wrote:
>>>        mmu_notifier_invalidate_range_start(&range);
>>>        tlb_start_vma(&tlb, vma);
>>>        walk_page_range(vma->vm_mm, range.start, range.end,
>>>                        &madvise_free_walk_ops, &tlb);
>>>        tlb_end_vma(&tlb, vma);
>>>        mmu_notifier_invalidate_range_end(&range);
>>>
>>
>> Indeed, we do setup the MMU notifier invalidation. We do the start/end
>> ... I was looking for PTE notifications.
>>
>> I spotted the set_pte_at(), not a set_pte_at_notify() like we do in
>> other code. Maybe that's not required here (digging through
>> documentation I'm still left clueless). [...]
>> Absolutely unclear to me when we *must* to use it, or if it is. Likely
>> its a pure optimization when we're *changing* a PTE.
> 
> Yes, .change_pte() is just an optimization. The original point of it
> was for KSM, so that KVM could flip the sPTE to a new location without
> first zapping it. At the time there was also an .invalidate_page()
> callback, and both of them were *not* bracketed by calls to
> mmu_notifier_invalidate_range_{start,end}()
> 
> Later on, both callbacks were changed to occur within an
> invalidate_range_start/end() block.
> 
> Commit 6bdb913f0a70 ("mm: wrap calls to set_pte_at_notify with
> invalidate_range_start and invalidate_range_end", 2012-10-09) did so
> for .change_pte(). The reason to do so was a bit roundabout, namely to
> allow sleepable .invalidate_page() hooks (because .change_pte() is not
> sleepable and at the time .invalidate_page() was used as a fallback
> for .change_pte()).
> 
> This however made KVM's usage of the .change_pte() callback completely
> moot, because KVM unmaps the sPTEs during .invalidate_range_start()
> and therefore .change_pte() has no hope of succeeding.
> 
> (Commit 369ea8242c0f ("mm/rmap: update to new mmu_notifier semantic
> v2", 2017-08-31) is where the other set of non-bracketed calls to MMU
> notifier callbacks was changed; calls to
> mmu_notifier_invalidate_page() were replaced by calls to
> mmu_notifier_invalidate_range(), bracketed by calls to
> mmu_notifier_invalidate_range_{start,end}()).
> 
> Since KVM is the only user of .change_pte(), we can remove
> .change_pte() and set_pte_at_notify() completely.

Nice, thanks for all that information!

-- 
Cheers,

David / dhildenb