* [PATCH 0/2] KVM: x86: vcpu_idx related cleanups
@ 2021-09-10 18:32 Sean Christopherson
2021-09-10 18:32 ` [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor Sean Christopherson
2021-09-10 18:32 ` [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array Sean Christopherson
0 siblings, 2 replies; 10+ messages in thread
From: Sean Christopherson @ 2021-09-10 18:32 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
Drop KVM's obsolete kvm_vcpu_get_idx() wrapper, and use vcpu_idx to
identify vCPU0 when updating guest time.
Sean Christopherson (2):
KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor
KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs
array
arch/x86/kvm/hyperv.c | 7 +++----
arch/x86/kvm/hyperv.h | 2 +-
arch/x86/kvm/x86.c | 2 +-
include/linux/kvm_host.h | 5 -----
4 files changed, 5 insertions(+), 11 deletions(-)
--
2.33.0.309.g3052b89438-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor
2021-09-10 18:32 [PATCH 0/2] KVM: x86: vcpu_idx related cleanups Sean Christopherson
@ 2021-09-10 18:32 ` Sean Christopherson
2021-09-12 10:49 ` Maxim Levitsky
2021-09-13 7:02 ` Vitaly Kuznetsov
2021-09-10 18:32 ` [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array Sean Christopherson
1 sibling, 2 replies; 10+ messages in thread
From: Sean Christopherson @ 2021-09-10 18:32 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
Read vcpu->vcpu_idx directly instead of bouncing through the one-line
wrapper, kvm_vcpu_get_idx(), and drop the wrapper. The wrapper is a
remnant of the original implementation and serves no purpose; remove it
before it gains more users.
Back when kvm_vcpu_get_idx() was added by commit 497d72d80a78 ("KVM: Add
kvm_vcpu_get_idx to get vcpu index in kvm->vcpus"), the implementation
was more than just a simple wrapper as vcpu->vcpu_idx did not exist and
retrieving the index meant walking over the vCPU array to find the given
vCPU.
When vcpu_idx was introduced by commit 8750e72a79dd ("KVM: remember
position in kvm->vcpus array"), the helper was left behind, likely to
avoid extra thrash (but even then there were only two users, the original
arm usage having been removed at some point in the past).
No functional change intended.
Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/hyperv.c | 7 +++----
arch/x86/kvm/hyperv.h | 2 +-
include/linux/kvm_host.h | 5 -----
3 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index fe4a02715266..04dbc001f4fc 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -939,7 +939,7 @@ static int kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++)
stimer_init(&hv_vcpu->stimer[i], i);
- hv_vcpu->vp_index = kvm_vcpu_get_idx(vcpu);
+ hv_vcpu->vp_index = vcpu->vcpu_idx;
return 0;
}
@@ -1444,7 +1444,6 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
switch (msr) {
case HV_X64_MSR_VP_INDEX: {
struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
- int vcpu_idx = kvm_vcpu_get_idx(vcpu);
u32 new_vp_index = (u32)data;
if (!host || new_vp_index >= KVM_MAX_VCPUS)
@@ -1459,9 +1458,9 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
* VP index is changing, adjust num_mismatched_vp_indexes if
* it now matches or no longer matches vcpu_idx.
*/
- if (hv_vcpu->vp_index == vcpu_idx)
+ if (hv_vcpu->vp_index == vcpu->vcpu_idx)
atomic_inc(&hv->num_mismatched_vp_indexes);
- else if (new_vp_index == vcpu_idx)
+ else if (new_vp_index == vcpu->vcpu_idx)
atomic_dec(&hv->num_mismatched_vp_indexes);
hv_vcpu->vp_index = new_vp_index;
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 730da8537d05..ed1c4e546d04 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -83,7 +83,7 @@ static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu)
{
struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
- return hv_vcpu ? hv_vcpu->vp_index : kvm_vcpu_get_idx(vcpu);
+ return hv_vcpu ? hv_vcpu->vp_index : vcpu->vcpu_idx;
}
int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e4d712e9f760..31071ad821e2 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -721,11 +721,6 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id)
return NULL;
}
-static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)
-{
- return vcpu->vcpu_idx;
-}
-
#define kvm_for_each_memslot(memslot, slots) \
for (memslot = &slots->memslots[0]; \
memslot < slots->memslots + slots->used_slots; memslot++) \
--
2.33.0.309.g3052b89438-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array
2021-09-10 18:32 [PATCH 0/2] KVM: x86: vcpu_idx related cleanups Sean Christopherson
2021-09-10 18:32 ` [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor Sean Christopherson
@ 2021-09-10 18:32 ` Sean Christopherson
2021-09-10 21:46 ` Jim Mattson
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Sean Christopherson @ 2021-09-10 18:32 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
Joerg Roedel, kvm, linux-kernel
Use vcpu_idx to identify vCPU0 when updating HyperV's TSC page, which is
shared by all vCPUs and "owned" by vCPU0 (because vCPU0 is the only vCPU
that's guaranteed to exist). Using kvm_get_vcpu() to find vCPU works,
but it's a rather odd and suboptimal method to check the index of a given
vCPU.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 86539c1686fa..6ab851df08d1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2969,7 +2969,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
offsetof(struct compat_vcpu_info, time));
if (vcpu->xen.vcpu_time_info_set)
kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_time_info_cache, 0);
- if (v == kvm_get_vcpu(v->kvm, 0))
+ if (!v->vcpu_idx)
kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
return 0;
}
--
2.33.0.309.g3052b89438-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array
2021-09-10 18:32 ` [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array Sean Christopherson
@ 2021-09-10 21:46 ` Jim Mattson
2021-09-12 10:52 ` Maxim Levitsky
2021-09-13 7:07 ` Vitaly Kuznetsov
2 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2021-09-10 21:46 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, kvm,
linux-kernel
On Fri, Sep 10, 2021 at 11:32 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Use vcpu_idx to identify vCPU0 when updating HyperV's TSC page, which is
> shared by all vCPUs and "owned" by vCPU0 (because vCPU0 is the only vCPU
> that's guaranteed to exist). Using kvm_get_vcpu() to find vCPU works,
> but it's a rather odd and suboptimal method to check the index of a given
> vCPU.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor
2021-09-10 18:32 ` [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor Sean Christopherson
@ 2021-09-12 10:49 ` Maxim Levitsky
2021-09-13 7:02 ` Vitaly Kuznetsov
1 sibling, 0 replies; 10+ messages in thread
From: Maxim Levitsky @ 2021-09-12 10:49 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
On Fri, 2021-09-10 at 11:32 -0700, Sean Christopherson wrote:
> Read vcpu->vcpu_idx directly instead of bouncing through the one-line
> wrapper, kvm_vcpu_get_idx(), and drop the wrapper. The wrapper is a
> remnant of the original implementation and serves no purpose; remove it
> before it gains more users.
>
> Back when kvm_vcpu_get_idx() was added by commit 497d72d80a78 ("KVM: Add
> kvm_vcpu_get_idx to get vcpu index in kvm->vcpus"), the implementation
> was more than just a simple wrapper as vcpu->vcpu_idx did not exist and
> retrieving the index meant walking over the vCPU array to find the given
> vCPU.
>
> When vcpu_idx was introduced by commit 8750e72a79dd ("KVM: remember
> position in kvm->vcpus array"), the helper was left behind, likely to
> avoid extra thrash (but even then there were only two users, the original
> arm usage having been removed at some point in the past).
>
> No functional change intended.
>
> Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/hyperv.c | 7 +++----
> arch/x86/kvm/hyperv.h | 2 +-
> include/linux/kvm_host.h | 5 -----
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index fe4a02715266..04dbc001f4fc 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -939,7 +939,7 @@ static int kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
> for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++)
> stimer_init(&hv_vcpu->stimer[i], i);
>
> - hv_vcpu->vp_index = kvm_vcpu_get_idx(vcpu);
> + hv_vcpu->vp_index = vcpu->vcpu_idx;
>
> return 0;
> }
> @@ -1444,7 +1444,6 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
> switch (msr) {
> case HV_X64_MSR_VP_INDEX: {
> struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
> - int vcpu_idx = kvm_vcpu_get_idx(vcpu);
> u32 new_vp_index = (u32)data;
>
> if (!host || new_vp_index >= KVM_MAX_VCPUS)
> @@ -1459,9 +1458,9 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
> * VP index is changing, adjust num_mismatched_vp_indexes if
> * it now matches or no longer matches vcpu_idx.
> */
> - if (hv_vcpu->vp_index == vcpu_idx)
> + if (hv_vcpu->vp_index == vcpu->vcpu_idx)
> atomic_inc(&hv->num_mismatched_vp_indexes);
> - else if (new_vp_index == vcpu_idx)
> + else if (new_vp_index == vcpu->vcpu_idx)
> atomic_dec(&hv->num_mismatched_vp_indexes);
>
> hv_vcpu->vp_index = new_vp_index;
> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index 730da8537d05..ed1c4e546d04 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -83,7 +83,7 @@ static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu)
> {
> struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
>
> - return hv_vcpu ? hv_vcpu->vp_index : kvm_vcpu_get_idx(vcpu);
> + return hv_vcpu ? hv_vcpu->vp_index : vcpu->vcpu_idx;
> }
>
> int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index e4d712e9f760..31071ad821e2 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -721,11 +721,6 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id)
> return NULL;
> }
>
> -static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)
> -{
> - return vcpu->vcpu_idx;
> -}
> -
> #define kvm_for_each_memslot(memslot, slots) \
> for (memslot = &slots->memslots[0]; \
> memslot < slots->memslots + slots->used_slots; memslot++) \
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array
2021-09-10 18:32 ` [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array Sean Christopherson
2021-09-10 21:46 ` Jim Mattson
@ 2021-09-12 10:52 ` Maxim Levitsky
2021-09-13 7:07 ` Vitaly Kuznetsov
2 siblings, 0 replies; 10+ messages in thread
From: Maxim Levitsky @ 2021-09-12 10:52 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
On Fri, 2021-09-10 at 11:32 -0700, Sean Christopherson wrote:
> Use vcpu_idx to identify vCPU0 when updating HyperV's TSC page, which is
> shared by all vCPUs and "owned" by vCPU0 (because vCPU0 is the only vCPU
> that's guaranteed to exist). Using kvm_get_vcpu() to find vCPU works,
> but it's a rather odd and suboptimal method to check the index of a given
> vCPU.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/x86.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 86539c1686fa..6ab851df08d1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2969,7 +2969,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> offsetof(struct compat_vcpu_info, time));
> if (vcpu->xen.vcpu_time_info_set)
> kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_time_info_cache, 0);
> - if (v == kvm_get_vcpu(v->kvm, 0))
> + if (!v->vcpu_idx)
> kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
> return 0;
> }
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor
2021-09-10 18:32 ` [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor Sean Christopherson
2021-09-12 10:49 ` Maxim Levitsky
@ 2021-09-13 7:02 ` Vitaly Kuznetsov
1 sibling, 0 replies; 10+ messages in thread
From: Vitaly Kuznetsov @ 2021-09-13 7:02 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Sean Christopherson, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
Sean Christopherson <seanjc@google.com> writes:
> Read vcpu->vcpu_idx directly instead of bouncing through the one-line
> wrapper, kvm_vcpu_get_idx(), and drop the wrapper. The wrapper is a
> remnant of the original implementation and serves no purpose; remove it
> before it gains more users.
>
> Back when kvm_vcpu_get_idx() was added by commit 497d72d80a78 ("KVM: Add
> kvm_vcpu_get_idx to get vcpu index in kvm->vcpus"), the implementation
> was more than just a simple wrapper as vcpu->vcpu_idx did not exist and
> retrieving the index meant walking over the vCPU array to find the given
> vCPU.
>
> When vcpu_idx was introduced by commit 8750e72a79dd ("KVM: remember
> position in kvm->vcpus array"), the helper was left behind, likely to
> avoid extra thrash (but even then there were only two users, the original
> arm usage having been removed at some point in the past).
>
> No functional change intended.
>
> Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/hyperv.c | 7 +++----
> arch/x86/kvm/hyperv.h | 2 +-
> include/linux/kvm_host.h | 5 -----
> 3 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index fe4a02715266..04dbc001f4fc 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -939,7 +939,7 @@ static int kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
> for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++)
> stimer_init(&hv_vcpu->stimer[i], i);
>
> - hv_vcpu->vp_index = kvm_vcpu_get_idx(vcpu);
> + hv_vcpu->vp_index = vcpu->vcpu_idx;
>
> return 0;
> }
> @@ -1444,7 +1444,6 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
> switch (msr) {
> case HV_X64_MSR_VP_INDEX: {
> struct kvm_hv *hv = to_kvm_hv(vcpu->kvm);
> - int vcpu_idx = kvm_vcpu_get_idx(vcpu);
> u32 new_vp_index = (u32)data;
>
> if (!host || new_vp_index >= KVM_MAX_VCPUS)
> @@ -1459,9 +1458,9 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
> * VP index is changing, adjust num_mismatched_vp_indexes if
> * it now matches or no longer matches vcpu_idx.
> */
> - if (hv_vcpu->vp_index == vcpu_idx)
> + if (hv_vcpu->vp_index == vcpu->vcpu_idx)
> atomic_inc(&hv->num_mismatched_vp_indexes);
> - else if (new_vp_index == vcpu_idx)
> + else if (new_vp_index == vcpu->vcpu_idx)
> atomic_dec(&hv->num_mismatched_vp_indexes);
>
> hv_vcpu->vp_index = new_vp_index;
> diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> index 730da8537d05..ed1c4e546d04 100644
> --- a/arch/x86/kvm/hyperv.h
> +++ b/arch/x86/kvm/hyperv.h
> @@ -83,7 +83,7 @@ static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu)
> {
> struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
>
> - return hv_vcpu ? hv_vcpu->vp_index : kvm_vcpu_get_idx(vcpu);
> + return hv_vcpu ? hv_vcpu->vp_index : vcpu->vcpu_idx;
> }
>
> int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index e4d712e9f760..31071ad821e2 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -721,11 +721,6 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id)
> return NULL;
> }
>
> -static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)
> -{
> - return vcpu->vcpu_idx;
> -}
> -
> #define kvm_for_each_memslot(memslot, slots) \
> for (memslot = &slots->memslots[0]; \
> memslot < slots->memslots + slots->used_slots; memslot++) \
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array
2021-09-10 18:32 ` [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array Sean Christopherson
2021-09-10 21:46 ` Jim Mattson
2021-09-12 10:52 ` Maxim Levitsky
@ 2021-09-13 7:07 ` Vitaly Kuznetsov
2021-09-20 14:57 ` Sean Christopherson
2 siblings, 1 reply; 10+ messages in thread
From: Vitaly Kuznetsov @ 2021-09-13 7:07 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
Sean Christopherson <seanjc@google.com> writes:
> Use vcpu_idx to identify vCPU0 when updating HyperV's TSC page, which is
> shared by all vCPUs and "owned" by vCPU0 (because vCPU0 is the only vCPU
> that's guaranteed to exist). Using kvm_get_vcpu() to find vCPU works,
> but it's a rather odd and suboptimal method to check the index of a given
> vCPU.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/x86.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 86539c1686fa..6ab851df08d1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2969,7 +2969,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> offsetof(struct compat_vcpu_info, time));
> if (vcpu->xen.vcpu_time_info_set)
> kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_time_info_cache, 0);
> - if (v == kvm_get_vcpu(v->kvm, 0))
> + if (!v->vcpu_idx)
> kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
> return 0;
> }
" ... instead of walking vCPUs array" in the Subject line is a bit
confusing because kvm_get_vcpu() doesn't actually walk anything, it just
returns 'kvm->vcpus[i]' after checking that we actually have that many
vCPUs. The patch itself is OK, so
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array
2021-09-13 7:07 ` Vitaly Kuznetsov
@ 2021-09-20 14:57 ` Sean Christopherson
2021-09-22 7:41 ` Paolo Bonzini
0 siblings, 1 reply; 10+ messages in thread
From: Sean Christopherson @ 2021-09-20 14:57 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Paolo Bonzini, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
On Mon, Sep 13, 2021, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
>
> > Use vcpu_idx to identify vCPU0 when updating HyperV's TSC page, which is
> > shared by all vCPUs and "owned" by vCPU0 (because vCPU0 is the only vCPU
> > that's guaranteed to exist). Using kvm_get_vcpu() to find vCPU works,
> > but it's a rather odd and suboptimal method to check the index of a given
> > vCPU.
> >
> > No functional change intended.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kvm/x86.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 86539c1686fa..6ab851df08d1 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2969,7 +2969,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> > offsetof(struct compat_vcpu_info, time));
> > if (vcpu->xen.vcpu_time_info_set)
> > kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_time_info_cache, 0);
> > - if (v == kvm_get_vcpu(v->kvm, 0))
> > + if (!v->vcpu_idx)
> > kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
> > return 0;
> > }
>
> " ... instead of walking vCPUs array" in the Subject line is a bit
> confusing because kvm_get_vcpu() doesn't actually walk anything, it just
> returns 'kvm->vcpus[i]' after checking that we actually have that many
> vCPUs. The patch itself is OK, so
Argh, yes, I have a feeling I wrote the changelog after digging into the history
of kvm_get_vcpu().
Paolo, can you tweak the shortlog to:
KVM: x86: Identify vCPU0 by its vcpu_idx instead of its vCPUs array entry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array
2021-09-20 14:57 ` Sean Christopherson
@ 2021-09-22 7:41 ` Paolo Bonzini
0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2021-09-22 7:41 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov
Cc: Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel
On 20/09/21 16:57, Sean Christopherson wrote:
> On Mon, Sep 13, 2021, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>>
>>> Use vcpu_idx to identify vCPU0 when updating HyperV's TSC page, which is
>>> shared by all vCPUs and "owned" by vCPU0 (because vCPU0 is the only vCPU
>>> that's guaranteed to exist). Using kvm_get_vcpu() to find vCPU works,
>>> but it's a rather odd and suboptimal method to check the index of a given
>>> vCPU.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>>> ---
>>> arch/x86/kvm/x86.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 86539c1686fa..6ab851df08d1 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -2969,7 +2969,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
>>> offsetof(struct compat_vcpu_info, time));
>>> if (vcpu->xen.vcpu_time_info_set)
>>> kvm_setup_pvclock_page(v, &vcpu->xen.vcpu_time_info_cache, 0);
>>> - if (v == kvm_get_vcpu(v->kvm, 0))
>>> + if (!v->vcpu_idx)
>>> kvm_hv_setup_tsc_page(v->kvm, &vcpu->hv_clock);
>>> return 0;
>>> }
>>
>> " ... instead of walking vCPUs array" in the Subject line is a bit
>> confusing because kvm_get_vcpu() doesn't actually walk anything, it just
>> returns 'kvm->vcpus[i]' after checking that we actually have that many
>> vCPUs. The patch itself is OK, so
>
> Argh, yes, I have a feeling I wrote the changelog after digging into the history
> of kvm_get_vcpu().
>
> Paolo, can you tweak the shortlog to:
>
> KVM: x86: Identify vCPU0 by its vcpu_idx instead of its vCPUs array entry
>
Done and queued. Patch 1 required some further s390 changes.
Paolo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-22 7:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10 18:32 [PATCH 0/2] KVM: x86: vcpu_idx related cleanups Sean Christopherson
2021-09-10 18:32 ` [PATCH 1/2] KVM: x86: Query vcpu->vcpu_idx directly and drop its accessor Sean Christopherson
2021-09-12 10:49 ` Maxim Levitsky
2021-09-13 7:02 ` Vitaly Kuznetsov
2021-09-10 18:32 ` [PATCH 2/2] KVM: x86: Identify vCPU0 by its vcpu_idx instead of walking vCPUs array Sean Christopherson
2021-09-10 21:46 ` Jim Mattson
2021-09-12 10:52 ` Maxim Levitsky
2021-09-13 7:07 ` Vitaly Kuznetsov
2021-09-20 14:57 ` Sean Christopherson
2021-09-22 7:41 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).