On Thu, 7 Dec 2023 at 13:19, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> On 7/12/23 12:10, Philippe Mathieu-Daudé wrote:
> > ARM Performance Monitor Unit is not reachable from user
> > emulation, restrict it to system emulation.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
What's the aim of this patch? In general if we can avoid
ifdefs then I prefer to avoid ifdefs, because they clutter
up the code. So they should come with a justification of
why they're necessary (eg "the QOM property is visible to
the end-user but pointless" or "we want to be able to
change this file to be compiled a different way and this is
a necessary substep" or whatever the reason is).
> > ---
> > target/arm/cpu.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 51f57fd5b4..60cf747fd6 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -1410,6 +1410,7 @@ static Property arm_cpu_pmsav7_dregion_property =
> > pmsav7_dregion,
> > qdev_prop_uint32, uint32_t);
> >
> > +#ifndef CONFIG_USER_ONLY
> > static bool arm_get_pmu(Object *obj, Error **errp)
> > {
> > ARMCPU *cpu = ARM_CPU(obj);
> > @@ -1432,6 +1433,7 @@ static void arm_set_pmu(Object *obj, bool value,
> > Error **errp)
> > }
> > cpu->has_pmu = value;
> > }
> > +#endif
> >
> > unsigned int gt_cntfrq_period_ns(ARMCPU *cpu)
> > {
> > @@ -1592,12 +1594,12 @@ void arm_cpu_post_init(Object *obj)
> > if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) {
> > qdev_property_add_static(DEVICE(obj), &arm_cpu_has_el2_property);
> > }
> > -#endif
> >
> > if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) {
> > cpu->has_pmu = true;
> > object_property_add_bool(obj, "pmu", arm_get_pmu, arm_set_pmu);
> > }
> > +#endif
>
> I think this patch is incomplete: should the PMU registers in
> v7_cp_reginfo[] be restricted to TCG?
I can't remember if we had this conversation on IRC, but
in general it's preferable not to restrict a regdef to
TCG only because that means that gdb won't be able to read
the register value if you're using KVM.
thanks
-- PMM
