Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 24 May 2022 21:00:42 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Lei Wang <lei4.wang@intel.com>
Cc:     pbonzini@redhat.com, vkuznets@redhat.com, wanpengli@tencent.com,
        jmattson@google.com, joro@8bytes.org, chenyi.qiang@intel.com,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 2/8] KVM: VMX: Add proper cache tracking for PKRS
Message-ID: <Yo1Hemue8+l5CPIT@google.com>
References: <20220424101557.134102-1-lei4.wang@intel.com>
 <20220424101557.134102-3-lei4.wang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220424101557.134102-3-lei4.wang@intel.com>
Precedence: bulk

On Sun, Apr 24, 2022, Lei Wang wrote:
> From: Chenyi Qiang <chenyi.qiang@intel.com>
> 
> Add PKRS caching into the standard register caching mechanism in order
> to take advantage of the availability checks provided by regs_avail.
> 
> This is because vcpu->arch.pkrs will be rarely acceesed by KVM, only in
> the case of host userspace MSR reads and GVA->GPA translation in
> following patches. It is unnecessary to keep it up-to-date at all times.
> 
> It also should be noted that the potential benefits of this caching are
> tenuous because the MSR read is not a hot path. it's nice-to-have so that
> we don't hesitate to rip it out in the future if there's a strong reason
> to drop the caching.


The patch looks fine, but this needs to be moved to the end of the series.
Definitely after "KVM: VMX: Expose PKS to guest", and maybe even after "KVM: VMX:
Enable PKS for nested VM".

If there's a bug with the caching logic, I want to be able to bisect to that.  By
implementing caching before any of the other PKS support, a bug in either the
caching or the virtualization will bisect to the PKS virtualization