* [PATCH 1/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_get_pit()
2019-12-07 9:25 [PATCH 0/6] remove unnecessary return val of kvm pit linmiaohe
@ 2019-12-07 9:25 ` linmiaohe
2019-12-07 9:25 ` [PATCH 2/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_set_pit() linmiaohe
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: linmiaohe @ 2019-12-07 9:25 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
The return val of kvm_vm_ioctl_get_pit() is always equal to 0, which means
there is no way to failed with this function. So remove the return val as
it's unnecessary to check against it.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/x86.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 392d473252f8..732f03c19fdc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4596,7 +4596,7 @@ static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
return r;
}
-static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
+static void kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
{
struct kvm_kpit_state *kps = &kvm->arch.vpit->pit_state;
@@ -4605,7 +4605,6 @@ static int kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
mutex_lock(&kps->lock);
memcpy(ps, &kps->channels, sizeof(*ps));
mutex_unlock(&kps->lock);
- return 0;
}
static int kvm_vm_ioctl_set_pit(struct kvm *kvm, struct kvm_pit_state *ps)
@@ -4981,9 +4980,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = -ENXIO;
if (!kvm->arch.vpit)
goto out;
- r = kvm_vm_ioctl_get_pit(kvm, &u.ps);
- if (r)
- goto out;
+ kvm_vm_ioctl_get_pit(kvm, &u.ps);
r = -EFAULT;
if (copy_to_user(argp, &u.ps, sizeof(struct kvm_pit_state)))
goto out;
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_set_pit()
2019-12-07 9:25 [PATCH 0/6] remove unnecessary return val of kvm pit linmiaohe
2019-12-07 9:25 ` [PATCH 1/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_get_pit() linmiaohe
@ 2019-12-07 9:25 ` linmiaohe
2019-12-07 9:25 ` [PATCH 3/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_get_pit2() linmiaohe
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: linmiaohe @ 2019-12-07 9:25 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
The return val of kvm_vm_ioctl_set_pit() is always equal to 0, which means
there is no way to failed with this function. So remove the return val as
it's unnecessary to check against it.
Also add BUILD_BUG_ON to guard against channels size changed unexpectly.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/x86.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 732f03c19fdc..5628dceb9fa5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4607,17 +4607,18 @@ static void kvm_vm_ioctl_get_pit(struct kvm *kvm, struct kvm_pit_state *ps)
mutex_unlock(&kps->lock);
}
-static int kvm_vm_ioctl_set_pit(struct kvm *kvm, struct kvm_pit_state *ps)
+static void kvm_vm_ioctl_set_pit(struct kvm *kvm, struct kvm_pit_state *ps)
{
int i;
struct kvm_pit *pit = kvm->arch.vpit;
+ BUILD_BUG_ON(sizeof(*ps) != sizeof(pit->pit_state.channels));
+
mutex_lock(&pit->pit_state.lock);
memcpy(&pit->pit_state.channels, ps, sizeof(*ps));
for (i = 0; i < 3; i++)
kvm_pit_load_count(pit, i, ps->channels[i].count, 0);
mutex_unlock(&pit->pit_state.lock);
- return 0;
}
static int kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
@@ -4994,7 +4995,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = -ENXIO;
if (!kvm->arch.vpit)
goto out;
- r = kvm_vm_ioctl_set_pit(kvm, &u.ps);
+ kvm_vm_ioctl_set_pit(kvm, &u.ps);
+ r = 0;
break;
}
case KVM_GET_PIT2: {
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_get_pit2()
2019-12-07 9:25 [PATCH 0/6] remove unnecessary return val of kvm pit linmiaohe
2019-12-07 9:25 ` [PATCH 1/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_get_pit() linmiaohe
2019-12-07 9:25 ` [PATCH 2/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_set_pit() linmiaohe
@ 2019-12-07 9:25 ` linmiaohe
2019-12-07 9:25 ` [PATCH 4/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_set_pit2() linmiaohe
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: linmiaohe @ 2019-12-07 9:25 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
The return val of kvm_vm_ioctl_get_pit2() is always equal to 0, which means
there is no way to failed with this function. So remove the return val as
it's unnecessary to check against it.
Also add BUILD_BUG_ON to guard against channels size changed unexpectly.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/x86.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5628dceb9fa5..637d5c6b92be 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4621,15 +4621,17 @@ static void kvm_vm_ioctl_set_pit(struct kvm *kvm, struct kvm_pit_state *ps)
mutex_unlock(&pit->pit_state.lock);
}
-static int kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
+static void kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
{
+ BUILD_BUG_ON(sizeof(ps->channels) !=
+ sizeof(kvm->arch.vpit->pit_state.channels));
+
mutex_lock(&kvm->arch.vpit->pit_state.lock);
memcpy(ps->channels, &kvm->arch.vpit->pit_state.channels,
sizeof(ps->channels));
ps->flags = kvm->arch.vpit->pit_state.flags;
mutex_unlock(&kvm->arch.vpit->pit_state.lock);
memset(&ps->reserved, 0, sizeof(ps->reserved));
- return 0;
}
static int kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
@@ -5003,9 +5005,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = -ENXIO;
if (!kvm->arch.vpit)
goto out;
- r = kvm_vm_ioctl_get_pit2(kvm, &u.ps2);
- if (r)
- goto out;
+ kvm_vm_ioctl_get_pit2(kvm, &u.ps2);
r = -EFAULT;
if (copy_to_user(argp, &u.ps2, sizeof(u.ps2)))
goto out;
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_set_pit2()
2019-12-07 9:25 [PATCH 0/6] remove unnecessary return val of kvm pit linmiaohe
` (2 preceding siblings ...)
2019-12-07 9:25 ` [PATCH 3/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_get_pit2() linmiaohe
@ 2019-12-07 9:25 ` linmiaohe
2019-12-07 9:25 ` [PATCH 5/6] KVM: x86: check kvm_pit outside kvm_vm_ioctl_reinject() linmiaohe
2019-12-07 9:25 ` [PATCH 6/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_reinject() linmiaohe
5 siblings, 0 replies; 8+ messages in thread
From: linmiaohe @ 2019-12-07 9:25 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
The return val of kvm_vm_ioctl_set_pit2() is always equal to 0, which means
there is no way to failed with this function. So remove the return val as
it's unnecessary to check against it.
Also add BUILD_BUG_ON to guard against channels size changed unexpectly.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/x86.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 637d5c6b92be..2d4e3a2dfec6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4634,13 +4634,15 @@ static void kvm_vm_ioctl_get_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
memset(&ps->reserved, 0, sizeof(ps->reserved));
}
-static int kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
+static void kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
{
int start = 0;
int i;
u32 prev_legacy, cur_legacy;
struct kvm_pit *pit = kvm->arch.vpit;
+ BUILD_BUG_ON(sizeof(ps->channels) != sizeof(pit->pit_state.channels));
+
mutex_lock(&pit->pit_state.lock);
prev_legacy = pit->pit_state.flags & KVM_PIT_FLAGS_HPET_LEGACY;
cur_legacy = ps->flags & KVM_PIT_FLAGS_HPET_LEGACY;
@@ -4653,7 +4655,6 @@ static int kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
kvm_pit_load_count(pit, i, pit->pit_state.channels[i].count,
start && i == 0);
mutex_unlock(&pit->pit_state.lock);
- return 0;
}
static int kvm_vm_ioctl_reinject(struct kvm *kvm,
@@ -5019,7 +5020,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = -ENXIO;
if (!kvm->arch.vpit)
goto out;
- r = kvm_vm_ioctl_set_pit2(kvm, &u.ps2);
+ kvm_vm_ioctl_set_pit2(kvm, &u.ps2);
+ r = 0;
break;
}
case KVM_REINJECT_CONTROL: {
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/6] KVM: x86: check kvm_pit outside kvm_vm_ioctl_reinject()
2019-12-07 9:25 [PATCH 0/6] remove unnecessary return val of kvm pit linmiaohe
` (3 preceding siblings ...)
2019-12-07 9:25 ` [PATCH 4/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_set_pit2() linmiaohe
@ 2019-12-07 9:25 ` linmiaohe
2020-01-15 17:53 ` Paolo Bonzini
2019-12-07 9:25 ` [PATCH 6/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_reinject() linmiaohe
5 siblings, 1 reply; 8+ messages in thread
From: linmiaohe @ 2019-12-07 9:25 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
check kvm_pit outside kvm_vm_ioctl_reinject() to keep codestyle consistent
with other kvm_pit func and prepare for futher cleanups.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/x86.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2d4e3a2dfec6..00b5d4ace383 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4662,9 +4662,6 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
{
struct kvm_pit *pit = kvm->arch.vpit;
- if (!pit)
- return -ENXIO;
-
/* pit->pit_state.lock was overloaded to prevent userspace from getting
* an inconsistent state after running multiple KVM_REINJECT_CONTROL
* ioctls in parallel. Use a separate lock if that ioctl isn't rare.
@@ -5029,6 +5026,9 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = -EFAULT;
if (copy_from_user(&control, argp, sizeof(control)))
goto out;
+ r = -ENXIO;
+ if (!kvm->arch.vpit)
+ goto out;
r = kvm_vm_ioctl_reinject(kvm, &control);
break;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 5/6] KVM: x86: check kvm_pit outside kvm_vm_ioctl_reinject()
2019-12-07 9:25 ` [PATCH 5/6] KVM: x86: check kvm_pit outside kvm_vm_ioctl_reinject() linmiaohe
@ 2020-01-15 17:53 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2020-01-15 17:53 UTC (permalink / raw)
To: linmiaohe, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: kvm, linux-kernel, x86
On 07/12/19 10:25, linmiaohe wrote:
> From: Miaohe Lin <linmiaohe@huawei.com>
>
> check kvm_pit outside kvm_vm_ioctl_reinject() to keep codestyle consistent
> with other kvm_pit func and prepare for futher cleanups.
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> arch/x86/kvm/x86.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2d4e3a2dfec6..00b5d4ace383 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4662,9 +4662,6 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
> {
> struct kvm_pit *pit = kvm->arch.vpit;
>
> - if (!pit)
> - return -ENXIO;
> -
> /* pit->pit_state.lock was overloaded to prevent userspace from getting
> * an inconsistent state after running multiple KVM_REINJECT_CONTROL
> * ioctls in parallel. Use a separate lock if that ioctl isn't rare.
> @@ -5029,6 +5026,9 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = -EFAULT;
> if (copy_from_user(&control, argp, sizeof(control)))
> goto out;
> + r = -ENXIO;
> + if (!kvm->arch.vpit)
> + goto out;
> r = kvm_vm_ioctl_reinject(kvm, &control);
> break;
> }
>
Applied this one, I don't think the others are a useful improvement.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 6/6] KVM: x86: remove always equal to 0 return val of kvm_vm_ioctl_reinject()
2019-12-07 9:25 [PATCH 0/6] remove unnecessary return val of kvm pit linmiaohe
` (4 preceding siblings ...)
2019-12-07 9:25 ` [PATCH 5/6] KVM: x86: check kvm_pit outside kvm_vm_ioctl_reinject() linmiaohe
@ 2019-12-07 9:25 ` linmiaohe
5 siblings, 0 replies; 8+ messages in thread
From: linmiaohe @ 2019-12-07 9:25 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
The return val of kvm_vm_ioctl_reinject() is always equal to 0, which means
there is no way to failed with this function. So remove the return val as
it's unnecessary to check against it.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
arch/x86/kvm/x86.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 00b5d4ace383..82b0403cb2c7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4657,7 +4657,7 @@ static void kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps)
mutex_unlock(&pit->pit_state.lock);
}
-static int kvm_vm_ioctl_reinject(struct kvm *kvm,
+static void kvm_vm_ioctl_reinject(struct kvm *kvm,
struct kvm_reinject_control *control)
{
struct kvm_pit *pit = kvm->arch.vpit;
@@ -4669,8 +4669,6 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
mutex_lock(&pit->pit_state.lock);
kvm_pit_set_reinject(pit, control->pit_reinject);
mutex_unlock(&pit->pit_state.lock);
-
- return 0;
}
/**
@@ -5029,7 +5027,8 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = -ENXIO;
if (!kvm->arch.vpit)
goto out;
- r = kvm_vm_ioctl_reinject(kvm, &control);
+ kvm_vm_ioctl_reinject(kvm, &control);
+ r = 0;
break;
}
case KVM_SET_BOOT_CPU_ID:
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread