Re: [PATCH RFC 0/4] KVM: MIPS: Provide arch-specific kvm_flush_remote_tlbs()

[Peter Xu]

On Tue, Mar 17, 2020 at 02:33:00PM +0100, Paolo Bonzini wrote:
> On 11/03/20 19:32, Peter Xu wrote:
> > On Wed, Feb 12, 2020 at 01:25:30PM +0100, Paolo Bonzini wrote:
> >> On 07/02/20 23:35, Peter Xu wrote:
> >>> [This series is RFC because I don't have MIPS to compile and test]
> >>>
> >>> kvm_flush_remote_tlbs() can be arch-specific, by either:
> >>>
> >>> - Completely replace kvm_flush_remote_tlbs(), like ARM, who is the
> >>>   only user of CONFIG_HAVE_KVM_ARCH_TLB_FLUSH_ALL so far
> >>>
> >>> - Doing something extra before kvm_flush_remote_tlbs(), like MIPS VZ
> >>>   support, however still wants to have the common tlb flush to be part
> >>>   of the process.  Could refer to kvm_vz_flush_shadow_all().  Then in
> >>>   MIPS it's awkward to flush remote TLBs: we'll need to call the mips
> >>>   hooks.
> >>>
> >>> It's awkward to have different ways to specialize this procedure,
> >>> especially MIPS cannot use the genenal interface which is quite a
> >>> pity.  It's good to make it a common interface.
> >>>
> >>> This patch series removes the 2nd MIPS usage above, and let it also
> >>> use the common kvm_flush_remote_tlbs() interface.  It should be
> >>> suggested that we always keep kvm_flush_remote_tlbs() be a common
> >>> entrance for tlb flushing on all archs.
> >>>
> >>> This idea comes from the reading of Sean's patchset on dynamic memslot
> >>> allocation, where a new dirty log specific hook is added for flushing
> >>> TLBs only for the MIPS code [1].  With this patchset, logically the
> >>> new hook in that patch can be dropped so we can directly use
> >>> kvm_flush_remote_tlbs().
> >>>
> >>> TODO: We can even extend another common interface for ranged TLB, but
> >>> let's see how we think about this series first.
> >>>
> >>> Any comment is welcomed, thanks.
> >>>
> >>> Peter Xu (4):
> >>>   KVM: Provide kvm_flush_remote_tlbs_common()
> >>>   KVM: MIPS: Drop flush_shadow_memslot() callback
> >>>   KVM: MIPS: Replace all the kvm_flush_remote_tlbs() references
> >>>   KVM: MIPS: Define arch-specific kvm_flush_remote_tlbs()
> >>>
> >>>  arch/mips/include/asm/kvm_host.h |  7 -------
> >>>  arch/mips/kvm/Kconfig            |  1 +
> >>>  arch/mips/kvm/mips.c             | 22 ++++++++++------------
> >>>  arch/mips/kvm/trap_emul.c        | 15 +--------------
> >>>  arch/mips/kvm/vz.c               | 14 ++------------
> >>>  include/linux/kvm_host.h         |  1 +
> >>>  virt/kvm/kvm_main.c              | 10 ++++++++--
> >>>  7 files changed, 23 insertions(+), 47 deletions(-)
> >>>
> >>
> >> Compile-tested and queued.
> > 
> > Just in case it fells through the crach - Paolo, do you still have
> > plan to queue this again?
> 
> Yes, I wanted to make it compile first though.  I'm undecided between
> queuing your series and killing KVM MIPS honestly.

Understood.  Yep killing that will provide the same thing too as what
the series wanted to do anyways, as we'll remove the only outlier of
kvm_flush_remote_tlbs().  Thanks,

-- 
Peter Xu