[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2e6c4c58-d4d2-69e2-f8ed-c93d9c13365b@redhat.com>
Date: Tue, 8 Mar 2022 19:24:57 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.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 3/8/22 19:11, Sean Christopherson wrote:
> On Mon, Feb 21, 2022, Paolo Bonzini wrote:
>> 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).
In which case bit 2 of EPTs doesn't change meaning, does it?
> 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.
Setting EFER.NX=0 might be okay for EPT/NPT, but I'd prefer to set it
respectively to 1 (X bit always present) and host EFER.NX (NX bit
present depending on host EFER).
For CR0.WP it should really be 1 in my opinion, because CR0.WP=0 implies
having a concept of user vs. supervisor access: CR0.WP=1 is the
"default", while CR0.WP=0 is "always allow *supervisor* writes".
>> 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".
Good point! It's safe and arguably clea{n,r}er, but it's still a pretty
large change.
> E.g. if this lands after the changes to not unload MMUs on cr0/cr4
> emulation, it will be quite the functional change.
I expect this to land first, so that the part where we don't really
agree on the implementation comes last and benefits from a more
understandable core.
Paolo
Powered by blists - more mailing lists