Date:   Wed, 1 Feb 2023 19:45:04 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Ben Gardon <bgardon@google.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>,
        Peter Xu <peterx@redhat.com>,
        David Matlack <dmatlack@google.com>,
        Vipin Sharma <vipinsh@google.com>,
        Nagareddy Reddy <nspreddy@google.com>
Subject: Re: [RFC 01/14] KVM: x86/MMU: Add shadow_mmu.(c|h)
Message-ID: <Y9rBQAJg+ITHnVfw@google.com>
References: <20221221222418.3307832-1-bgardon@google.com>
 <20221221222418.3307832-2-bgardon@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221221222418.3307832-2-bgardon@google.com>
Precedence: bulk

On Wed, Dec 21, 2022, Ben Gardon wrote:
> diff --git a/arch/x86/kvm/mmu/shadow_mmu.c b/arch/x86/kvm/mmu/shadow_mmu.c
> new file mode 100644
> index 000000000000..7bce5ec52b2e
> --- /dev/null
> +++ b/arch/x86/kvm/mmu/shadow_mmu.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KVM Shadow MMU
> + *
> + * This file implements the Shadow MMU: the KVM MMU implementation which has
> + * developed organically from hardware which did not have second level paging,
> + * and so used "shadow paging" to virtualize guest memory. The Shadow MMU is
> + * an alternative to the TDP MMU which only supports hardware with Two
> + * Dimentional Paging. (e.g. EPT on Intel or NPT on AMD CPUs.) Note that the
> + * Shadow MMU also supports TDP, it's just less scalable. The Shadow and TDP
> + * MMUs can cooperate to support nested virtualization on hardware with TDP.
> + */

Eh, I vote to omit the comment.  For newbies, Documentation is likely a better
landing spot for describing the MMUs, and people that are familiar with KVM x86
MMU already know what the shadow MMU is and does.  That way we avoid bikeshedding
this comment, at least in the conext of this series.  E.g. I'm pretty sure much
of the shadow MMU behavior wasn't developed organically, it was stolen from Xen.
And the line about the Shadow and TDP MMUs cooperating support nested virt is
loaded with assumptions and qualifiers, and makes it sound like nested virt only
works with _the_ TDP MMU as oposed to _a_ TDP MMU`.