From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F28DC433C1 for ; Thu, 25 Mar 2021 19:16:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D367A619B6 for ; Thu, 25 Mar 2021 19:16:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230293AbhCYTP4 (ORCPT ); Thu, 25 Mar 2021 15:15:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57178 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230159AbhCYTPf (ORCPT ); Thu, 25 Mar 2021 15:15:35 -0400 Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6F949C06175F for ; Thu, 25 Mar 2021 12:15:35 -0700 (PDT) Received: by mail-pf1-x42b.google.com with SMTP id l3so3048491pfc.7 for ; Thu, 25 Mar 2021 12:15:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Pps8ZS01M3z1xz1C0cNKowfwUf0UTmIQfYjXVnFmJ14=; b=KDN5PKPzJ+pYGa7kSdcdq9OqiudA1Ymdjo2z06MMOfUiE6cJt8TRmxv/67jaj24Wx+ EbyQU7F8rj3JpC4/FhqSpggGHf+V8VWlmwsubhbIfBpBsXWzwZXxihOTEf0gIm3hH+Cx x1qbbHG7RKT3LBuoPG10JqdLB+t1q9RLPE9RrRxynaitx7OxG61eyyOEFWX9P6usMPsy fMKgJ9X/6vFbFLHlNqZRuiRgDfP5ObxDmutb/f1mQIKkjRdL/EYMCwwGVjeG0VMKMk42 xTdPiQXZD0kuRs/u0pnL3PGf7vVG6/3Zpxdu3gN/BLBNxYQpg7kEtQ5HAgi7RyQdlDxo +V0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Pps8ZS01M3z1xz1C0cNKowfwUf0UTmIQfYjXVnFmJ14=; b=A6OdgEjQ8uJ8V+Vwtjh93EDBO9sIoUGRxv+I0mxscG/PjokQmzyiLVds6F7mjL2K2f laOx1xWphJY36CSUtwIdopupQ/vw2wn08rhLs8a8TBoBzMYJOhCQgSMqtKsSzWunCD/O GPNfm06n+zh+loz9mTWMXBk1toa8RDGKcvXtNXEqQQH5QvaHDE1mO155ZGQSv2cefClb 0cHZmRjqKsUP3XlnbODZvdBhpiV1hhklYHYWxdc43YN6OFWgF1pVwsls/NckhC3k/cO8 6CkF9He6gNDbcioCybmhrUc9lDcTAqXeem3XAOKCNcu+JYvxu9qAcnBE2XpLVe251E1a fBcw== X-Gm-Message-State: AOAM532lC6BYPoyVbSNd2IKEtSUuz69NfHpCxwTSgAKbWJTqFJpys4wU SfTDolCx5WyDBKQITVmQQML5XA== X-Google-Smtp-Source: ABdhPJzth6VurW7T55fgm24siBOTzI7x9W60MghvhahOjxjWVnDxiUhCx9tNMslxz9C1MsxdF7z+9A== X-Received: by 2002:a63:2e87:: with SMTP id u129mr8955650pgu.107.1616699734550; Thu, 25 Mar 2021 12:15:34 -0700 (PDT) Received: from google.com (240.111.247.35.bc.googleusercontent.com. [35.247.111.240]) by smtp.gmail.com with ESMTPSA id o197sm7014687pfd.42.2021.03.25.12.15.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Mar 2021 12:15:33 -0700 (PDT) Date: Thu, 25 Mar 2021 19:15:30 +0000 From: Sean Christopherson To: Ben Gardon Cc: Paolo Bonzini , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , kvm , LKML Subject: Re: [PATCH 2/2] KVM: x86/mmu: Ensure TLBs are flushed when yielding during NX zapping Message-ID: References: <20210319232006.3468382-1-seanjc@google.com> <20210319232006.3468382-3-seanjc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 23, 2021, Ben Gardon wrote: > On Tue, Mar 23, 2021 at 11:58 AM Sean Christopherson wrote: > > > > On Tue, Mar 23, 2021, Ben Gardon wrote: > > > On Mon, Mar 22, 2021 at 5:15 PM Sean Christopherson wrote: > > > > > > > > On Mon, Mar 22, 2021, Ben Gardon wrote: > > > > > It could be fixed by forbidding kvm_tdp_mmu_zap_gfn_range from > > > > > yielding. Since we should only need to zap one SPTE, the yield should > > > > > not be needed within the kvm_tdp_mmu_zap_gfn_range call. To ensure > > > > > that only one SPTE is zapped we would have to specify the root though. > > > > > Otherwise we could end up zapping all the entries for the same GFN > > > > > range under an unrelated root. > > > > > > > > Hmm, I originally did exactly that, but changed my mind because this zaps far > > > > more than 1 SPTE. This is zapping a SP that could be huge, but is not, which > > > > means it's guaranteed to have a non-zero number of child SPTEs. The worst case > > > > scenario is that SP is a PUD (potential 1gb page) and the leafs are 4k SPTEs. > > > > > > It's true that there are potentially 512^2 child sptes, but the code > > > to clear those after the single PUD spte is cleared doesn't yield > > > anyway. If the TDP MMU is only operating with one root (as we would > > > expect in most cases), there should only be one chance for it to > > > yield. > > > > Ah, right, I was thinking all the iterative flows yielded. Disallowing > > kvm_tdp_mmu_zap_gfn_range() from yielding in this case does seem like the best > > fix. Any objection to me sending v2 with that? > > That sounds good to me. Ewww. This analysis isn't 100% accurate. It's actually impossible for zap_gfn_range() to yield in this case. Even though it may walk multiple roots and levels, "yielded_gfn == next_last_level_gfn" will hold true until the iter attempts to step sideways. When the iter attempts to step sideways, it will always do so at the level that matches the zapping level, and so will always step past "end". Thus, tdp_root_for_each_pte() will break without ever yielding. That being said, I'm still going to send a patch to explicitly prevent this path from yielding. Relying on the above is waaaay too subtle and fragile. > > > I've considered how we could allow the recursive changed spte handlers > > > to yield, but it gets complicated quite fast because the caller needs > > > to know if it yielded and reset the TDP iterator to the root, and > > > there are some cases (mmu notifiers + vCPU path) where yielding is not > > > desirable. > > > > Urgh, yeah, seems like we'd quickly end up with a mess resembling the legacy MMU > > iterators. > > > > > > > > > > But, I didn't consider the interplay between invalid_list and the TDP MMU > > > > yielding. Hrm.