[RFC PATCH v3 09/29] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest

[Alexandru Elisei]

Hi Reiji,

On Mon, Nov 29, 2021 at 09:32:02PM -0800, Reiji Watanabe wrote:
> Hi Eric,
> 
> On Thu, Nov 25, 2021 at 12:30 PM Eric Auger <eauger at redhat.com> wrote:
> >
> > Hi Reiji,
> >
> > On 11/17/21 7:43 AM, Reiji Watanabe wrote:
> > > When ID_AA64DFR0_EL1.PMUVER or ID_DFR0_EL1.PERFMON is 0xf, which
> > > means IMPLEMENTATION DEFINED PMU supported, KVM unconditionally
> > > expose the value for the guest as it is.  Since KVM doesn't support
> > > IMPLEMENTATION DEFINED PMU for the guest, in that case KVM should
> > > exopse 0x0 (PMU is not implemented) instead.
> > s/exopse/expose
> > >
> > > Change cpuid_feature_cap_perfmon_field() to update the field value
> > > to 0x0 when it is 0xf.
> > is it wrong to expose the guest with a Perfmon value of 0xF? Then the
> > guest should not use it as a PMUv3?
> 
> > is it wrong to expose the guest with a Perfmon value of 0xF? Then the
> > guest should not use it as a PMUv3?
> 
> For the value 0xf in ID_AA64DFR0_EL1.PMUVER and ID_DFR0_EL1.PERFMON,
> Arm ARM says:
>   "IMPLEMENTATION DEFINED form of performance monitors supported,
>    PMUv3 not supported."
> 
> Since the PMU that KVM supports for guests is PMUv3, 0xf shouldn't
> be exposed to guests (And this patch series doesn't allow userspace
> to set the fields to 0xf for guests).

While it's true that a value of 0xf means that PMUv3 is not present (both
KVM and the PMU driver handle it this way) this is an userspace visible
change.

Are you sure there isn't software in the wild that relies on this value
being 0xf to detect that some non-Arm architected hardware is present?

Since both 0 and 0xf are valid values that mean that PMUv3 is not present,
I think it's best that both are kept.

Thanks,
Alex

> 
> Thanks,
> Reiji
> 
> >
> > Eric
> > >
> > > Fixes: 8e35aa642ee4 ("arm64: cpufeature: Extract capped perfmon fields")
> > > Signed-off-by: Reiji Watanabe <reijiw at google.com>
> > > ---
> > >  arch/arm64/include/asm/cpufeature.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > > index ef6be92b1921..fd7ad8193827 100644
> > > --- a/arch/arm64/include/asm/cpufeature.h
> > > +++ b/arch/arm64/include/asm/cpufeature.h
> > > @@ -553,7 +553,7 @@ cpuid_feature_cap_perfmon_field(u64 features, int field, u64 cap)
> > >
> > >       /* Treat IMPLEMENTATION DEFINED functionality as unimplemented */
> > >       if (val == ID_AA64DFR0_PMUVER_IMP_DEF)
> > > -             val = 0;
> > > +             return (features & ~mask);
> > >
> > >       if (val > cap) {
> > >               features &= ~mask;
> > >
> >