From: Like Xu <[email protected]>
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 <[email protected]>
---
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
У сб, 2023-10-07 у 14:40 +0800, Like Xu пише:
> From: Like Xu <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
Best regards,
Maxim Levitsky
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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
> Best regards,
> Maxim Levitsky
>
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.
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.
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.
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.
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