KVM: x86/xsave: Remove 'return void' expression for 'void function'

[PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Like Xu 2 years, 2 months ago

From: Like Xu <likexu@tencent.com>

The requested info will be stored in 'guest_xsave->region' referenced by
the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need
to explicitly use return void expression for a void function "static void
kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic].

Fixes: 2d287ec65e79 ("x86/fpu: Allow caller to constrain xfeatures when copying to uabi buffer")
Signed-off-by: Like Xu <likexu@tencent.com>
---
 arch/x86/kvm/x86.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fdb2b0e61c43..2571466a317f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5503,8 +5503,8 @@ static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
 static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
 					 struct kvm_xsave *guest_xsave)
 {
-	return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
-					     sizeof(guest_xsave->region));
+	kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
+				      sizeof(guest_xsave->region));
 }
 
 static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,

base-commit: 86701e115030e020a052216baa942e8547e0b487
-- 
2.42.0

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Sean Christopherson 2 years, 1 month ago

On Sat, 07 Oct 2023 14:40:19 +0800, Like Xu wrote:
> The requested info will be stored in 'guest_xsave->region' referenced by
> the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need
> to explicitly use return void expression for a void function "static void
> kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic].
> 
> 

Applied to kvm-x86 fixes, thanks!

[1/1] KVM: x86/xsave: Remove 'return void' expression for 'void function'
      https://github.com/kvm-x86/linux/commit/ef8d89033c3f

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Maxim Levitsky 2 years, 2 months ago

У сб, 2023-10-07 у 14:40 +0800, Like Xu пише:
> From: Like Xu <likexu@tencent.com>
> 
> The requested info will be stored in 'guest_xsave->region' referenced by
> the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need
> to explicitly use return void expression for a void function "static void
> kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic].
> 
> Fixes: 2d287ec65e79 ("x86/fpu: Allow caller to constrain xfeatures when copying to uabi buffer")
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
>  arch/x86/kvm/x86.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index fdb2b0e61c43..2571466a317f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5503,8 +5503,8 @@ static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
>  static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
>  					 struct kvm_xsave *guest_xsave)
>  {
> -	return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
> -					     sizeof(guest_xsave->region));
> +	kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
> +				      sizeof(guest_xsave->region));
>  }
>  
>  static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
> 
> base-commit: 86701e115030e020a052216baa942e8547e0b487
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Like Xu 2 years, 2 months ago

Emm, did we miss this little fix ?

On 11/10/2023 12:12 am, Maxim Levitsky wrote:
> У сб, 2023-10-07 у 14:40 +0800, Like Xu пише:
>> From: Like Xu <likexu@tencent.com>
>>
>> The requested info will be stored in 'guest_xsave->region' referenced by
>> the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need
>> to explicitly use return void expression for a void function "static void
>> kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic].
>>
>> Fixes: 2d287ec65e79 ("x86/fpu: Allow caller to constrain xfeatures when copying to uabi buffer")
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> ---
>>   arch/x86/kvm/x86.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index fdb2b0e61c43..2571466a317f 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -5503,8 +5503,8 @@ static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
>>   static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
>>   					 struct kvm_xsave *guest_xsave)
>>   {
>> -	return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
>> -					     sizeof(guest_xsave->region));
>> +	kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region,
>> +				      sizeof(guest_xsave->region));
>>   }
>>   
>>   static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
>>
>> base-commit: 86701e115030e020a052216baa942e8547e0b487
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
> 
> Best regards,
> 	Maxim Levitsky
>

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Sean Christopherson 2 years, 2 months ago

On Wed, Oct 25, 2023, Like Xu wrote:
> Emm, did we miss this little fix ?

No, I have it earmarked, it's just not a priority because it doesn't truly fix
anything.  Though I suppose it probably makes to apply it for 6.8, waiting one
more day to send PULL requests to Paolo isn't a problem.

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Sean Christopherson 2 years, 2 months ago

On Wed, Oct 25, 2023, Sean Christopherson wrote:
> On Wed, Oct 25, 2023, Like Xu wrote:
> > Emm, did we miss this little fix ?
> 
> No, I have it earmarked, it's just not a priority because it doesn't truly fix
> anything.  Though I suppose it probably makes to apply it for 6.8, waiting one
> more day to send PULL requests to Paolo isn't a problem.

Heh, when I tried to apply this I got reminded of why I held it for later.  I
want to apply it to kvm-x86/misc, but that's based on ~6.6-rc2 (plus a few KVM
patches), i.e. doesn't have the "buggy" commit.  I don't want to rebase "misc",
nor do I want to create a branch and PULL request for a single trivial commit.

So for logistical reasons, I'm not going apply this right away, but I will make
sure it gets into v6.7.

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Like Xu 2 years, 1 month ago

On 26/10/2023 9:10 am, Sean Christopherson wrote:
> On Wed, Oct 25, 2023, Sean Christopherson wrote:
>> On Wed, Oct 25, 2023, Like Xu wrote:
>>> Emm, did we miss this little fix ?
>>
>> No, I have it earmarked, it's just not a priority because it doesn't truly fix
>> anything.  Though I suppose it probably makes to apply it for 6.8, waiting one
>> more day to send PULL requests to Paolo isn't a problem.
> 
> Heh, when I tried to apply this I got reminded of why I held it for later.  I
> want to apply it to kvm-x86/misc, but that's based on ~6.6-rc2 (plus a few KVM
> patches), i.e. doesn't have the "buggy" commit.  I don't want to rebase "misc",
> nor do I want to create a branch and PULL request for a single trivial commit.
> 
> So for logistical reasons, I'm not going apply this right away, but I will make
> sure it gets into v6.7.

Thanks, and a similar pattern occurs with these functions:

  'write_register_operand'
  'account_shadowed'
  'unaccount_shadowed'
  'mtrr_lookup_fixed_next'
  'pre_svm_run'
  'svm_vcpu_deliver_sipi_vector'

Although the compiler will do the right thing, use 'return void' expression
deliberately without grounds for exemption may annoy some CI pipelines.

If you need more cleanup or a new version to cover all these cases above,
just let me know.

Re: [PATCH] KVM: x86/xsave: Remove 'return void' expression for 'void function'

Posted by Sean Christopherson 2 years, 1 month ago

On Thu, Nov 30, 2023, Like Xu wrote:
> On 26/10/2023 9:10 am, Sean Christopherson wrote:
> > On Wed, Oct 25, 2023, Sean Christopherson wrote:
> > > On Wed, Oct 25, 2023, Like Xu wrote:
> > > > Emm, did we miss this little fix ?
> > > 
> > > No, I have it earmarked, it's just not a priority because it doesn't truly fix
> > > anything.  Though I suppose it probably makes to apply it for 6.8, waiting one
> > > more day to send PULL requests to Paolo isn't a problem.
> > 
> > Heh, when I tried to apply this I got reminded of why I held it for later.  I
> > want to apply it to kvm-x86/misc, but that's based on ~6.6-rc2 (plus a few KVM
> > patches), i.e. doesn't have the "buggy" commit.  I don't want to rebase "misc",
> > nor do I want to create a branch and PULL request for a single trivial commit.
> > 
> > So for logistical reasons, I'm not going apply this right away, but I will make
> > sure it gets into v6.7.
> 
> Thanks, and a similar pattern occurs with these functions:
> 
>  'write_register_operand'
>  'account_shadowed'
>  'unaccount_shadowed'
>  'mtrr_lookup_fixed_next'
>  'pre_svm_run'
>  'svm_vcpu_deliver_sipi_vector'
> 
> Although the compiler will do the right thing, use 'return void' expression
> deliberately without grounds for exemption may annoy some CI pipelines.
> 
> If you need more cleanup or a new version to cover all these cases above,
> just let me know.

I'd rather update the CI pipelines to turn off -Wpedantic.  There is zero chance
that -Wpedantic will ever get enabled for kernel builds, the kernel is deliberately
not ISO C compliant.  I have no objection to cleaning up kvm_vcpu_ioctl_x86_get_xsave()
because it's an obvious goof and a recent change, but like checkpatch warnings,
I don't want to go around "fixing" warnings unless they are actively problematic
for humans.