[PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[Shameer Kolothum]

The stub for !CONFIG_HAVE_KVM_DIRTY_RING case is missing.

Fixes: cf87ac739e48 ("KVM: x86: Introduce KVM_REQ_DIRTY_RING_SOFT_FULL")
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 include/linux/kvm_dirty_ring.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h
index 4862c98d80d3..a00301059da5 100644
--- a/include/linux/kvm_dirty_ring.h
+++ b/include/linux/kvm_dirty_ring.h
@@ -69,6 +69,11 @@ static inline void kvm_dirty_ring_free(struct kvm_dirty_ring *ring)
 {
 }
 
+static inline bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu)
+{
+	return false;
+}
+
 #else /* CONFIG_HAVE_KVM_DIRTY_RING */
 
 int kvm_cpu_dirty_log_size(void);
-- 
2.34.1

Re: [PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[Sean Christopherson]

On Thu, Mar 16, 2023, Shameer Kolothum wrote:
> The stub for !CONFIG_HAVE_KVM_DIRTY_RING case is missing.

No stub is needed.  kvm_dirty_ring_check_request() isn't called from common code,
and should not (and isn't unless I'm missing something) be called from arch code
unless CONFIG_HAVE_KVM_DIRTY_RING=y.

x86 and arm64 are the only users, and they both select HAVE_KVM_DIRTY_RING
unconditionally when KVM is enabled.

RE: [PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Sean Christopherson [mailto:seanjc@google.com]
> Sent: 16 March 2023 17:02
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; gshan@redhat.com;
> maz@kernel.org
> Subject: Re: [PATCH] KVM: Add the missing stub function for
> kvm_dirty_ring_check_request()
> 
> On Thu, Mar 16, 2023, Shameer Kolothum wrote:
> > The stub for !CONFIG_HAVE_KVM_DIRTY_RING case is missing.
> 
> No stub is needed.  kvm_dirty_ring_check_request() isn't called from
> common code,
> and should not (and isn't unless I'm missing something) be called from arch
> code
> unless CONFIG_HAVE_KVM_DIRTY_RING=y.
> 
> x86 and arm64 are the only users, and they both select
> HAVE_KVM_DIRTY_RING
> unconditionally when KVM is enabled.

Yes, it is at present not called from anywhere other than x86 and arm64.
But I still think since it is a common helper, better to have a stub.

Thanks,
Shameer

Re: [PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[Sean Christopherson]

On Thu, Mar 16, 2023, Shameerali Kolothum Thodi wrote:
> > From: Sean Christopherson [mailto:seanjc@google.com]
> > On Thu, Mar 16, 2023, Shameer Kolothum wrote:
> > > The stub for !CONFIG_HAVE_KVM_DIRTY_RING case is missing.
> > 
> > No stub is needed.  kvm_dirty_ring_check_request() isn't called from
> > common code,
> > and should not (and isn't unless I'm missing something) be called from arch
> > code
> > unless CONFIG_HAVE_KVM_DIRTY_RING=y.
> > 
> > x86 and arm64 are the only users, and they both select
> > HAVE_KVM_DIRTY_RING
> > unconditionally when KVM is enabled.
> 
> Yes, it is at present not called from anywhere other than x86 and arm64.
> But I still think since it is a common helper, better to have a stub.

Why?  It buys us nothing other than dead code, and even worse it could let a bug
that would otherwise be caught during build time escape to run time.

RE: [PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Sean Christopherson [mailto:seanjc@google.com]
> Sent: 16 March 2023 19:57
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; gshan@redhat.com;
> maz@kernel.org
> Subject: Re: [PATCH] KVM: Add the missing stub function for
> kvm_dirty_ring_check_request()
> 
> On Thu, Mar 16, 2023, Shameerali Kolothum Thodi wrote:
> > > From: Sean Christopherson [mailto:seanjc@google.com] On Thu, Mar 16,
> > > 2023, Shameer Kolothum wrote:
> > > > The stub for !CONFIG_HAVE_KVM_DIRTY_RING case is missing.
> > >
> > > No stub is needed.  kvm_dirty_ring_check_request() isn't called from
> > > common code, and should not (and isn't unless I'm missing something)
> > > be called from arch code unless CONFIG_HAVE_KVM_DIRTY_RING=y.
> > >
> > > x86 and arm64 are the only users, and they both select
> > > HAVE_KVM_DIRTY_RING unconditionally when KVM is enabled.
> >
> > Yes, it is at present not called from anywhere other than x86 and arm64.
> > But I still think since it is a common helper, better to have a stub.
> 
> Why?  It buys us nothing other than dead code, and even worse it could let
> a bug that would otherwise be caught during build time escape to run time.

Agree, it buys nothing now:) It just came up while I was playing with a custom
build without HAVE_KVM_DIRTY_RING. Since all other functions there has a stub
just thought it would make it easier for future common usage. We could very well
leave it till that comes up as well.

Thanks,
Shameer

Re: [PATCH] KVM: Add the missing stub function for kvm_dirty_ring_check_request()

[Sean Christopherson]

On Thu, Mar 16, 2023, Shameerali Kolothum Thodi wrote:
> > From: Sean Christopherson [mailto:seanjc@google.com]
> > On Thu, Mar 16, 2023, Shameerali Kolothum Thodi wrote:
> > > > From: Sean Christopherson [mailto:seanjc@google.com] On Thu, Mar 16,
> > > > 2023, Shameer Kolothum wrote:
> > > > > The stub for !CONFIG_HAVE_KVM_DIRTY_RING case is missing.
> > > >
> > > > No stub is needed.  kvm_dirty_ring_check_request() isn't called from
> > > > common code, and should not (and isn't unless I'm missing something)
> > > > be called from arch code unless CONFIG_HAVE_KVM_DIRTY_RING=y.
> > > >
> > > > x86 and arm64 are the only users, and they both select
> > > > HAVE_KVM_DIRTY_RING unconditionally when KVM is enabled.
> > >
> > > Yes, it is at present not called from anywhere other than x86 and arm64.
> > > But I still think since it is a common helper, better to have a stub.
> > 
> > Why?  It buys us nothing other than dead code, and even worse it could let
> > a bug that would otherwise be caught during build time escape to run time.
> 
> Agree, it buys nothing now:) It just came up while I was playing with a custom
> build without HAVE_KVM_DIRTY_RING. Since all other functions there has a stub
> just thought it would make it easier for future common usage. We could very well
> leave it till that comes up as well.

Stubs are typically only added when they are strictly necessary.  Providing a stub
would make things "easier" in the sense that it could theoretically avoid a build
error, but as above, in many cases we _want_ build errors when new code behaves
in a way that diverges from what's expected/established.