Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 23 Jun 2021 14:50:36 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Yu Zhang <yu.c.zhang@linux.intel.com>,
        Maxim Levitsky <mlevitsk@redhat.com>
Subject: Re: [PATCH 03/54] KVM: x86: Properly reset MMU context at vCPU
 RESET/INIT
Message-ID: <YNNKPADJamqhmDpV@google.com>
References: <20210622175739.3610207-1-seanjc@google.com>
 <20210622175739.3610207-4-seanjc@google.com>
 <3c3f9251-cab4-70ee-6e38-85fe8e6579f7@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3c3f9251-cab4-70ee-6e38-85fe8e6579f7@redhat.com>
Precedence: bulk

On Wed, Jun 23, 2021, Paolo Bonzini wrote:
> On 22/06/21 19:56, Sean Christopherson wrote:
> > +	/*
> > +	 * Reset the MMU context if paging was enabled prior to INIT (which is
> > +	 * implied if CR0.PG=1 as CR0 will be '0' prior to RESET).  Unlike the
> > +	 * standard CR0/CR4/EFER modification paths, only CR0.PG needs to be
> > +	 * checked because it is unconditionally cleared on INIT and all other
> > +	 * paging related bits are ignored if paging is disabled, i.e. CR0.WP,
> > +	 * CR4, and EFER changes are all irrelevant if CR0.PG was '0'.
> > +	 */
> > +	if (old_cr0 & X86_CR0_PG)
> > +		kvm_mmu_reset_context(vcpu);
> 
> Hmm, I'll answer myself, is it because of the plan to add a vCPU reset
> ioctl?

Heh, no, I'm not thinking that far ahead at the moment.

Using "if (init_event)" also resets the MMU when paging was disabled prior to
INIT, which is unnecessary.  "if (init_event && (old_cr0 & X86_CR0_PG))" would
obviously work, but I guess I was feeling clever.

As for why I don't want to unnecessarily reset the MMU, my preference for the MMU
role/context logic is to be as precise as possible to help document "why".  Doing
a MMU reset on any INIT obviously won't break anything, but it doesn't highlight
that the true motivation is CR0.PG being cleared, not simply that INIT occurred.
I.e. the MMU context is a KVM construct, there is no architectural model that
we're trying to follow.