[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 3/6] target/arm: Always add pmu property for Armv7-A/R+
From: |
Peter Maydell |
Subject: |
Re: [PATCH v4 3/6] target/arm: Always add pmu property for Armv7-A/R+ |
Date: |
Mon, 29 Jul 2024 16:13:52 +0100 |
On Sat, 20 Jul 2024 at 10:31, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> kvm-steal-time and sve properties are added for KVM even if the
> corresponding features are not available. Always add pmu property for
> Armv8. Note that the property is added only for Armv7-A/R+ as QEMU
> currently emulates PMU only for such versions, and a different
> version may have a different definition of PMU or may not have one at
> all.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> target/arm/cpu.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 19191c239181..c1955a82fb3c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1741,6 +1741,10 @@ void arm_cpu_post_init(Object *obj)
>
> if (!arm_feature(&cpu->env, ARM_FEATURE_M)) {
> qdev_property_add_static(DEVICE(obj),
> &arm_cpu_reset_hivecs_property);
> +
> + if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> + object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
> + }
Not every V7 CPU has a PMU[*]. Unfortunately for PMUv1 the
architecture did not define an ID register field for it,
so there's no ID field you can look at to distinguish
"has PMUv1" from "has no PMU". (For PMUv2 and later you
can look at ID_DFR0 bits [27:24]; or for AArch64
ID_AA64DFR0_EL1.PMUVer.) This is why we have the
ARM_FEATURE_PMU feature bit. So the correct way to determine
"does this CPU have a PMU and so it's OK to add the 'pmu'
property" is to look at ARM_FEATURE_PMU. Which is what
we already do.
Alternatively, if you want to make the property always
present even on CPUs where it can't be set, you need
to have some mechanism for having the user's attempt to
enable it fail. But mostly for Arm at the moment we
have properties which are only present when they're
meaningful. (I'm not opposed to changing this -- it would
arguably be cleaner to have properties be per-class,
not per-object, to aid in introspection. But it's a big
task and probably not easy.)
[*] It happens that all the v7 CPUs that QEMU currently
models do have at least a PMUv1, but that's not an
architectural requirement.
thanks
-- PMM
- [PATCH v4 0/6] target/arm/kvm: Report PMU unavailability, Akihiko Odaki, 2024/07/20
- [PATCH v4 1/6] target/arm/kvm: Set PMU for host only when available, Akihiko Odaki, 2024/07/20
- [PATCH v4 2/6] target/arm/kvm: Do not silently remove PMU, Akihiko Odaki, 2024/07/20
- [PATCH v4 3/6] target/arm: Always add pmu property for Armv7-A/R+, Akihiko Odaki, 2024/07/20
- Re: [PATCH v4 3/6] target/arm: Always add pmu property for Armv7-A/R+,
Peter Maydell <=
- [PATCH v4 4/6] hvf: arm: Raise an exception for sysreg by default, Akihiko Odaki, 2024/07/20
- [PATCH v4 5/6] hvf: arm: Properly disable PMU, Akihiko Odaki, 2024/07/20
- [PATCH v4 6/6] hvf: arm: Do not advance PC when raising an exception, Akihiko Odaki, 2024/07/20
- Re: [PATCH v4 0/6] target/arm/kvm: Report PMU unavailability, Peter Maydell, 2024/07/29