MIME-Version: 1.0
References: <20230202182809.1929122-1-bgardon@google.com> <20230202182809.1929122-10-bgardon@google.com>
 <ZBio2Cs7UrkkilTc@google.com>
In-Reply-To: <ZBio2Cs7UrkkilTc@google.com>
From:   Ben Gardon <bgardon@google.com>
Date:   Tue, 21 Mar 2023 11:43:10 -0700
Message-ID: <CANgfPd-QFtBO0ndYQwtJ13XSPF6tNSwwJDyq93=B36u3gohoxg@mail.gmail.com>
Subject: Re: [PATCH 09/21] KVM: x86/MMU: Move paging_tmpl.h includes to shadow_mmu.c
To:     Sean Christopherson <seanjc@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>,
        Ricardo Koller <ricarkol@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Mar 20, 2023 at 11:41=E2=80=AFAM Sean Christopherson <seanjc@google=
.com> wrote:
>
> First off, I apologize for not giving this feedback in the RFC.  I didn't=
 think
> too hard about the impliciations of moving paging_tmpl.h until I actually=
 looked
> at the code.
>
> On Thu, Feb 02, 2023, Ben Gardon wrote:
> > Move the integration point for paging_tmpl.h to shadow_mmu.c since
> > paging_tmpl.h is ostensibly part of the Shadow MMU.
>
> Ostensibly indeed.  While a simple majority of paging_tmpl.h is indeed un=
ique to
> the shadow MMU, all of the guest walker code needs to exist independent o=
f the
> shadow MMU.  And that code is signficant both in terms of lines of code, =
and
> more importantly in terms of understanding its role in KVM at large.
>
> This is essentially the same mess that eventually led the cpu_role vs. ro=
ot_role
> cleanup, and I think we should figure out a way to give paging_tmpl.h sim=
ilar
> treatment.  E.g. split paging_tmpl.h itself in some way.
>
> Unfortunately, this is a sticking point for me.  If the code movement wer=
e minor
> and/or cleaner in nature (definitely not your fault, simply the reality o=
f the
> code base), I might feel differently.  But as it stands, there is a lot o=
f churn
> to get to an endpoint that has significant flaws.
>
> So while I love the idea of separating the MMU implementations from the c=
ommon
> MMU logic, because the guest walker stuff is a lynchpin of sorts, e.g. sp=
litting
> out the guest walker logic could go hand-in-hand with reworking guest_mmu=
, I don't
> want to take this series as is.
>
> Sadly, as much as I'm itching to dive in and do a bit of exploration, I a=
m woefully
> short on bandwidth right now, so all I can do is say no.  Sorry :-(

Fair enough, thanks for taking a look. I'm not going to have bandwidth
in the foreseeable future to work on this any more either,
unfortunately. I'd love it is someone picked up this series and did
the paging_tmpl.h split, but that's  going to be a lot of work, so in
the meantime, I don't mind just letting this die.