From: Ben Gardon <bgardon@google.com>
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>
Subject: Re: [PATCH 09/21] KVM: x86/MMU: Move paging_tmpl.h includes to shadow_mmu.c
Date: Tue, 21 Mar 2023 11:43:10 -0700 [thread overview]
Message-ID: <CANgfPd-QFtBO0ndYQwtJ13XSPF6tNSwwJDyq93=B36u3gohoxg@mail.gmail.com> (raw)
In-Reply-To: <ZBio2Cs7UrkkilTc@google.com>
On Mon, Mar 20, 2023 at 11:41 AM 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 unique to
> the shadow MMU, all of the guest walker code needs to exist independent of 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. root_role
> cleanup, and I think we should figure out a way to give paging_tmpl.h similar
> treatment. E.g. split paging_tmpl.h itself in some way.
>
> Unfortunately, this is a sticking point for me. If the code movement were minor
> and/or cleaner in nature (definitely not your fault, simply the reality of the
> code base), I might feel differently. But as it stands, there is a lot of churn
> to get to an endpoint that has significant flaws.
>
> So while I love the idea of separating the MMU implementations from the common
> MMU logic, because the guest walker stuff is a lynchpin of sorts, e.g. splitting
> 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 am 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.
next prev parent reply other threads:[~2023-03-21 18:43 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-02 18:27 [PATCH 00/21] KVM: x86/MMU: Formalize the Shadow MMU Ben Gardon
2023-02-02 18:27 ` [PATCH 01/21] KVM: x86/mmu: Rename slot rmap walkers to add clarity and clean up code Ben Gardon
2023-02-02 18:27 ` [PATCH 02/21] KVM: x86/mmu: Replace comment with an actual lockdep assertion on mmu_lock Ben Gardon
2023-02-02 18:27 ` [PATCH 03/21] KVM: x86/mmu: Clean up mmu.c functions that put return type on separate line Ben Gardon
2023-02-02 18:27 ` [PATCH 04/21] KVM: x86/MMU: Add shadow_mmu.(c|h) Ben Gardon
2023-02-02 18:27 ` [PATCH 05/21] KVM: x86/MMU: Expose functions for the Shadow MMU Ben Gardon
2023-02-02 18:27 ` [PATCH 06/21] KVM: x86/mmu: Get rid of is_cpuid_PSE36() Ben Gardon
2023-03-20 18:51 ` Sean Christopherson
2023-02-02 18:27 ` [PATCH 08/21] KVM: x86/MMU: Expose functions for paging_tmpl.h Ben Gardon
2023-02-02 18:27 ` [PATCH 09/21] KVM: x86/MMU: Move paging_tmpl.h includes to shadow_mmu.c Ben Gardon
2023-03-20 18:41 ` Sean Christopherson
2023-03-21 18:43 ` Ben Gardon [this message]
2023-02-02 18:27 ` [PATCH 10/21] KVM: x86/MMU: Clean up Shadow MMU exports Ben Gardon
2023-02-02 18:27 ` [PATCH 11/21] KVM: x86/MMU: Cleanup shrinker interface with Shadow MMU Ben Gardon
2023-02-04 13:52 ` kernel test robot
2023-02-04 19:10 ` kernel test robot
2023-02-02 18:28 ` [PATCH 12/21] KVM: x86/MMU: Clean up naming of exported Shadow MMU functions Ben Gardon
2023-02-02 18:28 ` [PATCH 13/21] KVM: x86/MMU: Fix naming on prepare / commit zap page functions Ben Gardon
2023-02-02 18:28 ` [PATCH 14/21] KVM: x86/MMU: Factor Shadow MMU wrprot / clear dirty ops out of mmu.c Ben Gardon
2023-02-02 18:28 ` [PATCH 15/21] KVM: x86/MMU: Remove unneeded exports from shadow_mmu.c Ben Gardon
2023-02-04 14:44 ` kernel test robot
2023-02-02 18:28 ` [PATCH 16/21] KVM: x86/MMU: Wrap uses of kvm_handle_gfn_range in mmu.c Ben Gardon
2023-02-02 18:28 ` [PATCH 17/21] KVM: x86/MMU: Add kvm_shadow_mmu_ to the last few functions in shadow_mmu.h Ben Gardon
2023-02-02 18:28 ` [PATCH 18/21] KVM: x86/mmu: Move split cache topup functions to shadow_mmu.c Ben Gardon
2023-02-02 18:28 ` [PATCH 19/21] KVM: x86/mmu: Move Shadow MMU part of kvm_mmu_zap_all() to shadow_mmu.h Ben Gardon
2023-02-02 18:28 ` [PATCH 20/21] KVM: x86/mmu: Move Shadow MMU init/teardown to shadow_mmu.c Ben Gardon
2023-02-02 18:28 ` [PATCH 21/21] KVM: x86/mmu: Split out Shadow MMU lockless walk begin/end Ben Gardon
2023-03-20 19:09 ` [PATCH 00/21] KVM: x86/MMU: Formalize the Shadow MMU Sean Christopherson
2023-03-23 23:02 ` Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANgfPd-QFtBO0ndYQwtJ13XSPF6tNSwwJDyq93=B36u3gohoxg@mail.gmail.com' \
--to=bgardon@google.com \
--cc=dmatlack@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=ricarkol@google.com \
--cc=seanjc@google.com \
--cc=vipinsh@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).