linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: arm64: fix MPIDR to vcpu index cache to support vcpu change at runtime
@ 2023-11-28 13:54 l00313349
  2023-11-29 19:18 ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: l00313349 @ 2023-11-28 13:54 UTC (permalink / raw)
  To: linux-arm-kernel, kvmarm
  Cc: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
	catalin.marinas, will, zhengchuan, xiexiangyou

From: Ting Li <liting8@huawei.com>

Consider vcpu change at runtime, for example online_vcpus is 8,
but only start 2 vcpu threads, MPIDR to vcpu index 0 may be overwrited
and vm hangs. When start other vcpu threads, cache will be outdated
and always get mismatch.
To solve this problem, codes update as followed:
1.First time only vcpu 0 can init MPIDR data, and use flags to avoid
index overwrite.
2.When MPIDR cache mismatch, fallback to old iterative method, and reinit
MPIDR to vcpu index cache data.

Signed-off-by: Ting Li <liting8@huawei.com>
---
 arch/arm64/kvm/arm.c | 49 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e5f75f1f1085..a3c038ad2f41 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -584,14 +584,16 @@ static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
 static void kvm_init_mpidr_data(struct kvm *kvm)
 {
 	struct kvm_mpidr_data *data = NULL;
+	bool *index_inited_flags = NULL;
+	struct kvm_mpidr_data *data_to_del = NULL;
 	unsigned long c, mask, nr_entries;
 	u64 aff_set = 0, aff_clr = ~0UL;
 	struct kvm_vcpu *vcpu;

-	mutex_lock(&kvm->arch.config_lock);
+	if (atomic_read(&kvm->online_vcpus) == 1)
+		return;

-	if (kvm->arch.mpidr_data || atomic_read(&kvm->online_vcpus) == 1)
-		goto out;
+	mutex_lock(&kvm->arch.config_lock);

 	kvm_for_each_vcpu(c, vcpu, kvm) {
 		u64 aff = kvm_vcpu_get_mpidr_aff(vcpu);
@@ -618,16 +620,37 @@ static void kvm_init_mpidr_data(struct kvm *kvm)
 	if (!data)
 		goto out;

+	/*
+	 * Use index inited flags to avoid index overwrite.
+	 */
+	index_inited_flags = kcalloc(nr_entries, sizeof(bool), GFP_KERNEL);
+	if (!index_inited_flags) {
+		kfree(data);
+		goto out;
+	}
+
 	data->mpidr_mask = mask;

 	kvm_for_each_vcpu(c, vcpu, kvm) {
 		u64 aff = kvm_vcpu_get_mpidr_aff(vcpu);
 		u16 index = kvm_mpidr_index(data, aff);

+		/*
+		 * When index has inited, ignore set conflict index.
+		 */
+		if (index_inited_flags[index])
+			continue;
 		data->cmpidr_to_idx[index] = c;
+		index_inited_flags[index] = true;
 	}
-
+	/*
+	 * When mpidr_data exist, change to new data then free old data.
+	 */
+	if (kvm->arch.mpidr_data)
+		data_to_del = kvm->arch.mpidr_data;
 	kvm->arch.mpidr_data = data;
+	kfree(data_to_del);
+	kfree(index_inited_flags);
 out:
 	mutex_unlock(&kvm->arch.config_lock);
 }
@@ -655,7 +678,8 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 	if (likely(vcpu_has_run_once(vcpu)))
 		return 0;

-	kvm_init_mpidr_data(kvm);
+	if (vcpu->vcpu_id == 0)
+		kvm_init_mpidr_data(kvm);

 	kvm_arm_vcpu_init_debug(vcpu);

@@ -2459,15 +2483,24 @@ struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)

 		vcpu = kvm_get_vcpu(kvm,
 				    kvm->arch.mpidr_data->cmpidr_to_idx[idx]);
+		/*
+		 * When mpidr mismatch, fallback to old iterative method.
+		 */
 		if (mpidr != kvm_vcpu_get_mpidr_aff(vcpu))
 			vcpu = NULL;
-
-		return vcpu;
+		else
+			return vcpu;
 	}

 	kvm_for_each_vcpu(i, vcpu, kvm) {
-		if (mpidr == kvm_vcpu_get_mpidr_aff(vcpu))
+		if (mpidr == kvm_vcpu_get_mpidr_aff(vcpu)) {
+			/*
+			 * Reinit MPIDR to vcpu index cache.
+			 */
+			if (kvm->arch.mpidr_data)
+				kvm_init_mpidr_data(kvm);
 			return vcpu;
+		}
 	}
 	return NULL;
 }
--
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: fix MPIDR to vcpu index cache to support vcpu change at runtime
  2023-11-28 13:54 [PATCH] KVM: arm64: fix MPIDR to vcpu index cache to support vcpu change at runtime l00313349
@ 2023-11-29 19:18 ` Marc Zyngier
  2023-12-01 10:48   ` liting (Q)
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2023-11-29 19:18 UTC (permalink / raw)
  To: l00313349
  Cc: linux-arm-kernel, kvmarm, oliver.upton, james.morse,
	suzuki.poulose, yuzenghui, catalin.marinas, will, zhengchuan,
	xiexiangyou

On Tue, 28 Nov 2023 13:54:02 +0000,
l00313349 <liting8@huawei.com> wrote:
> 
> From: Ting Li <liting8@huawei.com>
> 
> Consider vcpu change at runtime, for example online_vcpus is 8,
> but only start 2 vcpu threads, MPIDR to vcpu index 0 may be overwrited
> and vm hangs. When start other vcpu threads, cache will be outdated
> and always get mismatch.

If userspace creates two vcpus with the same vcpu_id, and therefore
the same MPIDR value, that's userspace's problem. Don't do that.

Why should we do anything else?

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: fix MPIDR to vcpu index cache to support vcpu change at runtime
  2023-11-29 19:18 ` Marc Zyngier
@ 2023-12-01 10:48   ` liting (Q)
  2023-12-01 12:15     ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: liting (Q) @ 2023-12-01 10:48 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, kvmarm, oliver.upton, james.morse,
	suzuki.poulose, yuzenghui, catalin.marinas, will, zhengchuan,
	xiexiangyou, liting8

On 2023/11/30 3:18, Marc Zyngier wrote:
> On Tue, 28 Nov 2023 13:54:02 +0000,
> l00313349 <liting8@huawei.com> wrote:
>> From: Ting Li <liting8@huawei.com>
>>
>> Consider vcpu change at runtime, for example online_vcpus is 8,
>> but only start 2 vcpu threads, MPIDR to vcpu index 0 may be overwrited
>> and vm hangs. When start other vcpu threads, cache will be outdated
>> and always get mismatch.
> If userspace creates two vcpus with the same vcpu_id, and therefore
> the same MPIDR value, that's userspace's problem. Don't do that.
>
> Why should we do anything else?
>
> 	M.
Hi Marc,
Thanks for your reply. Maybe you have misunderstanding about it.
The problem is not two vcpus with the same vcpu_id, but different vcpu_id
have the same MPIDR value in some situation such as vcpu hotplug.
In my case, total vcpu count is 8 but only start 2 vcpu at the beginning,
then vcpu_id 2-7 will get the same MPIDR value 0 with vcpu_id 0, so
index 0 will be overwrited to vcpu_id 7.
Please check, thanks.
Ting Li

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: arm64: fix MPIDR to vcpu index cache to support vcpu change at runtime
  2023-12-01 10:48   ` liting (Q)
@ 2023-12-01 12:15     ` Marc Zyngier
  0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2023-12-01 12:15 UTC (permalink / raw)
  To: liting (Q)
  Cc: linux-arm-kernel, kvmarm, oliver.upton, james.morse,
	suzuki.poulose, yuzenghui, catalin.marinas, will, zhengchuan,
	xiexiangyou

Hi Ting,

On Fri, 01 Dec 2023 10:48:54 +0000,
"liting (Q)" <liting8@huawei.com> wrote:
> 
> On 2023/11/30 3:18, Marc Zyngier wrote:
> > On Tue, 28 Nov 2023 13:54:02 +0000,
> > l00313349 <liting8@huawei.com> wrote:
> >> From: Ting Li <liting8@huawei.com>
> >> 
> >> Consider vcpu change at runtime, for example online_vcpus is 8,
> >> but only start 2 vcpu threads, MPIDR to vcpu index 0 may be overwrited
> >> and vm hangs. When start other vcpu threads, cache will be outdated
> >> and always get mismatch.
> > If userspace creates two vcpus with the same vcpu_id, and therefore
> > the same MPIDR value, that's userspace's problem. Don't do that.
> > 
> > Why should we do anything else?
> > 
> > 	M.
> Hi Marc,
> Thanks for your reply. Maybe you have misunderstanding about it.
> The problem is not two vcpus with the same vcpu_id, but different vcpu_id
> have the same MPIDR value in some situation such as vcpu hotplug.

But that's *wrong*. The whole point of MPIDR is that it distinguishes
between CPUs. By definition, MPIDR *must* be unique in the system, and
if you have two vcpus with the same MPIDR, you have a broken VM.

> In my case, total vcpu count is 8 but only start 2 vcpu at the beginning,
> then vcpu_id 2-7 will get the same MPIDR value 0 with vcpu_id 0, so
> index 0 will be overwrited to vcpu_id 7.

And that's wrong as per the letter of the architecture. Here's what it
says: "In an implementation containing multiple PEs, each PE is
identified by a unique affinity value reported by MPIDR_EL1{Aff3,
Aff2, Aff1, Aff0}" (D12.4 Attributability).

You don't have a case here. You are creating a VM that is in complete
violation of the architecture. It is your choice to do so, but don't
expect things to work.

> Please check, thanks.

I did.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-12-01 12:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-28 13:54 [PATCH] KVM: arm64: fix MPIDR to vcpu index cache to support vcpu change at runtime l00313349
2023-11-29 19:18 ` Marc Zyngier
2023-12-01 10:48   ` liting (Q)
2023-12-01 12:15     ` Marc Zyngier

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).