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

From: Andrew Jones <ajones@ventanamicro.com>
To: 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
Subject: Re: [PATCH v4 20/20] KVM: selftests: get-reg-list: add Permission Indirection registers
Date: Mon,  3 Jul 2023 14:03:45 +0200	[thread overview]
Message-ID: <20230703120344.14247-2-ajones@ventanamicro.com> (raw)
In-Reply-To: <20230606145859.697944-21-joey.gouly@arm.com>

> 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. 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 */
> -- 
> 2.25.1
> 

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