linux-kernel - Re: [PATCH 06/12] KVM: MMU: rename kvm_mmu

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: <4e05cfc5-55bb-1273-5309-46ed4fe52fed@redhat.com>
Date:   Fri, 11 Feb 2022 11:07:53 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        vkuznets@...hat.com, mlevitsk@...hat.com, dmatlack@...gle.com
Subject: Re: [PATCH 06/12] KVM: MMU: rename kvm_mmu_reload

On 2/11/22 01:27, Sean Christopherson wrote:
> On Wed, Feb 09, 2022, Paolo Bonzini wrote:
>> The name of kvm_mmu_reload is very confusing for two reasons:
>> first, KVM_REQ_MMU_RELOAD actually does not call it; second,
>> it only does anything if there is no valid root.
>>
>> Rename it to kvm_mmu_ensure_valid_root, which matches the actual
>> behavior better.
> 
> 100% agree that kvm_mmu_reload() is a terrible name, but kvm_mmu_ensure_valid_root()
> isn't much better, e.g. it sounds like a sanity check and nothing more.

I would have thought that would be more of a check_valid_root().  There 
are other functions in the kernel following the idea that "ensure" means 
idempotency: skb_ensure_writable, perf_cgroup_ensure_storage, 
btf_ensure_modifiable and libbpf_ensure_mem in libbpf.  I'm not a native 
speaker but, at least in computing, "ensure" seems to mean not just "to 
make certain that (something) will be true", but also taking steps if 
that's not already the case.

I also thought of "establish_valid_root", but it has the opposite 
problem---it does not convey well, if at all, that the root could be 
valid already.

Paolo

> 
> Maybe just be very literalal?
> 
>    kvm_mmu_load_if_necessary()
>    kvm_mmu_load_if_invalid()
> 
> Or follow cond_sched()?
> 
>    kvm_mmu_cond_load()
>