kvm-steal-time and sve properties are added for KVM even if the
corresponding features are not available. Always add pmu property too.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 target/arm/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 9e1d15701468..32508644aee7 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1781,9 +1781,10 @@ void arm_cpu_post_init(Object *obj)
 
     if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
         cpu->has_pmu = true;
-        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
     }
 
+    object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
+
     /*
      * Allow user to turn off VFP and Neon support, but only for TCG --
      * KVM does not currently allow us to lie to the guest about its

-- 
2.45.2

On Tue, 16 Jul 2024 at 09:28, 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 too.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  target/arm/cpu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 9e1d15701468..32508644aee7 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1781,9 +1781,10 @@ void arm_cpu_post_init(Object *obj)
>
>      if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
>          cpu->has_pmu = true;
> -        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
>      }
>
> +    object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
> +
>      /*
>       * Allow user to turn off VFP and Neon support, but only for TCG --
>       * KVM does not currently allow us to lie to the guest about its

Before we do this we need to do something to forbid setting
the pmu property to true on CPUs which don't have it. That is:

 * for CPUs which do have a PMU, we should default to present, and
   allow the user to turn it on and off with pmu=on/off
 * for CPUs which do not have a PMU, we should not let the user
   turn it on and off (either by not providing the property, or
   else by making the property-set method raise an error, or by
   having realize detect the discrepancy and raise an error)

thanks
-- PMM

On 2024/07/16 20:32, Peter Maydell wrote:
> On Tue, 16 Jul 2024 at 09:28, 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 too.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   target/arm/cpu.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 9e1d15701468..32508644aee7 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1781,9 +1781,10 @@ void arm_cpu_post_init(Object *obj)
>>
>>       if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
>>           cpu->has_pmu = true;
>> -        object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
>>       }
>>
>> +    object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
>> +
>>       /*
>>        * Allow user to turn off VFP and Neon support, but only for TCG --
>>        * KVM does not currently allow us to lie to the guest about its
> 
> Before we do this we need to do something to forbid setting
> the pmu property to true on CPUs which don't have it. That is:
> 
>   * for CPUs which do have a PMU, we should default to present, and
>     allow the user to turn it on and off with pmu=on/off
>   * for CPUs which do not have a PMU, we should not let the user
>     turn it on and off (either by not providing the property, or
>     else by making the property-set method raise an error, or by
>     having realize detect the discrepancy and raise an error)

I don't think there is any reason to prohibit adding a PMU to a CPU that 
doesn't have when you allow to remove one. For example, neoverse-v1 
should always have PMU in the real world.

Perhaps it may make sense to prohibit adding a PMU when the CPU is not 
Armv8 as the PMU we emulate is apparently PMUv3, which is part of Armv8.

Regards,
Akihiko Odaki

On Tue, 16 Jul 2024 at 12:36, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> On 2024/07/16 20:32, Peter Maydell wrote:
> > On Tue, 16 Jul 2024 at 09:28, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> > Before we do this we need to do something to forbid setting
> > the pmu property to true on CPUs which don't have it. That is:
> >
> >   * for CPUs which do have a PMU, we should default to present, and
> >     allow the user to turn it on and off with pmu=on/off
> >   * for CPUs which do not have a PMU, we should not let the user
> >     turn it on and off (either by not providing the property, or
> >     else by making the property-set method raise an error, or by
> >     having realize detect the discrepancy and raise an error)
>
> I don't think there is any reason to prohibit adding a PMU to a CPU that
> doesn't have when you allow to remove one. For example, neoverse-v1
> should always have PMU in the real world.

For example, the Cortex-M3 doesn't have a PMU anything like the
A-profile one, so we shouldn't allow the user to set pmu=on.
The Arm1176 doesn't have a PMU like the one we emulate, so we
shouldn't allow the user to turn it on. All the CPUs where it
is reasonable and architecturally valid to have a PMU set the
ARM_FEATURE_PMU bit, so there (by design) is no CPU where that
bit isn't set by default but could reasonably be enabled by
the user.

Conversely, the PMUv3 is architecturally optional, so it's not
unreasonable to allow the user to disable it even if the
real-hardware Neoverse-V1 doesn't provide that as a config
option in the RTL.

thanks
-- PMM

On 16/7/24 10:28, Akihiko Odaki wrote:
> kvm-steal-time and sve properties are added for KVM even if the
> corresponding features are not available. Always add pmu property too.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   target/arm/cpu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>