All of lore.kernel.org
 help / color / mirror / Atom feed
From: Reiji Watanabe <reijiw@google.com>
To: Marc Zyngier <maz@kernel.org>, kvmarm@lists.linux.dev
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ricardo Koller <ricarkol@google.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>,
	Reiji Watanabe <reijiw@google.com>
Subject: [PATCH v2 4/8] KVM: arm64: PMU: Disallow userspace to set PMCR.N greater than the host value
Date: Mon, 16 Jan 2023 17:35:38 -0800	[thread overview]
Message-ID: <20230117013542.371944-5-reijiw@google.com> (raw)
In-Reply-To: <20230117013542.371944-1-reijiw@google.com>

Currently, KVM allows userspace to set PMCR_EL0 to any values
with KVM_SET_ONE_REG for a vCPU with PMUv3 configured.

Disallow userspace to set PMCR_EL0.N to a value that is greater
than the host value (KVM_SET_ONE_REG will fail), as KVM doesn't
support more event counters than the host HW implements.
Although this is an ABI change, this change only affects
userspace setting PMCR_EL0.N to a larger value than the host.
As accesses to unadvertised event counters indices is CONSTRAINED
UNPREDICTABLE behavior, and PMCR_EL0.N was reset to the host value
on every vCPU reset before this series, I can't think of any
use case where a user space would do that.

Also, ignore writes to read-only bits that are cleared on vCPU reset,
and RES{0,1} bits (including writable bits that KVM doesn't support
yet), as those bits shouldn't be modified (at least with
the current KVM).

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 67c1bd39b478..e4bff9621473 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -958,6 +958,43 @@ static bool access_pmuserenr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	return true;
 }
 
+static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
+		    u64 val)
+{
+	u64 host_pmcr, host_n, new_n, mutable_mask;
+
+	new_n = (val >> ARMV8_PMU_PMCR_N_SHIFT) & ARMV8_PMU_PMCR_N_MASK;
+
+	host_pmcr = read_sysreg(pmcr_el0);
+	host_n = (host_pmcr >> ARMV8_PMU_PMCR_N_SHIFT) & ARMV8_PMU_PMCR_N_MASK;
+
+	/* The vCPU can't have more counters than the host have. */
+	if (new_n > host_n)
+		return -EINVAL;
+
+	/*
+	 * Ignore writes to RES0 bits, read only bits that are cleared on
+	 * vCPU reset, and writable bits that KVM doesn't support yet.
+	 * (i.e. only PMCR.N and bits [7:0] are mutable from userspace)
+	 * The LP bit is RES0 when FEAT_PMUv3p5 is not supported on the vCPU.
+	 * But, we leave the bit as it is here, as the vCPU's PMUver might
+	 * be changed later (NOTE: the bit will be cleared on first vCPU run
+	 * if necessary).
+	 */
+	mutable_mask = (ARMV8_PMU_PMCR_MASK |
+			(ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT));
+	val &= mutable_mask;
+	val |= (__vcpu_sys_reg(vcpu, r->reg) & ~mutable_mask);
+
+	/* The LC bit is RES1 when AArch32 is not supported */
+	if (!kvm_supports_32bit_el0())
+		val |= ARMV8_PMU_PMCR_LC;
+
+	__vcpu_sys_reg(vcpu, r->reg) = val;
+
+	return 0;
+}
+
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
 #define DBG_BCR_BVR_WCR_WVR_EL1(n)					\
 	{ SYS_DESC(SYS_DBGBVRn_EL1(n)),					\
@@ -1718,7 +1755,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_SVCR), undef_access },
 
 	{ PMU_SYS_REG(SYS_PMCR_EL0), .access = access_pmcr,
-	  .reset = reset_pmcr, .reg = PMCR_EL0 },
+	  .reset = reset_pmcr, .reg = PMCR_EL0, .set_user = set_pmcr },
 	{ PMU_SYS_REG(SYS_PMCNTENSET_EL0),
 	  .access = access_pmcnten, .reg = PMCNTENSET_EL0 },
 	{ PMU_SYS_REG(SYS_PMCNTENCLR_EL0),
-- 
2.39.0.314.g84b9a713c41-goog


WARNING: multiple messages have this Message-ID (diff)
From: Reiji Watanabe <reijiw@google.com>
To: Marc Zyngier <maz@kernel.org>, kvmarm@lists.linux.dev
Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	 James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	 Zenghui Yu <yuzenghui@huawei.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	 Paolo Bonzini <pbonzini@redhat.com>,
	Ricardo Koller <ricarkol@google.com>,
	 Oliver Upton <oliver.upton@linux.dev>,
	Jing Zhang <jingzhangos@google.com>,
	 Raghavendra Rao Anata <rananta@google.com>,
	Reiji Watanabe <reijiw@google.com>
Subject: [PATCH v2 4/8] KVM: arm64: PMU: Disallow userspace to set PMCR.N greater than the host value
Date: Mon, 16 Jan 2023 17:35:38 -0800	[thread overview]
Message-ID: <20230117013542.371944-5-reijiw@google.com> (raw)
In-Reply-To: <20230117013542.371944-1-reijiw@google.com>

Currently, KVM allows userspace to set PMCR_EL0 to any values
with KVM_SET_ONE_REG for a vCPU with PMUv3 configured.

Disallow userspace to set PMCR_EL0.N to a value that is greater
than the host value (KVM_SET_ONE_REG will fail), as KVM doesn't
support more event counters than the host HW implements.
Although this is an ABI change, this change only affects
userspace setting PMCR_EL0.N to a larger value than the host.
As accesses to unadvertised event counters indices is CONSTRAINED
UNPREDICTABLE behavior, and PMCR_EL0.N was reset to the host value
on every vCPU reset before this series, I can't think of any
use case where a user space would do that.

Also, ignore writes to read-only bits that are cleared on vCPU reset,
and RES{0,1} bits (including writable bits that KVM doesn't support
yet), as those bits shouldn't be modified (at least with
the current KVM).

Signed-off-by: Reiji Watanabe <reijiw@google.com>
---
 arch/arm64/kvm/sys_regs.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 67c1bd39b478..e4bff9621473 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -958,6 +958,43 @@ static bool access_pmuserenr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	return true;
 }
 
+static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
+		    u64 val)
+{
+	u64 host_pmcr, host_n, new_n, mutable_mask;
+
+	new_n = (val >> ARMV8_PMU_PMCR_N_SHIFT) & ARMV8_PMU_PMCR_N_MASK;
+
+	host_pmcr = read_sysreg(pmcr_el0);
+	host_n = (host_pmcr >> ARMV8_PMU_PMCR_N_SHIFT) & ARMV8_PMU_PMCR_N_MASK;
+
+	/* The vCPU can't have more counters than the host have. */
+	if (new_n > host_n)
+		return -EINVAL;
+
+	/*
+	 * Ignore writes to RES0 bits, read only bits that are cleared on
+	 * vCPU reset, and writable bits that KVM doesn't support yet.
+	 * (i.e. only PMCR.N and bits [7:0] are mutable from userspace)
+	 * The LP bit is RES0 when FEAT_PMUv3p5 is not supported on the vCPU.
+	 * But, we leave the bit as it is here, as the vCPU's PMUver might
+	 * be changed later (NOTE: the bit will be cleared on first vCPU run
+	 * if necessary).
+	 */
+	mutable_mask = (ARMV8_PMU_PMCR_MASK |
+			(ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT));
+	val &= mutable_mask;
+	val |= (__vcpu_sys_reg(vcpu, r->reg) & ~mutable_mask);
+
+	/* The LC bit is RES1 when AArch32 is not supported */
+	if (!kvm_supports_32bit_el0())
+		val |= ARMV8_PMU_PMCR_LC;
+
+	__vcpu_sys_reg(vcpu, r->reg) = val;
+
+	return 0;
+}
+
 /* Silly macro to expand the DBG{BCR,BVR,WVR,WCR}n_EL1 registers in one go */
 #define DBG_BCR_BVR_WCR_WVR_EL1(n)					\
 	{ SYS_DESC(SYS_DBGBVRn_EL1(n)),					\
@@ -1718,7 +1755,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_SVCR), undef_access },
 
 	{ PMU_SYS_REG(SYS_PMCR_EL0), .access = access_pmcr,
-	  .reset = reset_pmcr, .reg = PMCR_EL0 },
+	  .reset = reset_pmcr, .reg = PMCR_EL0, .set_user = set_pmcr },
 	{ PMU_SYS_REG(SYS_PMCNTENSET_EL0),
 	  .access = access_pmcnten, .reg = PMCNTENSET_EL0 },
 	{ PMU_SYS_REG(SYS_PMCNTENCLR_EL0),
-- 
2.39.0.314.g84b9a713c41-goog


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

  parent reply	other threads:[~2023-01-17  1:38 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-17  1:35 [PATCH v2 0/8] KVM: arm64: PMU: Allow userspace to limit the number of PMCs on vCPU Reiji Watanabe
2023-01-17  1:35 ` Reiji Watanabe
2023-01-17  1:35 ` [PATCH v2 1/8] KVM: arm64: PMU: Have reset_pmu_reg() to clear a register Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-20 14:04   ` Marc Zyngier
2023-01-20 14:04     ` Marc Zyngier
2023-01-20 14:11     ` Marc Zyngier
2023-01-20 14:11       ` Marc Zyngier
2023-01-21  5:18       ` Reiji Watanabe
2023-01-21  5:18         ` Reiji Watanabe
2023-01-17  1:35 ` [PATCH v2 2/8] KVM: arm64: PMU: Use reset_pmu_reg() for PMUSERENR_EL0 and PMCCFILTR_EL0 Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-17  1:35 ` [PATCH v2 3/8] KVM: arm64: PMU: Preserve vCPU's PMCR_EL0.N value on vCPU reset Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-20  0:30   ` Oliver Upton
2023-01-20  0:30     ` Oliver Upton
2023-01-20 12:12     ` Marc Zyngier
2023-01-20 12:12       ` Marc Zyngier
2023-01-20 18:04       ` Oliver Upton
2023-01-20 18:04         ` Oliver Upton
2023-01-20 18:53         ` Reiji Watanabe
2023-01-20 18:53           ` Reiji Watanabe
2023-01-17  1:35 ` Reiji Watanabe [this message]
2023-01-17  1:35   ` [PATCH v2 4/8] KVM: arm64: PMU: Disallow userspace to set PMCR.N greater than the host value Reiji Watanabe
2023-01-20 14:18   ` Marc Zyngier
2023-01-20 14:18     ` Marc Zyngier
2023-01-17  1:35 ` [PATCH v2 5/8] tools: arm64: Import perf_event.h Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-17  1:35 ` [PATCH v2 6/8] KVM: selftests: aarch64: Introduce vpmu_counter_access test Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-17  1:35 ` [PATCH v2 7/8] KVM: selftests: aarch64: vPMU register test for implemented counters Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-18  7:47   ` Shaoqin Huang
2023-01-18  7:47     ` Shaoqin Huang
2023-01-19  3:02     ` Reiji Watanabe
2023-01-19  3:02       ` Reiji Watanabe
2023-01-17  1:35 ` [PATCH v2 8/8] KVM: selftests: aarch64: vPMU register test for unimplemented counters Reiji Watanabe
2023-01-17  1:35   ` Reiji Watanabe
2023-01-18  7:49   ` Shaoqin Huang
2023-01-18  7:49     ` Shaoqin Huang
2023-01-19  3:04     ` Reiji Watanabe
2023-01-19  3:04       ` Reiji Watanabe
2023-01-17  7:25 ` [PATCH v2 0/8] KVM: arm64: PMU: Allow userspace to limit the number of PMCs on vCPU Shaoqin Huang
2023-01-17  7:25   ` Shaoqin Huang
2023-01-18  5:53   ` Reiji Watanabe
2023-01-18  5:53     ` Reiji Watanabe

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=20230117013542.371944-5-reijiw@google.com \
    --to=reijiw@google.com \
    --cc=alexandru.elisei@arm.com \
    --cc=james.morse@arm.com \
    --cc=jingzhangos@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=rananta@google.com \
    --cc=ricarkol@google.com \
    --cc=suzuki.poulose@arm.com \
    --cc=yuzenghui@huawei.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.