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

[Alexandru Elisei]

Hi Reiji,

On Wed, Dec 01, 2021 at 03:53:18PM +0000, Alexandru Elisei wrote:
> 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.

Sorry, somehow I managed to get myself confused and didn't realize that
this is only used by KVM.

What I said above about the possibility of software existing that pokes IMP
DEF registers when PMUVer = 0xf is in fact a good argument for this patch,
because KVM injects an undefined exception when a guest tries to access
such registers.

Thanks,
Alex

> 
> 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;
> > > >
> > >