Hi Marc,

On 2/3/21 10:32 AM, Marc Zyngier wrote:

On 2021-01-27 17:41, Alexandru Elisei wrote:

quoted

Hi Marc,

Had another look at the patch, comments below.

On 1/25/21 12:26 PM, Marc Zyngier wrote:

quoted

Upgrading the PMU code from ARMv8.1 to ARMv8.4 turns out to be
pretty easy. All that is required is support for PMMIR_EL1, which
is read-only, and for which returning 0 is a valid option as long
as we don't advertise STALL_SLOT as an implemented event.

Let's just do that and adjust what we return to the guest.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/sysreg.h |  3 +++
 arch/arm64/kvm/pmu-emul.c       |  6 ++++++
 arch/arm64/kvm/sys_regs.c       | 11 +++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 8b5e7e5c3cc8..2fb3f386588c 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h

@@ -846,7 +846,10 @@

 #define ID_DFR0_PERFMON_SHIFT        24

+#define ID_DFR0_PERFMON_8_0        0x3
 #define ID_DFR0_PERFMON_8_1        0x4
+#define ID_DFR0_PERFMON_8_4        0x5
+#define ID_DFR0_PERFMON_8_5        0x6

 #define ID_ISAR4_SWP_FRAC_SHIFT        28
 #define ID_ISAR4_PSR_M_SHIFT        24

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 398f6df1bbe4..72cd704a8368 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c

@@ -795,6 +795,12 @@ u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)

         base = 0;
     } else {
         val = read_sysreg(pmceid1_el0);
+        /*
+         * Don't advertise STALL_SLOT, as PMMIR_EL0 is handled
+         * as RAZ
+         */
+        if (vcpu->kvm->arch.pmuver >= ID_AA64DFR0_PMUVER_8_4)
+            val &= ~BIT_ULL(ARMV8_PMUV3_PERFCTR_STALL_SLOT - 32);

This is confusing the me. We have kvm->arch.pmuver set to the hardware
PMU version
(as set by __armv8pmu_probe_pmu()), but we ignore it when reporting the PMU
version to the guest. Why do we do that? We limit the event number in
kvm_pmu_event_mask() based on the hardware PMU version, so even if we advertise
Armv8.4 PMU, support for all those extra events added by Arm8.1 PMU is
missing (I hope I understood the code correctly).

That's a bit of mess-up. We obtain ID_AA64DFR0_EL1 from the sanitised
regs, but do most of our handling based on kvm->arch.pmuver. They really
should be the same, because that's what the sanitised registers give
you.

As for the events themselves, I don't get your drift. We do support
all the ARMv8.1 PMU events as long as the HW supports it, and we
don't lie to the guest about it either (cpuid_feature_cap_perfmon_field
does *cap* the field to some value, it doesn't allow it to increase
past what the HW supports).

That's the piece that I was missing - I didn't realize that
cpuid_feature_cap_perfmon_field() makes sure that the final version doesn't exceed
what the hardware supports. Thanks for clearing it up!

quoted

I looked at commit c854188ea010 ("KVM: arm64: limit PMU version to PMUv3 for
ARMv8.1") which changed read_id_reg() to report PMUv3 for Armv8.1
unconditionally,
and there's no explanation why PMUv3 for Armv8.1 was chosen instead of
plain PMUv3 (PMUVer = 0b0100).

We picked ARMv8.1 because this is the first PMU revision that gives
you events in the 0x4000 range, all of which are available on
common ARMv8.2 HW.

Yes, makes sense in the context of cpuid_feature_cap_perfmon_field() capping
PMUVer based on the hardware supported version.

Thanks,
Alex

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