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>, Shaoqin Huang <shahuang@redhat.com>, Reiji Watanabe <reijiw@google.com> Subject: [PATCH v3 03/14] KVM: arm64: PMU: Don't use the sanitized value for PMUVer Date: Thu, 2 Feb 2023 20:20:45 -0800 [thread overview] Message-ID: <20230203042056.1794649-2-reijiw@google.com> (raw) In-Reply-To: <20230203042056.1794649-1-reijiw@google.com> Now, the guest has the default PMU on the first vCPU reset when a PMU is configured for any of vCPUs on the guest. For a vCPU with PMU configured, use the PMUver of the guest's PMU as the default value of ID_AA64DFR0_EL1.PMUVer and as the limit value of the field, instead of its sanitized value (The sanitized value could be inappropriate for these on some heterogeneous PMU systems, as only one of PMUs on the system can be associated with the guest. See the previous patch for more details). When a PMU for the guest is changed, the PMUVer for the guest will be reset based on the new PMU. On heterogeneous systems, this might end up changing the PMUVer that is set by userspace for the guest if userspace changes the PMUVer before using KVM_ARM_VCPU_PMU_V3_SET_PMU. This change isn't nice though. Other options considered are not updating the PMUVer even when the PMU for the guest is changed, or setting PMUVer to the new limit value only when it is larger than the limit. The former might end up exposing PMUVer that KVM can't support. The latter is inconvenient as the default PMUVer for the PMU set by KVM_ARM_VCPU_PMU_V3_SET_PMU will be an unknown (but supported) value, and userspace explicitly need to set the PMUVer for the guest to use the host PMUVer value. Signed-off-by: Reiji Watanabe <reijiw@google.com> --- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/arm.c | 6 ----- arch/arm64/kvm/pmu-emul.c | 2 ++ arch/arm64/kvm/sys_regs.c | 38 +++++++++++++++++++++---------- include/kvm/arm_pmu.h | 1 - 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 35a159d131b5..33839077a95c 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -230,6 +230,7 @@ struct kvm_arch { struct { u8 imp:4; u8 unimp:4; + u8 imp_limit; } dfr0_pmuver; /* Hypercall features firmware registers' descriptor */ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 9c5573bc4614..41f478344a4d 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -164,12 +164,6 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) set_default_spectre(kvm); kvm_arm_init_hypercalls(kvm); - /* - * Initialise the default PMUver before there is a chance to - * create an actual PMU. - */ - kvm->arch.dfr0_pmuver.imp = kvm_arm_pmu_get_pmuver_limit(); - return 0; err_free_cpumask: diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index c98020ca427e..49580787ee09 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -878,6 +878,8 @@ int kvm_arm_set_vm_pmu(struct kvm *kvm, struct arm_pmu *arm_pmu) } kvm->arch.arm_pmu = arm_pmu; + kvm->arch.dfr0_pmuver.imp_limit = min_t(u8, arm_pmu->pmuver, ID_AA64DFR0_EL1_PMUVer_V3P5); + kvm->arch.dfr0_pmuver.imp = kvm->arch.dfr0_pmuver.imp_limit; return 0; } diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index c6cbfe6b854b..c1ec4a68b914 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1259,8 +1259,11 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, { u8 pmuver, host_pmuver; bool valid_pmu; + u64 current_val = read_id_reg(vcpu, rd); + int ret = -EINVAL; - host_pmuver = kvm_arm_pmu_get_pmuver_limit(); + mutex_lock(&vcpu->kvm->lock); + host_pmuver = vcpu->kvm->arch.dfr0_pmuver.imp_limit; /* * Allow AA64DFR0_EL1.PMUver to be set from userspace as long @@ -1270,26 +1273,30 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, */ pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), val); if ((pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF && pmuver > host_pmuver)) - return -EINVAL; + goto out; valid_pmu = (pmuver != 0 && pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF); /* Make sure view register and PMU support do match */ if (kvm_vcpu_has_pmu(vcpu) != valid_pmu) - return -EINVAL; + goto out; /* We can only differ with PMUver, and anything else is an error */ - val ^= read_id_reg(vcpu, rd); + val ^= current_val; val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer); if (val) - return -EINVAL; + goto out; if (valid_pmu) vcpu->kvm->arch.dfr0_pmuver.imp = pmuver; else vcpu->kvm->arch.dfr0_pmuver.unimp = pmuver; - return 0; + ret = 0; +out: + mutex_unlock(&vcpu->kvm->lock); + + return ret; } static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, @@ -1298,8 +1305,11 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, { u8 perfmon, host_perfmon; bool valid_pmu; + u64 current_val = read_id_reg(vcpu, rd); + int ret = -EINVAL; - host_perfmon = pmuver_to_perfmon(kvm_arm_pmu_get_pmuver_limit()); + mutex_lock(&vcpu->kvm->lock); + host_perfmon = pmuver_to_perfmon(vcpu->kvm->arch.dfr0_pmuver.imp_limit); /* * Allow DFR0_EL1.PerfMon to be set from userspace as long as @@ -1310,26 +1320,30 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, perfmon = FIELD_GET(ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon), val); if ((perfmon != ID_DFR0_EL1_PerfMon_IMPDEF && perfmon > host_perfmon) || (perfmon != 0 && perfmon < ID_DFR0_EL1_PerfMon_PMUv3)) - return -EINVAL; + goto out; valid_pmu = (perfmon != 0 && perfmon != ID_DFR0_EL1_PerfMon_IMPDEF); /* Make sure view register and PMU support do match */ if (kvm_vcpu_has_pmu(vcpu) != valid_pmu) - return -EINVAL; + goto out; /* We can only differ with PerfMon, and anything else is an error */ - val ^= read_id_reg(vcpu, rd); + val ^= current_val; val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon); if (val) - return -EINVAL; + goto out; if (valid_pmu) vcpu->kvm->arch.dfr0_pmuver.imp = perfmon_to_pmuver(perfmon); else vcpu->kvm->arch.dfr0_pmuver.unimp = perfmon_to_pmuver(perfmon); - return 0; + ret = 0; +out: + mutex_unlock(&vcpu->kvm->lock); + + return ret; } /* diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h index 7b5c5c8c634b..c7da46c7377e 100644 --- a/include/kvm/arm_pmu.h +++ b/include/kvm/arm_pmu.h @@ -95,7 +95,6 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu); #define kvm_pmu_is_3p5(vcpu) \ (vcpu->kvm->arch.dfr0_pmuver.imp >= ID_AA64DFR0_EL1_PMUVer_V3P5) -u8 kvm_arm_pmu_get_pmuver_limit(void); int kvm_arm_set_vm_pmu(struct kvm *kvm, struct arm_pmu *arm_pmu); #else -- 2.39.1.519.gcb327c4b5f-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>, Shaoqin Huang <shahuang@redhat.com>, Reiji Watanabe <reijiw@google.com> Subject: [PATCH v3 03/14] KVM: arm64: PMU: Don't use the sanitized value for PMUVer Date: Thu, 2 Feb 2023 20:20:45 -0800 [thread overview] Message-ID: <20230203042056.1794649-2-reijiw@google.com> (raw) In-Reply-To: <20230203042056.1794649-1-reijiw@google.com> Now, the guest has the default PMU on the first vCPU reset when a PMU is configured for any of vCPUs on the guest. For a vCPU with PMU configured, use the PMUver of the guest's PMU as the default value of ID_AA64DFR0_EL1.PMUVer and as the limit value of the field, instead of its sanitized value (The sanitized value could be inappropriate for these on some heterogeneous PMU systems, as only one of PMUs on the system can be associated with the guest. See the previous patch for more details). When a PMU for the guest is changed, the PMUVer for the guest will be reset based on the new PMU. On heterogeneous systems, this might end up changing the PMUVer that is set by userspace for the guest if userspace changes the PMUVer before using KVM_ARM_VCPU_PMU_V3_SET_PMU. This change isn't nice though. Other options considered are not updating the PMUVer even when the PMU for the guest is changed, or setting PMUVer to the new limit value only when it is larger than the limit. The former might end up exposing PMUVer that KVM can't support. The latter is inconvenient as the default PMUVer for the PMU set by KVM_ARM_VCPU_PMU_V3_SET_PMU will be an unknown (but supported) value, and userspace explicitly need to set the PMUVer for the guest to use the host PMUVer value. Signed-off-by: Reiji Watanabe <reijiw@google.com> --- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/arm.c | 6 ----- arch/arm64/kvm/pmu-emul.c | 2 ++ arch/arm64/kvm/sys_regs.c | 38 +++++++++++++++++++++---------- include/kvm/arm_pmu.h | 1 - 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 35a159d131b5..33839077a95c 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -230,6 +230,7 @@ struct kvm_arch { struct { u8 imp:4; u8 unimp:4; + u8 imp_limit; } dfr0_pmuver; /* Hypercall features firmware registers' descriptor */ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 9c5573bc4614..41f478344a4d 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -164,12 +164,6 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) set_default_spectre(kvm); kvm_arm_init_hypercalls(kvm); - /* - * Initialise the default PMUver before there is a chance to - * create an actual PMU. - */ - kvm->arch.dfr0_pmuver.imp = kvm_arm_pmu_get_pmuver_limit(); - return 0; err_free_cpumask: diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index c98020ca427e..49580787ee09 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -878,6 +878,8 @@ int kvm_arm_set_vm_pmu(struct kvm *kvm, struct arm_pmu *arm_pmu) } kvm->arch.arm_pmu = arm_pmu; + kvm->arch.dfr0_pmuver.imp_limit = min_t(u8, arm_pmu->pmuver, ID_AA64DFR0_EL1_PMUVer_V3P5); + kvm->arch.dfr0_pmuver.imp = kvm->arch.dfr0_pmuver.imp_limit; return 0; } diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index c6cbfe6b854b..c1ec4a68b914 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1259,8 +1259,11 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, { u8 pmuver, host_pmuver; bool valid_pmu; + u64 current_val = read_id_reg(vcpu, rd); + int ret = -EINVAL; - host_pmuver = kvm_arm_pmu_get_pmuver_limit(); + mutex_lock(&vcpu->kvm->lock); + host_pmuver = vcpu->kvm->arch.dfr0_pmuver.imp_limit; /* * Allow AA64DFR0_EL1.PMUver to be set from userspace as long @@ -1270,26 +1273,30 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu, */ pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), val); if ((pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF && pmuver > host_pmuver)) - return -EINVAL; + goto out; valid_pmu = (pmuver != 0 && pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF); /* Make sure view register and PMU support do match */ if (kvm_vcpu_has_pmu(vcpu) != valid_pmu) - return -EINVAL; + goto out; /* We can only differ with PMUver, and anything else is an error */ - val ^= read_id_reg(vcpu, rd); + val ^= current_val; val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer); if (val) - return -EINVAL; + goto out; if (valid_pmu) vcpu->kvm->arch.dfr0_pmuver.imp = pmuver; else vcpu->kvm->arch.dfr0_pmuver.unimp = pmuver; - return 0; + ret = 0; +out: + mutex_unlock(&vcpu->kvm->lock); + + return ret; } static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, @@ -1298,8 +1305,11 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, { u8 perfmon, host_perfmon; bool valid_pmu; + u64 current_val = read_id_reg(vcpu, rd); + int ret = -EINVAL; - host_perfmon = pmuver_to_perfmon(kvm_arm_pmu_get_pmuver_limit()); + mutex_lock(&vcpu->kvm->lock); + host_perfmon = pmuver_to_perfmon(vcpu->kvm->arch.dfr0_pmuver.imp_limit); /* * Allow DFR0_EL1.PerfMon to be set from userspace as long as @@ -1310,26 +1320,30 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu, perfmon = FIELD_GET(ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon), val); if ((perfmon != ID_DFR0_EL1_PerfMon_IMPDEF && perfmon > host_perfmon) || (perfmon != 0 && perfmon < ID_DFR0_EL1_PerfMon_PMUv3)) - return -EINVAL; + goto out; valid_pmu = (perfmon != 0 && perfmon != ID_DFR0_EL1_PerfMon_IMPDEF); /* Make sure view register and PMU support do match */ if (kvm_vcpu_has_pmu(vcpu) != valid_pmu) - return -EINVAL; + goto out; /* We can only differ with PerfMon, and anything else is an error */ - val ^= read_id_reg(vcpu, rd); + val ^= current_val; val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon); if (val) - return -EINVAL; + goto out; if (valid_pmu) vcpu->kvm->arch.dfr0_pmuver.imp = perfmon_to_pmuver(perfmon); else vcpu->kvm->arch.dfr0_pmuver.unimp = perfmon_to_pmuver(perfmon); - return 0; + ret = 0; +out: + mutex_unlock(&vcpu->kvm->lock); + + return ret; } /* diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h index 7b5c5c8c634b..c7da46c7377e 100644 --- a/include/kvm/arm_pmu.h +++ b/include/kvm/arm_pmu.h @@ -95,7 +95,6 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu); #define kvm_pmu_is_3p5(vcpu) \ (vcpu->kvm->arch.dfr0_pmuver.imp >= ID_AA64DFR0_EL1_PMUVer_V3P5) -u8 kvm_arm_pmu_get_pmuver_limit(void); int kvm_arm_set_vm_pmu(struct kvm *kvm, struct arm_pmu *arm_pmu); #else -- 2.39.1.519.gcb327c4b5f-goog _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-02-03 4:23 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-03 4:20 [PATCH v3 02/14] KVM: arm64: PMU: Set the default PMU for the guest on vCPU reset Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe [this message] 2023-02-03 4:20 ` [PATCH v3 03/14] KVM: arm64: PMU: Don't use the sanitized value for PMUVer Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 04/14] KVM: arm64: PMU: Don't use the PMUVer of the PMU set for the guest Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 05/14] KVM: arm64: PMU: Clear PM{C,I}NTEN{SET,CLR} and PMOVS{SET,CLR} on vCPU reset Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 06/14] KVM: arm64: PMU: Don't define the sysreg reset() for PM{USERENR,CCFILTR}_EL0 Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 07/14] KVM: arm64: PMU: Simplify extracting PMCR_EL0.N Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 08/14] KVM: arm64: PMU: Add a helper to read a vCPU's PMCR_EL0 Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 09/14] KVM: arm64: PMU: Set PMCR_EL0.N for vCPU based on the associated PMU Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 10/14] KVM: arm64: PMU: Allow userspace to limit PMCR_EL0.N for the guest Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 11/14] tools: arm64: Import perf_event.h Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 12/14] KVM: selftests: aarch64: Introduce vpmu_counter_access test Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 13/14] KVM: selftests: aarch64: vPMU register test for implemented counters Reiji Watanabe 2023-02-03 4:20 ` Reiji Watanabe 2023-02-03 4:20 ` [PATCH v3 14/14] KVM: selftests: aarch64: vPMU register test for unimplemented counters Reiji Watanabe 2023-02-03 4:20 ` 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=20230203042056.1794649-2-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=shahuang@redhat.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: linkBe 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.