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, 9 May 2023 09:40:37 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Chao Gao <chao.gao@intel.com>
CC:     <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <pbonzini@redhat.com>, <seanjc@google.com>
Subject: Re: [PATCH] KVM: x86: Zap all TDP leaf entries according noncoherent
 DMA count
Message-ID: <ZFmkleUwgv/38UrI@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230508034700.7686-1-yan.y.zhao@intel.com>
 <ZFiinKOqGDvyVAkT@chao-email>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ZFiinKOqGDvyVAkT@chao-email>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?pUpK48OBWG6WigxyNh8x3vBz1UGNTiOVsl4ieaQ38g/vTyBQRKgA3ed1O6OD?=
 =?us-ascii?Q?vHSCRxPLlcZtDBHiXq79+q6+tdl5gPOsjQ9k/uOltQ18xygKJO7c4supVnNi?=
 =?us-ascii?Q?guBjVwH/2OD8DkvubBNnLNL8DxjmaUvBmOJYJnkuHkJpvL/oVUQorrFYECBy?=
 =?us-ascii?Q?VngHvabitOh3vkadWSiQLb6KuAZ09vKb6/Gt89H+R0yTsqDUJx3X7rF8IwNf?=
 =?us-ascii?Q?Uc+6vvA+pp5nUdXfYLFL5ujq4UniwKMKRkuL/o0LiERhryadoVNE4r+TyGHE?=
 =?us-ascii?Q?pkXJS8HxRbY9llBGX9BMaGLKtAtGygpN7mG5slYx4Mg8JhchCCwRXnbix1gA?=
 =?us-ascii?Q?XlbL7/l2yGr1vZMhxkYf7ggo0V8t32sF2byWgpNerisGOpIAObdyzYBG+aK7?=
 =?us-ascii?Q?Yae46AHAIULU2yV+9OeGqOnYxkhSoSOP4gp7FZ8dpK4KeUVg8PQUSiCcWi4q?=
 =?us-ascii?Q?QiX1GupNf4lUvgkfDjZTuRKBJQVW0ifPpgY6jrEFFyWlaR/R7sXIXcihfuOv?=
 =?us-ascii?Q?30aFjHNtZFmdhfT+LKwUS/lDtG1jDoGQl4otiOK0VYHTHiAPVJDUrlKaSPPH?=
 =?us-ascii?Q?i9V8VB7sJh7uWMt68A+7TiVSrxhmCaFFN0hHZsWtjM11YOVKs6sJnFOqUpSF?=
 =?us-ascii?Q?R0BF9ZaRS5/mb3xviyPrDs1BmijBeGQ2qF5HhyjidcrcbOFNByyA2Ze8hiKx?=
 =?us-ascii?Q?7AmQZkyVVz332V199uLO7b/BhnMc7NHA4f0AQjMHOjmtJuxGGyLrw3012MZO?=
 =?us-ascii?Q?GiuB+obSKqAOc6KnEh2RepSbBMgGuOszFCQoHrEA8mN7kHAHzxAuoO1UIg1z?=
 =?us-ascii?Q?pujakqCFQMy3dXK63HLRsf884HN5KwdtmXCkZkjAksZ50oI7BvdFWuryFXTF?=
 =?us-ascii?Q?txIqYoICEVAhC1M7gzcPC2HM2EZPBEfllRojatwiEecncl+D/EaHXJy34FT8?=
 =?us-ascii?Q?jyUAJo0ONCqpJ3x9HAZQ9xVHC9YiFqC+ZDpkhvqcePtMHfNTwJeK7gWQwVav?=
 =?us-ascii?Q?H6mTl6fUwKXyR0HbY5wau+xN8kV5U7f7uWSdvp5SjuIrKyUCnh3JxXYueCoZ?=
 =?us-ascii?Q?iPkSWAVH9abK1jCJuFLAm2hCrBI5Ro9D9/Usqy/zu8CDxfw1wBx5gutP3Mjp?=
 =?us-ascii?Q?yRqLQchgWqvACwqDgdjq40+bU7g1UR2gqnRpu7uLSujNTFt3dxnbwjAeF5IU?=
 =?us-ascii?Q?c/tIf+dn065AK8Gy1yMuuMGd3peNUpoK5RPYKMoXgeOKLoTPPqryZLxXrBBl?=
 =?us-ascii?Q?ta6k/phv++w5N4CZlRAQym8M/IYD6VTHkFI7VBuRVI+1Oo0NGQNEIKwsGng+?=
 =?us-ascii?Q?ixUbfWb0i8LmSGUe0giom0XlJ1x4sFMtFR5ev8viKwt3T0e6OHZXGRxeev6W?=
 =?us-ascii?Q?BeePLNe4BIUObKPpvuw74F7fz4g29C7jQQ9q+SnJ4cmwUFvzwRQH9Q7bQOsD?=
 =?us-ascii?Q?S2CBJp/Y9uoHVPHsL/6cJvH8442UWmsIqgejmLP6iABCiEhD+joA6dRvKX3B?=
 =?us-ascii?Q?6NKXCSYQUysBQXt9xOrbhGzxb6b+X+bpE3I8jnwGTcK+GnSwtFgJtkonBPxx?=
 =?us-ascii?Q?QIaGecF3vJjKwtD6cfovXYAMg8ubNd+2yaqcX2r9?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b9add375-513e-4a1e-4686-08db5031e335
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 May 2023 02:05:40.1524
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bjkWW0ajwsbE9r3QO3XCcf+dEbM60Z3h2YtNe5ySlCRidpYaBACXlPsey/FcQq73hk/jmeVgLYJMUs56hmkMoQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6091
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, May 08, 2023 at 03:19:56PM +0800, Chao Gao wrote:
> On Mon, May 08, 2023 at 11:47:00AM +0800, Yan Zhao wrote:
> >Zap all TDP leaf entries when noncoherent DMA count goes from 0 to !0, or
> >from !0 to 0.
> >
> >When there's no noncoherent DMA device, EPT memory type is
> >((MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | VMX_EPT_IPAT_BIT)
> >
> >When there're noncoherent DMA devices, EPT memory type needs to honor
> >guest CR0_CD and MTRR settings.
> >
> >So, if noncoherent DMA count changes between 0 and !0, EPT leaf entries
> >need to be zapped to clear stale memory type.
> >
> >This issue might be hidden when VFIO adding/removing MMIO regions of the
> >noncoherent DMA devices on device attaching/de-attaching because
> >usually the MMIO regions will be disabled/enabled for several times during
> >guest PCI probing. And in KVM, TDP entries are all zapped on memslot
> >removal.
> >
> >However, this issue may appear when kvm_mmu_zap_all_fast() is not called
> >before KVM slot removal, e.g. as for TDX, only leaf entries for the
> >memslot to be removed is zapped.
> >
> >static void kvm_mmu_invalidate_zap_pages_in_memslot(struct kvm *kvm,
> >                        struct kvm_memory_slot *slot,
> >                        struct kvm_page_track_notifier_node *node)
> >{
> >        if (kvm_gfn_shared_mask(kvm))
> >                /*
> >                 * Secure-EPT requires to release PTs from the leaf.  The
> >                 * optimization to zap root PT first with child PT doesn't
> >                 * work.
> >                 */
> >                kvm_mmu_zap_memslot(kvm, slot);
> >        else
> >                kvm_mmu_zap_all_fast(kvm);
> >}
> 
> TDX code isn't merged. So, I think you'd better not use TDX as an argument.
> 
Ok. But I just want to explain that kvm_mmu_zap_all_fast() is not
desired in some cases during slot DELETE. TDX is a good example here.

> >
> >And even without TDX's case, in some extreme conditions if MMIO regions
> >are not disabled during device attaching, e.g. if guest does not cause
> >the MMIO region disabling in QEMU.
> >Then TDP zap will not be called and wrong EPT memory type might be
> >retained.
> >
> >So, do the TDP zapping of all leaf entries when present/non-present state
> >of noncoherent DMA devices changes to ensure stale entries cleaned away.
> >And as this is not a frequent operation, the extra zap should be fine.
> >
> >Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> >---
> > arch/x86/kvm/x86.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >index e7f78fe79b32..99a825722d95 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -13145,13 +13145,15 @@ EXPORT_SYMBOL_GPL(kvm_arch_has_assigned_device);
> > 
> > void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
> > {
> >-	atomic_inc(&kvm->arch.noncoherent_dma_count);
> >+	if (atomic_inc_return(&kvm->arch.noncoherent_dma_count) == 1)
> 
> >+		kvm_zap_gfn_range(kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
> 
> The issue is specific to EPT. shouldn't this be conditional on tdp_enabled, like
> update_mtrr()?
> 
Yes. good point.
Maybe also include checking of shadow_memtype_mask.

> Likewise, shouldn't we omit to call kvm_zap_gfn_range() in kvm_post_set_cr0() if
> tdp_enabled is false?
I think so.
And also check tdp_enabled and shadow_memtype_mask in the case of update_mtrr().

Will add a helper function in next version.

Thanks, Chao!
> 
> > }
> > EXPORT_SYMBOL_GPL(kvm_arch_register_noncoherent_dma);
> > 
> > void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm)
> > {
> >-	atomic_dec(&kvm->arch.noncoherent_dma_count);
> >+	if (!atomic_dec_return(&kvm->arch.noncoherent_dma_count))
> >+		kvm_zap_gfn_range(kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
> > }
> > EXPORT_SYMBOL_GPL(kvm_arch_unregister_noncoherent_dma);
> > 
> >-- 
> >2.17.1
> >