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

From: Joey Gouly <joey.gouly@arm.com>
To: Andrew Jones <ajones@ventanamicro.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>
Subject: Re: [PATCH v4 20/20] KVM: selftests: get-reg-list: add Permission Indirection registers
Date: Wed, 12 Jul 2023 15:59:17 +0100	[thread overview]
Message-ID: <20230712145917.GA3526439@e124191.cambridge.arm.com> (raw)
In-Reply-To: <20230703120344.14247-2-ajones@ventanamicro.com>

Hi,

On Mon, Jul 03, 2023 at 02:03:45PM +0200, Andrew Jones wrote:
> > Add new system registers:
> >   - ID_AA64MMFR3_EL1
> >   - TCR2_EL1
> >   - PIRE0_EL1
> >   - PIR_EL1
> > 
> > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Oliver Upton <oliver.upton@linux.dev>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  .../selftests/kvm/aarch64/get-reg-list.c      | 21 +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > index 3ab236ceb6fc..4f10055af2aa 100644
> > --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> > @@ -56,6 +56,24 @@ struct feature_id_reg {
> >  };
> >  
> >  static struct feature_id_reg feat_id_regs[] = {
> > +	{
> > +		ARM64_SYS_REG(3, 0, 2, 0, 3),	/* TCR2_EL1 */
> > +		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
> > +		0,
> > +		1
> > +	},
> > +	{
> > +		ARM64_SYS_REG(3, 0, 10, 2, 2),	/* PIRE0_EL1 */
> > +		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
> > +		4,
> > +		1
> > +	},
> > +	{
> > +		ARM64_SYS_REG(3, 0, 10, 2, 3),	/* PIR_EL1 */
> > +		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
> > +		4,
> > +		1
> > +	}
> >  };
> >  
> >  struct vcpu_config {
> > @@ -873,12 +891,15 @@ static __u64 base_regs[] = {
> >  	ARM64_SYS_REG(3, 0, 2, 0, 0),	/* TTBR0_EL1 */
> >  	ARM64_SYS_REG(3, 0, 2, 0, 1),	/* TTBR1_EL1 */
> >  	ARM64_SYS_REG(3, 0, 2, 0, 2),	/* TCR_EL1 */
> > +	ARM64_SYS_REG(3, 0, 2, 0, 3),	/* TCR2_EL1 */
> >  	ARM64_SYS_REG(3, 0, 5, 1, 0),	/* AFSR0_EL1 */
> >  	ARM64_SYS_REG(3, 0, 5, 1, 1),	/* AFSR1_EL1 */
> >  	ARM64_SYS_REG(3, 0, 5, 2, 0),	/* ESR_EL1 */
> >  	ARM64_SYS_REG(3, 0, 6, 0, 0),	/* FAR_EL1 */
> >  	ARM64_SYS_REG(3, 0, 7, 4, 0),	/* PAR_EL1 */
> >  	ARM64_SYS_REG(3, 0, 10, 2, 0),	/* MAIR_EL1 */
> > +	ARM64_SYS_REG(3, 0, 10, 2, 2),	/* PIRE0_EL1 */
> > +	ARM64_SYS_REG(3, 0, 10, 2, 3),	/* PIR_EL1 */
> 
> Hi Joey,
> 
> 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.

Thanks,
Joey

[1] https://lore.kernel.org/linux-arm-kernel/86y1o5yjs7.wl-maz@kernel.org/

> That way the subtest can SKIP when the new registers
> aren't present. That approach should avoid the need for modifying
> for_each_missing_reg() (well, after changing the call of check_supported()
> to be after configuring the vcpu, in order to pass the vcpu to it, and
> after extending check_supported() to call check_supported_feat_reg()).
> 
> Thanks,
> drew
> 
> >  	ARM64_SYS_REG(3, 0, 10, 3, 0),	/* AMAIR_EL1 */
> >  	ARM64_SYS_REG(3, 0, 12, 0, 0),	/* VBAR_EL1 */
> >  	ARM64_SYS_REG(3, 0, 12, 1, 1),	/* DISR_EL1 */

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