All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	David Brazdil <dbrazdil@google.com>,
	Eric Auger <eric.auger@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Qian Cai <qcai@redhat.com>,
	Shannon Zhao <shannon.zhao@linux.alibaba.com>,
	James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	kernel-team@android.com, kvmarm@lists.cs.columbia.edu,
	kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: [PATCH 11/18] KVM: arm64: Move double-checked lock to kvm_vgic_map_resources()
Date: Thu,  7 Jan 2021 11:20:54 +0000	[thread overview]
Message-ID: <20210107112101.2297944-12-maz@kernel.org> (raw)
In-Reply-To: <20210107112101.2297944-1-maz@kernel.org>

From: Alexandru Elisei <alexandru.elisei@arm.com>

kvm_vgic_map_resources() is called when a VCPU if first run and it maps all
the VGIC MMIO regions. To prevent double-initialization, the VGIC uses the
ready variable to keep track of the state of resources and the global KVM
mutex to protect against concurrent accesses. After the lock is taken, the
variable is checked again in case another VCPU took the lock between the
current VCPU reading ready equals false and taking the lock.

The double-checked lock pattern is spread across four different functions:
in kvm_vcpu_first_run_init(), in kvm_vgic_map_resource() and in
vgic_{v2,v3}_map_resources(), which makes it hard to reason about and
introduces minor code duplication. Consolidate the checks in
kvm_vgic_map_resources(), where the lock is taken.

No functional change intended.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201201150157.223625-4-alexandru.elisei@arm.com
---
 arch/arm64/kvm/arm.c            | 8 +++-----
 arch/arm64/kvm/vgic/vgic-init.c | 6 ++++++
 arch/arm64/kvm/vgic/vgic-v2.c   | 3 ---
 arch/arm64/kvm/vgic/vgic-v3.c   | 3 ---
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e207e4541f55..ab782c480e9a 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -580,11 +580,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 		 * Map the VGIC hardware resources before running a vcpu the
 		 * first time on this VM.
 		 */
-		if (unlikely(!vgic_ready(kvm))) {
-			ret = kvm_vgic_map_resources(kvm);
-			if (ret)
-				return ret;
-		}
+		ret = kvm_vgic_map_resources(kvm);
+		if (ret)
+			return ret;
 	} else {
 		/*
 		 * Tell the rest of the code that there are userspace irqchip
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 32e32d67a127..a2f4d1c85f00 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -428,7 +428,13 @@ int kvm_vgic_map_resources(struct kvm *kvm)
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
+	if (likely(vgic_ready(kvm)))
+		return 0;
+
 	mutex_lock(&kvm->lock);
+	if (vgic_ready(kvm))
+		goto out;
+
 	if (!irqchip_in_kernel(kvm))
 		goto out;
 
diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index ebf53a4e1296..7f38c1a93639 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -306,9 +306,6 @@ int vgic_v2_map_resources(struct kvm *kvm)
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
-	if (vgic_ready(kvm))
-		goto out;
-
 	if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
 	    IS_VGIC_ADDR_UNDEF(dist->vgic_cpu_base)) {
 		kvm_err("Need to set vgic cpu and dist addresses first\n");
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9cdf39a94a63..35029c5cb0f1 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -500,9 +500,6 @@ int vgic_v3_map_resources(struct kvm *kvm)
 	int ret = 0;
 	int c;
 
-	if (vgic_ready(kvm))
-		goto out;
-
 	kvm_for_each_vcpu(c, vcpu, kvm) {
 		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
-- 
2.29.2


WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kernel-team@android.com, kvm@vger.kernel.org,
	Qian Cai <qcai@redhat.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Shannon Zhao <shannon.zhao@linux.alibaba.com>,
	linux-arm-kernel@lists.infradead.org,
	Nathan Chancellor <natechancellor@gmail.com>,
	kvmarm@lists.cs.columbia.edu
Subject: [PATCH 11/18] KVM: arm64: Move double-checked lock to kvm_vgic_map_resources()
Date: Thu,  7 Jan 2021 11:20:54 +0000	[thread overview]
Message-ID: <20210107112101.2297944-12-maz@kernel.org> (raw)
In-Reply-To: <20210107112101.2297944-1-maz@kernel.org>

From: Alexandru Elisei <alexandru.elisei@arm.com>

kvm_vgic_map_resources() is called when a VCPU if first run and it maps all
the VGIC MMIO regions. To prevent double-initialization, the VGIC uses the
ready variable to keep track of the state of resources and the global KVM
mutex to protect against concurrent accesses. After the lock is taken, the
variable is checked again in case another VCPU took the lock between the
current VCPU reading ready equals false and taking the lock.

The double-checked lock pattern is spread across four different functions:
in kvm_vcpu_first_run_init(), in kvm_vgic_map_resource() and in
vgic_{v2,v3}_map_resources(), which makes it hard to reason about and
introduces minor code duplication. Consolidate the checks in
kvm_vgic_map_resources(), where the lock is taken.

No functional change intended.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201201150157.223625-4-alexandru.elisei@arm.com
---
 arch/arm64/kvm/arm.c            | 8 +++-----
 arch/arm64/kvm/vgic/vgic-init.c | 6 ++++++
 arch/arm64/kvm/vgic/vgic-v2.c   | 3 ---
 arch/arm64/kvm/vgic/vgic-v3.c   | 3 ---
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e207e4541f55..ab782c480e9a 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -580,11 +580,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 		 * Map the VGIC hardware resources before running a vcpu the
 		 * first time on this VM.
 		 */
-		if (unlikely(!vgic_ready(kvm))) {
-			ret = kvm_vgic_map_resources(kvm);
-			if (ret)
-				return ret;
-		}
+		ret = kvm_vgic_map_resources(kvm);
+		if (ret)
+			return ret;
 	} else {
 		/*
 		 * Tell the rest of the code that there are userspace irqchip
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 32e32d67a127..a2f4d1c85f00 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -428,7 +428,13 @@ int kvm_vgic_map_resources(struct kvm *kvm)
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
+	if (likely(vgic_ready(kvm)))
+		return 0;
+
 	mutex_lock(&kvm->lock);
+	if (vgic_ready(kvm))
+		goto out;
+
 	if (!irqchip_in_kernel(kvm))
 		goto out;
 
diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index ebf53a4e1296..7f38c1a93639 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -306,9 +306,6 @@ int vgic_v2_map_resources(struct kvm *kvm)
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
-	if (vgic_ready(kvm))
-		goto out;
-
 	if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
 	    IS_VGIC_ADDR_UNDEF(dist->vgic_cpu_base)) {
 		kvm_err("Need to set vgic cpu and dist addresses first\n");
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9cdf39a94a63..35029c5cb0f1 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -500,9 +500,6 @@ int vgic_v3_map_resources(struct kvm *kvm)
 	int ret = 0;
 	int c;
 
-	if (vgic_ready(kvm))
-		goto out;
-
 	kvm_for_each_vcpu(c, vcpu, kvm) {
 		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
-- 
2.29.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	kernel-team@android.com, kvm@vger.kernel.org,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Qian Cai <qcai@redhat.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Shannon Zhao <shannon.zhao@linux.alibaba.com>,
	Eric Auger <eric.auger@redhat.com>,
	James Morse <james.morse@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	Nathan Chancellor <natechancellor@gmail.com>,
	David Brazdil <dbrazdil@google.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	kvmarm@lists.cs.columbia.edu,
	Julien Thierry <julien.thierry.kdev@gmail.com>
Subject: [PATCH 11/18] KVM: arm64: Move double-checked lock to kvm_vgic_map_resources()
Date: Thu,  7 Jan 2021 11:20:54 +0000	[thread overview]
Message-ID: <20210107112101.2297944-12-maz@kernel.org> (raw)
In-Reply-To: <20210107112101.2297944-1-maz@kernel.org>

From: Alexandru Elisei <alexandru.elisei@arm.com>

kvm_vgic_map_resources() is called when a VCPU if first run and it maps all
the VGIC MMIO regions. To prevent double-initialization, the VGIC uses the
ready variable to keep track of the state of resources and the global KVM
mutex to protect against concurrent accesses. After the lock is taken, the
variable is checked again in case another VCPU took the lock between the
current VCPU reading ready equals false and taking the lock.

The double-checked lock pattern is spread across four different functions:
in kvm_vcpu_first_run_init(), in kvm_vgic_map_resource() and in
vgic_{v2,v3}_map_resources(), which makes it hard to reason about and
introduces minor code duplication. Consolidate the checks in
kvm_vgic_map_resources(), where the lock is taken.

No functional change intended.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20201201150157.223625-4-alexandru.elisei@arm.com
---
 arch/arm64/kvm/arm.c            | 8 +++-----
 arch/arm64/kvm/vgic/vgic-init.c | 6 ++++++
 arch/arm64/kvm/vgic/vgic-v2.c   | 3 ---
 arch/arm64/kvm/vgic/vgic-v3.c   | 3 ---
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e207e4541f55..ab782c480e9a 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -580,11 +580,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 		 * Map the VGIC hardware resources before running a vcpu the
 		 * first time on this VM.
 		 */
-		if (unlikely(!vgic_ready(kvm))) {
-			ret = kvm_vgic_map_resources(kvm);
-			if (ret)
-				return ret;
-		}
+		ret = kvm_vgic_map_resources(kvm);
+		if (ret)
+			return ret;
 	} else {
 		/*
 		 * Tell the rest of the code that there are userspace irqchip
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index 32e32d67a127..a2f4d1c85f00 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -428,7 +428,13 @@ int kvm_vgic_map_resources(struct kvm *kvm)
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
+	if (likely(vgic_ready(kvm)))
+		return 0;
+
 	mutex_lock(&kvm->lock);
+	if (vgic_ready(kvm))
+		goto out;
+
 	if (!irqchip_in_kernel(kvm))
 		goto out;
 
diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index ebf53a4e1296..7f38c1a93639 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -306,9 +306,6 @@ int vgic_v2_map_resources(struct kvm *kvm)
 	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
-	if (vgic_ready(kvm))
-		goto out;
-
 	if (IS_VGIC_ADDR_UNDEF(dist->vgic_dist_base) ||
 	    IS_VGIC_ADDR_UNDEF(dist->vgic_cpu_base)) {
 		kvm_err("Need to set vgic cpu and dist addresses first\n");
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9cdf39a94a63..35029c5cb0f1 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -500,9 +500,6 @@ int vgic_v3_map_resources(struct kvm *kvm)
 	int ret = 0;
 	int c;
 
-	if (vgic_ready(kvm))
-		goto out;
-
 	kvm_for_each_vcpu(c, vcpu, kvm) {
 		struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
-- 
2.29.2


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

  parent reply	other threads:[~2021-01-07 11:51 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-07 11:20 [GIT PULL] KVM/arm64 fixes for 5.11, take #1 Marc Zyngier
2021-01-07 11:20 ` Marc Zyngier
2021-01-07 11:20 ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 01/18] KVM: arm64: Don't access PMCR_EL0 when no PMU is available Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 02/18] KVM: arm64: Prevent use of invalid PSCI v0.1 function IDs Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 03/18] KVM: arm64: Use lm_alias in nVHE-only VA conversion Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 04/18] KVM: arm64: Skip computing hyp VA layout for VHE Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 05/18] KVM: arm64: Minor cleanup of hyp variables used in host Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 06/18] KVM: arm64: Remove unused includes in psci-relay.c Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 07/18] KVM: arm64: Move skip_host_instruction to adjust_pc.h Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 08/18] KVM: arm64: Declutter host PSCI 0.1 handling Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 09/18] KVM: Documentation: Add arm64 KVM_RUN error codes Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 10/18] KVM: arm64: arch_timer: Remove VGIC initialization check Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` Marc Zyngier [this message]
2021-01-07 11:20   ` [PATCH 11/18] KVM: arm64: Move double-checked lock to kvm_vgic_map_resources() Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 12/18] KVM: arm64: Update comment in kvm_vgic_map_resources() Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 13/18] KVM: arm64: Remove redundant call to kvm_pmu_vcpu_reset() Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 14/18] KVM: arm64: Consolidate dist->ready setting into kvm_vgic_map_resources() Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 15/18] KVM: arm64: Fix hyp_cpu_pm_{init,exit} __init annotation Marc Zyngier
2021-01-07 11:20   ` [PATCH 15/18] KVM: arm64: Fix hyp_cpu_pm_{init, exit} " Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20 ` [PATCH 16/18] KVM: arm64: Remove spurious semicolon in reg_to_encoding() Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:20   ` Marc Zyngier
2021-01-07 11:21 ` [PATCH 17/18] KVM: arm64: Replace KVM_ARM_PMU with HW_PERF_EVENTS Marc Zyngier
2021-01-07 11:21   ` Marc Zyngier
2021-01-07 11:21   ` Marc Zyngier
2021-01-07 11:21 ` [PATCH 18/18] arm64: cpufeature: remove non-exist CONFIG_KVM_ARM_HOST Marc Zyngier
2021-01-07 11:21   ` Marc Zyngier
2021-01-07 11:21   ` Marc Zyngier
2021-01-07 18:19 ` [GIT PULL] KVM/arm64 fixes for 5.11, take #1 Paolo Bonzini
2021-01-07 18:19   ` Paolo Bonzini
2021-01-07 18:19   ` Paolo Bonzini
2021-01-07 23:09 ` Paolo Bonzini
2021-01-07 23:09   ` Paolo Bonzini
2021-01-07 23:09   ` Paolo Bonzini
2021-01-08  8:22   ` Marc Zyngier
2021-01-08  8:22     ` Marc Zyngier
2021-01-08  8:22     ` Marc Zyngier
2021-01-08 10:03     ` Paolo Bonzini
2021-01-08 10:03       ` Paolo Bonzini
2021-01-08 10:03       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210107112101.2297944-12-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=alexandru.elisei@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=dbrazdil@google.com \
    --cc=eric.auger@redhat.com \
    --cc=james.morse@arm.com \
    --cc=julien.thierry.kdev@gmail.com \
    --cc=kernel-team@android.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=natechancellor@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=qcai@redhat.com \
    --cc=shannon.zhao@linux.alibaba.com \
    --cc=suzuki.poulose@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.