linux-kernel - Re: [PATCH v2 12/25] KVM: x86/mmu: cleanup computation of MMU roles for two-dimensional paging

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: <YiecYxd/YreGFWpB@google.com>
Date:   Tue, 8 Mar 2022 18:11:47 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        dmatlack@...gle.com
Subject: Re: [PATCH v2 12/25] KVM: x86/mmu: cleanup computation of MMU roles
 for two-dimensional paging

On Mon, Feb 21, 2022, Paolo Bonzini wrote:
> Inline kvm_calc_mmu_role_common into its sole caller, and simplify it
> by removing the computation of unnecessary bits.
> 
> Extended bits are unnecessary because page walking uses the CPU mode,
> and EFER.NX/CR0.WP can be set to one unconditionally---matching the
> format of shadow pages rather than the format of guest pages.

But they don't match the format of shadow pages.  EPT has an equivalent to NX in
that KVM can always clear X, but KVM explicitly supports running with EPT and
EFER.NX=0 in the host (32-bit non-PAE kernels).

CR0.WP equally confusing.  Yes, both EPT and NPT enforce write protection at all
times, but EPT has no concept of user vs. supervisor in the EPT tables themselves,
at least with respect to writes (thanks mode-based execution for the qualifier...).
NPT is even worse as the APM explicitly states:

  The host hCR0.WP bit is ignored under nested paging.

Unless there's some hidden dependency I'm missing, I'd prefer we arbitrarily leave
them zero.

> The MMU role for two dimensional paging does still depend on the CPU mode,

Heh, don't think it's necessary to spell out TDP, and I think it would be helpful
to write it as "non-nested TDP" since the surrounding patches deal with both.

> even if only barely so, due to SMM and guest mode; for consistency,
> pass it down to kvm_calc_tdp_mmu_root_page_role instead of querying
> the vcpu with is_smm or is_guest_mode.

The changelog should call out this is a _significant_ change in behavior for KVM,
as it allows reusing shadow pages with different guest MMU "role bits".  E.g. if
this lands after the changes to not unload MMUs on cr0/cr4 emulation, it will be
quite the functional change.