2023-07-22 02:55:56

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

From: David Matlack <[email protected]>

Rename kvm_arch_flush_remote_tlb() and the associated macro
__KVM_HAVE_ARCH_FLUSH_REMOTE_TLB to kvm_arch_flush_remote_tlbs() and
__KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS respectively.

Making the name plural matches kvm_flush_remote_tlbs() and makes it more
clear that this function can affect more than one remote TLB.

No functional change intended.

Signed-off-by: David Matlack <[email protected]>
Signed-off-by: Raghavendra Rao Ananta <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Reviewed-by: Shaoqin Huang <[email protected]>
---
arch/mips/include/asm/kvm_host.h | 4 ++--
arch/mips/kvm/mips.c | 2 +-
arch/x86/include/asm/kvm_host.h | 4 ++--
include/linux/kvm_host.h | 4 ++--
virt/kvm/kvm_main.c | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 04cedf9f8811..9b0ad8f3bf32 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -896,7 +896,7 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}

-#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
-int kvm_arch_flush_remote_tlb(struct kvm *kvm);
+#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
+int kvm_arch_flush_remote_tlbs(struct kvm *kvm);

#endif /* __MIPS_KVM_HOST_H__ */
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index aa5583a7b05b..4b7bc39a4173 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -981,7 +981,7 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)

}

-int kvm_arch_flush_remote_tlb(struct kvm *kvm)
+int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
{
kvm_mips_callbacks->prepare_flush_shadow(kvm);
return 1;
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 28bd38303d70..a2d3cfc2eb75 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1794,8 +1794,8 @@ static inline struct kvm *kvm_arch_alloc_vm(void)
#define __KVM_HAVE_ARCH_VM_FREE
void kvm_arch_free_vm(struct kvm *kvm);

-#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
-static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
+#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
+static inline int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
{
if (kvm_x86_ops.flush_remote_tlbs &&
!static_call(kvm_x86_flush_remote_tlbs)(kvm))
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9d3ac7720da9..e3f968b38ae9 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1479,8 +1479,8 @@ static inline void kvm_arch_free_vm(struct kvm *kvm)
}
#endif

-#ifndef __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
-static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
+#ifndef __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
+static inline int kvm_arch_flush_remote_tlbs(struct kvm *kvm)
{
return -ENOTSUPP;
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index dfbaafbe3a00..70e5479797ac 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -361,7 +361,7 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
* kvm_make_all_cpus_request() reads vcpu->mode. We reuse that
* barrier here.
*/
- if (!kvm_arch_flush_remote_tlb(kvm)
+ if (!kvm_arch_flush_remote_tlbs(kvm)
|| kvm_make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
++kvm->stat.generic.remote_tlb_flush;
}
--
2.41.0.487.g6d72f3e995-goog



2023-07-27 10:56:05

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

On Sat, 22 Jul 2023 03:22:40 +0100,
Raghavendra Rao Ananta <[email protected]> wrote:
>
> From: David Matlack <[email protected]>
>
> Rename kvm_arch_flush_remote_tlb() and the associated macro
> __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB to kvm_arch_flush_remote_tlbs() and
> __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS respectively.
>
> Making the name plural matches kvm_flush_remote_tlbs() and makes it more
> clear that this function can affect more than one remote TLB.
>
> No functional change intended.
>
> Signed-off-by: David Matlack <[email protected]>
> Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> Reviewed-by: Gavin Shan <[email protected]>
> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> Reviewed-by: Shaoqin Huang <[email protected]>
> ---
> arch/mips/include/asm/kvm_host.h | 4 ++--
> arch/mips/kvm/mips.c | 2 +-
> arch/x86/include/asm/kvm_host.h | 4 ++--
> include/linux/kvm_host.h | 4 ++--
> virt/kvm/kvm_main.c | 2 +-
> 5 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index 04cedf9f8811..9b0ad8f3bf32 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -896,7 +896,7 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
> static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
>
> -#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
> -int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> +#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
> +int kvm_arch_flush_remote_tlbs(struct kvm *kvm);

How about making this prototype global? I don't see a point in having
it per-architecture, specially as you are adding arm64 to that mix in
the following patch.

M.

--
Without deviation from the norm, progress is not possible.

2023-07-31 17:50:37

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

On Thu, Jul 27, 2023 at 3:24 AM Marc Zyngier <[email protected]> wrote:
>
> On Sat, 22 Jul 2023 03:22:40 +0100,
> Raghavendra Rao Ananta <[email protected]> wrote:
> >
> > From: David Matlack <[email protected]>
> >
> > Rename kvm_arch_flush_remote_tlb() and the associated macro
> > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB to kvm_arch_flush_remote_tlbs() and
> > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS respectively.
> >
> > Making the name plural matches kvm_flush_remote_tlbs() and makes it more
> > clear that this function can affect more than one remote TLB.
> >
> > No functional change intended.
> >
> > Signed-off-by: David Matlack <[email protected]>
> > Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> > Reviewed-by: Gavin Shan <[email protected]>
> > Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> > Reviewed-by: Shaoqin Huang <[email protected]>
> > ---
> > arch/mips/include/asm/kvm_host.h | 4 ++--
> > arch/mips/kvm/mips.c | 2 +-
> > arch/x86/include/asm/kvm_host.h | 4 ++--
> > include/linux/kvm_host.h | 4 ++--
> > virt/kvm/kvm_main.c | 2 +-
> > 5 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> > index 04cedf9f8811..9b0ad8f3bf32 100644
> > --- a/arch/mips/include/asm/kvm_host.h
> > +++ b/arch/mips/include/asm/kvm_host.h
> > @@ -896,7 +896,7 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
> > static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> > static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> >
> > -#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
> > -int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> > +#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
> > +int kvm_arch_flush_remote_tlbs(struct kvm *kvm);
>
> How about making this prototype global? I don't see a point in having
> it per-architecture, specially as you are adding arm64 to that mix in
> the following patch.
>
We can make it global, but I'm not sure what was the intention of the
original author. My guess is that he was following the same style that
we have for some of the other kvm_arch_*() functions
(kvm_arch_free_vm() for example)?

- Raghavendra
> M.
>
> --
> Without deviation from the norm, progress is not possible.

2023-07-31 23:29:30

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

On Mon, Jul 31, 2023, Raghavendra Rao Ananta wrote:
> On Thu, Jul 27, 2023 at 3:24 AM Marc Zyngier <[email protected]> wrote:
> >
> > On Sat, 22 Jul 2023 03:22:40 +0100,
> > Raghavendra Rao Ananta <[email protected]> wrote:
> > >
> > > From: David Matlack <[email protected]>
> > >
> > > Rename kvm_arch_flush_remote_tlb() and the associated macro
> > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB to kvm_arch_flush_remote_tlbs() and
> > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS respectively.
> > >
> > > Making the name plural matches kvm_flush_remote_tlbs() and makes it more
> > > clear that this function can affect more than one remote TLB.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: David Matlack <[email protected]>
> > > Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> > > Reviewed-by: Gavin Shan <[email protected]>
> > > Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> > > Reviewed-by: Shaoqin Huang <[email protected]>
> > > ---
> > > arch/mips/include/asm/kvm_host.h | 4 ++--
> > > arch/mips/kvm/mips.c | 2 +-
> > > arch/x86/include/asm/kvm_host.h | 4 ++--
> > > include/linux/kvm_host.h | 4 ++--
> > > virt/kvm/kvm_main.c | 2 +-
> > > 5 files changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> > > index 04cedf9f8811..9b0ad8f3bf32 100644
> > > --- a/arch/mips/include/asm/kvm_host.h
> > > +++ b/arch/mips/include/asm/kvm_host.h
> > > @@ -896,7 +896,7 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
> > > static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> > > static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> > >
> > > -#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
> > > -int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> > > +#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
> > > +int kvm_arch_flush_remote_tlbs(struct kvm *kvm);
> >
> > How about making this prototype global? I don't see a point in having
> > it per-architecture, specially as you are adding arm64 to that mix in
> > the following patch.
> >
> We can make it global, but I'm not sure what was the intention of the
> original author. My guess is that he was following the same style that
> we have for some of the other kvm_arch_*() functions
> (kvm_arch_free_vm() for example)?

Heh, KVM has a *lot* of code that was written with questionable style. I agree
with Marc, I can't think of a single reason not to have the definition in common
code. Declaring the function doesn't preclude a "static inline" implementation,
and we could even keep the prototype under an #ifdef, e.g.

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9d3ac7720da9..5ac64f933547 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1484,6 +1484,8 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
{
return -ENOTSUPP;
}
+#else
+int kvm_arch_flush_remote_tlb(struct kvm *kvm);
#endif

#ifdef __KVM_HAVE_ARCH_NONCOHERENT_DMA


2023-08-01 02:01:35

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

On Mon, Jul 31, 2023 at 2:42 PM Sean Christopherson <[email protected]> wrote:
>
> On Mon, Jul 31, 2023, Raghavendra Rao Ananta wrote:
> > On Thu, Jul 27, 2023 at 3:24 AM Marc Zyngier <[email protected]> wrote:
> > >
> > > On Sat, 22 Jul 2023 03:22:40 +0100,
> > > Raghavendra Rao Ananta <[email protected]> wrote:
> > > >
> > > > From: David Matlack <[email protected]>
> > > >
> > > > Rename kvm_arch_flush_remote_tlb() and the associated macro
> > > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB to kvm_arch_flush_remote_tlbs() and
> > > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS respectively.
> > > >
> > > > Making the name plural matches kvm_flush_remote_tlbs() and makes it more
> > > > clear that this function can affect more than one remote TLB.
> > > >
> > > > No functional change intended.
> > > >
> > > > Signed-off-by: David Matlack <[email protected]>
> > > > Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> > > > Reviewed-by: Gavin Shan <[email protected]>
> > > > Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> > > > Reviewed-by: Shaoqin Huang <[email protected]>
> > > > ---
> > > > arch/mips/include/asm/kvm_host.h | 4 ++--
> > > > arch/mips/kvm/mips.c | 2 +-
> > > > arch/x86/include/asm/kvm_host.h | 4 ++--
> > > > include/linux/kvm_host.h | 4 ++--
> > > > virt/kvm/kvm_main.c | 2 +-
> > > > 5 files changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> > > > index 04cedf9f8811..9b0ad8f3bf32 100644
> > > > --- a/arch/mips/include/asm/kvm_host.h
> > > > +++ b/arch/mips/include/asm/kvm_host.h
> > > > @@ -896,7 +896,7 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
> > > > static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> > > > static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> > > >
> > > > -#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
> > > > -int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> > > > +#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
> > > > +int kvm_arch_flush_remote_tlbs(struct kvm *kvm);
> > >
> > > How about making this prototype global? I don't see a point in having
> > > it per-architecture, specially as you are adding arm64 to that mix in
> > > the following patch.
> > >
> > We can make it global, but I'm not sure what was the intention of the
> > original author. My guess is that he was following the same style that
> > we have for some of the other kvm_arch_*() functions
> > (kvm_arch_free_vm() for example)?
>
> Heh, KVM has a *lot* of code that was written with questionable style. I agree
> with Marc, I can't think of a single reason not to have the definition in common
> code. Declaring the function doesn't preclude a "static inline" implementation,
> and we could even keep the prototype under an #ifdef, e.g.
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 9d3ac7720da9..5ac64f933547 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1484,6 +1484,8 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
> {
> return -ENOTSUPP;
> }
> +#else
> +int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> #endif
>
> #ifdef __KVM_HAVE_ARCH_NONCOHERENT_DMA
>
Thanks for the suggestions; I can go with a common declaration. Along
with that, do we want to keep defining
__KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS in the arch code that supports it or
convert it into a CONFIG_?

- Raghavendra

2023-08-02 16:50:06

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

On Wed, Aug 02, 2023, Marc Zyngier wrote:
> On Tue, 01 Aug 2023 01:42:54 +0100,
> Raghavendra Rao Ananta <[email protected]> wrote:
> > Thanks for the suggestions; I can go with a common declaration. Along
> > with that, do we want to keep defining
> > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS in the arch code that supports it or
> > convert it into a CONFIG_?
>
> This isn't something that a user can select, more something that is an
> architectural decision. Maybe in a later patch if there is a consensus
> around that, but probably not as part of this series.

+1. I agree it's annoying that KVM uses a mix of Kconfigs and manual #defines
for the various "KVM_HAVE" knobs, but we have so many of both that one-off
conversions without a real need don't make much sense.

2023-08-02 16:51:59

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

On Tue, 01 Aug 2023 01:42:54 +0100,
Raghavendra Rao Ananta <[email protected]> wrote:
>
> On Mon, Jul 31, 2023 at 2:42 PM Sean Christopherson <[email protected]> wrote:
> >
> > On Mon, Jul 31, 2023, Raghavendra Rao Ananta wrote:
> > > On Thu, Jul 27, 2023 at 3:24 AM Marc Zyngier <[email protected]> wrote:
> > > >
> > > > On Sat, 22 Jul 2023 03:22:40 +0100,
> > > > Raghavendra Rao Ananta <[email protected]> wrote:
> > > > >
> > > > > From: David Matlack <[email protected]>
> > > > >
> > > > > Rename kvm_arch_flush_remote_tlb() and the associated macro
> > > > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB to kvm_arch_flush_remote_tlbs() and
> > > > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS respectively.
> > > > >
> > > > > Making the name plural matches kvm_flush_remote_tlbs() and makes it more
> > > > > clear that this function can affect more than one remote TLB.
> > > > >
> > > > > No functional change intended.
> > > > >
> > > > > Signed-off-by: David Matlack <[email protected]>
> > > > > Signed-off-by: Raghavendra Rao Ananta <[email protected]>
> > > > > Reviewed-by: Gavin Shan <[email protected]>
> > > > > Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> > > > > Reviewed-by: Shaoqin Huang <[email protected]>
> > > > > ---
> > > > > arch/mips/include/asm/kvm_host.h | 4 ++--
> > > > > arch/mips/kvm/mips.c | 2 +-
> > > > > arch/x86/include/asm/kvm_host.h | 4 ++--
> > > > > include/linux/kvm_host.h | 4 ++--
> > > > > virt/kvm/kvm_main.c | 2 +-
> > > > > 5 files changed, 8 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> > > > > index 04cedf9f8811..9b0ad8f3bf32 100644
> > > > > --- a/arch/mips/include/asm/kvm_host.h
> > > > > +++ b/arch/mips/include/asm/kvm_host.h
> > > > > @@ -896,7 +896,7 @@ static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
> > > > > static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
> > > > > static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> > > > >
> > > > > -#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLB
> > > > > -int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> > > > > +#define __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS
> > > > > +int kvm_arch_flush_remote_tlbs(struct kvm *kvm);
> > > >
> > > > How about making this prototype global? I don't see a point in having
> > > > it per-architecture, specially as you are adding arm64 to that mix in
> > > > the following patch.
> > > >
> > > We can make it global, but I'm not sure what was the intention of the
> > > original author. My guess is that he was following the same style that
> > > we have for some of the other kvm_arch_*() functions
> > > (kvm_arch_free_vm() for example)?
> >
> > Heh, KVM has a *lot* of code that was written with questionable style. I agree
> > with Marc, I can't think of a single reason not to have the definition in common
> > code. Declaring the function doesn't preclude a "static inline" implementation,
> > and we could even keep the prototype under an #ifdef, e.g.
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 9d3ac7720da9..5ac64f933547 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -1484,6 +1484,8 @@ static inline int kvm_arch_flush_remote_tlb(struct kvm *kvm)
> > {
> > return -ENOTSUPP;
> > }
> > +#else
> > +int kvm_arch_flush_remote_tlb(struct kvm *kvm);
> > #endif
> >
> > #ifdef __KVM_HAVE_ARCH_NONCOHERENT_DMA
> >
> Thanks for the suggestions; I can go with a common declaration. Along
> with that, do we want to keep defining
> __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS in the arch code that supports it or
> convert it into a CONFIG_?

This isn't something that a user can select, more something that is an
architectural decision. Maybe in a later patch if there is a consensus
around that, but probably not as part of this series.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2023-08-03 00:08:34

by Raghavendra Rao Ananta

[permalink] [raw]
Subject: Re: [PATCH v7 01/12] KVM: Rename kvm_arch_flush_remote_tlb() to kvm_arch_flush_remote_tlbs()

Okay, so just the #define in the respective arch header with a global
declaration. I'll consider this in v8.

Thanks,
Raghavendra

On Wed, Aug 2, 2023 at 9:10 AM Sean Christopherson <[email protected]> wrote:
>
> On Wed, Aug 02, 2023, Marc Zyngier wrote:
> > On Tue, 01 Aug 2023 01:42:54 +0100,
> > Raghavendra Rao Ananta <[email protected]> wrote:
> > > Thanks for the suggestions; I can go with a common declaration. Along
> > > with that, do we want to keep defining
> > > __KVM_HAVE_ARCH_FLUSH_REMOTE_TLBS in the arch code that supports it or
> > > convert it into a CONFIG_?
> >
> > This isn't something that a user can select, more something that is an
> > architectural decision. Maybe in a later patch if there is a consensus
> > around that, but probably not as part of this series.
>
> +1. I agree it's annoying that KVM uses a mix of Kconfigs and manual #defines
> for the various "KVM_HAVE" knobs, but we have so many of both that one-off
> conversions without a real need don't make much sense.