linux-kernel - Re: [PATCH 11/11] KVM/MMU: Flush tlb in the kvm_age

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]

Date:   Tue, 8 Jan 2019 11:42:05 +0800
From:   Tianyu Lan <lantianyu1986@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        Lan Tianyu <Tianyu.Lan@...rosoft.com>,
        Radim Krcmar <rkrcmar@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, bp@...en8.de,
        "H. Peter Anvin" <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        kvm <kvm@...r.kernel.org>,
        "linux-kernel@...r kernel org" <linux-kernel@...r.kernel.org>,
        michael.h.kelley@...rosoft.com, kys@...rosoft.com,
        vkuznets@...hat.com, linux@...linux.org.uk
Subject: Re: [PATCH 11/11] KVM/MMU: Flush tlb in the kvm_age_rmapp()

Hi Paolo:
               Thanks for your review.

On Tue, Jan 8, 2019 at 12:31 AM Paolo Bonzini <pbonzini@...hat.com> wrote:
>
> On 07/01/19 04:42, Tianyu Lan wrote:
> >> I'm assuming you're
> >> clearing young to avoid the flush in kvm_mmu_notifier_clear_flush_young(),
> >> but keeping that flush is silly since it will never be invoked.  Just
> >> squash this patch with patch 10/11 so that you can remove the unnecessary
> >> flush in kvm_mmu_notifier_clear_flush_young() and preserve young.
> >>
> > The platform may provide tlb flush with address range as granularity. My changes
> > are to use range flush when it's available. kvm_mmu_notifier_clear_flush_young()
> > is common function for all platforms and most platforms still need the
> > flush in the
> > kvm_mmu_notifier_clear_flush_young(). I think it's better to separate
> > flush request and
> > "young" from return value of kvm_age_hva(). New flush parameter I
> > added in the patch 10
> > can be changed to a pointer and kvm_age_hva() can use it to return
> > flush request.
>
> There are two possibilities:
>
> - pass a "bool *flush".  If NULL, kvm_age_hva should not flush.  If not
> NULL, kvm_age_hva should receive a true *flush, and should change it to
> false if kvm_age_hva takes care of the flush
>
> - pass a "bool flush".  In patch 10, change all kvm_age_hva
> implementation to do the flush if they return 1.
>
> I think I prefer the latter, in this case the small code duplication is
> offset by a simpler API.
>

>From my understanding, this means to move the flush in the
kvm_mmu_notifier_clear_flush_young()
to kvm_age_hva() and do flush in kvm_age_hva() when young is >0 and "flush"
parameter is true, right?
-- 
Best regards
Tianyu Lan