Re: [PATCH] KVM: selftests: Add coverage of MTE system registers

[Marc Zyngier <maz@kernel.org>]

On Sun, 12 Mar 2023 14:35:13 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Sun, Mar 12, 2023 at 10:29:11AM +0000, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > >  static struct vcpu_config *vcpu_configs[] = {
> > >  	&vregs_config,
> > > @@ -1131,5 +1163,6 @@ static struct vcpu_config *vcpu_configs[] = {
> > >  	&sve_pmu_config,
> > >  	&pauth_config,
> > >  	&pauth_pmu_config,
> > > +	&mte_config,
> > >  };
> > >  static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
> 
> > Is there any reason why we sidestep the combinations of MTE with PAuth
> > and PMU? I know this leads to an exponential set growth, but this is
> > the very purpose of this test, and we found bugs related to this in
> > the past.
> 
> The test is already not bothering with the combinations of SVE
> and pointer auth, it appeared that the intent of the test was
> only to test specific combinations.  From what's there it looks
> more like there's something with PMU interacting specially with
> things (it's all X and X+PMU) that needs coverage.  I couldn't
> see anything between it and MTE, though I nearly added a MTE+PMU
> combination just for the sake of it.  It's one of those areas
> where it's hard to determine if there's an intent behind the
> implementation choices made or if they're just whatever someone
> happened to write and not particularly important or desired.

It *is* desired. We've had cases of flags being reset at the wrong
time and leading to issues that would be detected by this test. The
PMU stuff is indeed one example, but similar things could happen
between SVE+MTE, for example.

> 
> > A good first step would be to be able to build these combinations
> > dynamically, and only then add new sublists to the mix.
> 
> That would certainly be a good idea, if we were heading in that
> direction I'd also expect negative tests checking that for
> example pointer authentication registers don't appear when that's
> not enabled.  I'm not sure that it's worth blocking all new
> coverage for that though, there is still value in having a bit of
> basic coverage even if not all the combinations are covered yet.

Then where is the incentive to get it fixed? People will just keep
piling stuff, and the coverage will increasingly become worse.

We have to do it as some point, and now is as good a time as any.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel