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;
Message-ID: <f76cbfeca245fcd5e5d69cb91af9a0a1d6aaf1d0.camel@redhat.com>
Subject: Re: [PATCH 4/4] KVM: x86/mmu: Restrict mapping level based on guest
 MTRR iff they're used
From:   Maxim Levitsky <mlevitsk@redhat.com>
To:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Mon, 18 Jul 2022 15:08:15 +0300
In-Reply-To: <20220715230016.3762909-5-seanjc@google.com>
References: <20220715230016.3762909-1-seanjc@google.com>
         <20220715230016.3762909-5-seanjc@google.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.4 (3.40.4-5.fc34) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, 2022-07-15 at 23:00 +0000, Sean Christopherson wrote:
> Restrict the mapping level for SPTEs based on the guest MTRRs if and only
> if KVM may actually use the guest MTRRs to compute the "real" memtype.
> For all forms of paging, guest MTRRs are purely virtual in the sense that
> they are completely ignored by hardware, i.e. they affect the memtype
> only if software manually consumes them.  The only scenario where KVM
> consumes the guest MTRRs is when shadow_memtype_mask is non-zero and the
> guest has non-coherent DMA, in all other cases KVM simply leaves the PAT
> field in SPTEs as '0' to encode WB memtype.
> 
> Note, KVM may still ultimately ignore guest MTRRs, e.g. if the backing
> pfn is host MMIO, but false positives are ok as they only cause a slight
> performance blip (unless the guest is doing weird things with its MTRRs,
> which is extremely unlikely).
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/mmu/mmu.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 52664c3caaab..82f38af06f5c 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4295,14 +4295,26 @@ EXPORT_SYMBOL_GPL(kvm_handle_page_fault);
>  
>  int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>  {
> -       while (fault->max_level > PG_LEVEL_4K) {
> -               int page_num = KVM_PAGES_PER_HPAGE(fault->max_level);
> -               gfn_t base = (fault->addr >> PAGE_SHIFT) & ~(page_num - 1);
> +       /*
> +        * If the guest's MTRRs may be used to compute the "real" memtype,
> +        * restrict the mapping level to ensure KVM uses a consistent memtype
> +        * across the entire mapping.  If the host MTRRs are ignored by TDP
> +        * (shadow_memtype_mask is non-zero), and the VM has non-coherent DMA
> +        * (DMA doesn't snoop CPU caches), KVM's ABI is to honor the memtype
> +        * from the guest's MTRRs so that guest accesses to memory that is
> +        * DMA'd aren't cached against the guest's wishes.
> +        *
> +        * Note, KVM may still ultimately ignore guest MTRRs for certain PFNs,
> +        * e.g. KVM will force UC memtype for host MMIO.
> +        */
> +       if (shadow_memtype_mask && kvm_arch_has_noncoherent_dma(vcpu->kvm)) {
> +               for ( ; fault->max_level > PG_LEVEL_4K; --fault->max_level) {
> +                       int page_num = KVM_PAGES_PER_HPAGE(fault->max_level);
> +                       gfn_t base = (fault->addr >> PAGE_SHIFT) & ~(page_num - 1);
>  
> -               if (kvm_mtrr_check_gfn_range_consistency(vcpu, base, page_num))
> -                       break;
> -
> -               --fault->max_level;
> +                       if (kvm_mtrr_check_gfn_range_consistency(vcpu, base, page_num))
> +                               break;
> +               }
>         }
>  
>         return direct_page_fault(vcpu, fault);


Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky