Re: [PATCH v4 20/20] KVM: selftests: get-reg-list: add Permission Indirection registers

From: Andrew Jones <ajones@ventanamicro.com>
To: Joey Gouly <joey.gouly@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, nd@arm.com,
	broonie@kernel.org, catalin.marinas@arm.com, james.morse@arm.com,
	mark.rutland@arm.com, maz@kernel.org, oliver.upton@linux.dev,
	shuah@kernel.org, suzuki.poulose@arm.com, will@kernel.org,
	yuzenghui@huawei.com, haibo1.xu@intel.com
Subject: Re: [PATCH v4 20/20] KVM: selftests: get-reg-list: add Permission Indirection registers
Date: Wed, 19 Jul 2023 17:11:33 +0200	[thread overview]
Message-ID: <20230712-e563781672725ca058131775@orel> (raw)
In-Reply-To: <20230712145917.GA3526439@e124191.cambridge.arm.com>

On Wed, Jul 12, 2023 at 03:59:17PM +0100, Joey Gouly wrote:
> On Mon, Jul 03, 2023 at 02:03:45PM +0200, Andrew Jones wrote:
...
> > Any reason these registers needed to be added to the base reg set? Usually
> > new registers get added to their own sublist, which then get tested by
> > their own subtest.
> 
> I added them to the base set because there is no feature/capability to enable PIE [1] (unlike SVE, PMU etc)
> That means they have to be part of the base set, otherwise the test will complain about missing regs.
>

Hi Joey,

Without the registers in a blessed list, then I wouldn't expect them to be
reported as 'missing', but rather as 'new' (and only when running on a
platform that supports them and with a recent enough KVM to have them
exposed). There are two ways to approach registers like these:

 1) the way this patch has done it, i.e. add the registers to the "base"
    list so they'll be expected by all subtests, ensuring they'll never
    appear as 'new', but then also avoid false-positive 'missing' failures
    by filtering them out of the missing list when necessary.

 2) add the registers to subtests which only run when the registers
    should be present (skipping the tests when not) and then always filter
    the registers out of the 'new' list. (It's always safe to filter
    explicitly defined registers out of the new list since "new" means new
    to the selftest, but the selftest must be aware of those registers in
    order to explicitly filter them, which means they can't be new :-)

To me, (2) is the cleaner approach, particularly because we've had subtest
skipping support and 'new' register filtering support from nearly the
beginning. riscv also has some nonconfigurable registers which are only
present when the platform supports them. The riscv get-one-reg series[1]
has taken approach (2) to handle those.

At some point maybe we can consider converting the permission indirection
registers to approach (2), but it's not critical to do anytime soon. So,
for now, we'll rebase the riscv series in a way that preserves using
approach (1) for aarch64, but also allows using approach (2) for riscv.

[1] https://lore.kernel.org/all/cover.1688010022.git.haibo1.xu@intel.com/

Thanks,
drew

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