From: Itaru Kitayama <itaru.kitayama@linux.dev>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
kvm@vger.kernel.org, Joey Gouly <joey.gouly@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Oliver Upton <oliver.upton@linux.dev>,
Zenghui Yu <yuzenghui@huawei.com>,
Eric Auger <eric.auger@redhat.com>
Subject: Re: [PATCH 4/4] KVM: arm64: selftest: vgic-v3: Add basic GICv3 sysreg userspace access test
Date: Wed, 23 Jul 2025 14:00:40 +0900 [thread overview]
Message-ID: <aIBseJ3aO+hMVAee@vm4> (raw)
In-Reply-To: <20250718111154.104029-5-maz@kernel.org>
On Fri, Jul 18, 2025 at 12:11:54PM +0100, Marc Zyngier wrote:
> We have a lot of more or less useful vgic tests, but none of them
> tracks the availability of GICv3 system registers, which is a bit
> annoying.
>
> Add one such test, which covers both EL1 and EL2 registers.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
I've tested this selftest on the RevC FVP with kvm-arm.mode=nested.
Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com>
Running GIC_v3 tests.
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='657'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='682'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='682'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='657'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='657'
SKIP SYS_ICC_AP0R1_EL1 for read
SKIP SYS_ICC_AP0R1_EL1 for write
SKIP SYS_ICC_AP0R2_EL1 for read
SKIP SYS_ICC_AP0R2_EL1 for write
SKIP SYS_ICC_AP0R3_EL1 for read
SKIP SYS_ICC_AP0R3_EL1 for write
SKIP SYS_ICC_AP1R1_EL1 for read
SKIP SYS_ICC_AP1R1_EL1 for write
SKIP SYS_ICC_AP1R2_EL1 for read
SKIP SYS_ICC_AP1R2_EL1 for write
SKIP SYS_ICC_AP1R3_EL1 for read
SKIP SYS_ICC_AP1R3_EL1 for write
SKIP SYS_ICH_AP0R1_EL2 for read
SKIP SYS_ICH_AP0R1_EL2 for write
SKIP SYS_ICH_AP0R2_EL2 for read
SKIP SYS_ICH_AP0R2_EL2 for write
SKIP SYS_ICH_AP0R3_EL2 for read
SKIP SYS_ICH_AP0R3_EL2 for write
SKIP SYS_ICH_AP1R1_EL2 for read
SKIP SYS_ICH_AP1R1_EL2 for write
SKIP SYS_ICH_AP1R2_EL2 for read
SKIP SYS_ICH_AP1R2_EL2 for write
SKIP SYS_ICH_AP1R3_EL2 for read
SKIP SYS_ICH_AP1R3_EL2 for write
__vm_create: mode='PA-bits:40, VA-bits:48, 4K pages' type='0', pages='672'
> ---
> tools/testing/selftests/kvm/arm64/vgic_init.c | 219 +++++++++++++++++-
> 1 file changed, 217 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/arm64/vgic_init.c b/tools/testing/selftests/kvm/arm64/vgic_init.c
> index b3b5fb0ff0a9a..7d508dcdbf23d 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_init.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_init.c
> @@ -9,6 +9,8 @@
> #include <asm/kvm.h>
> #include <asm/kvm_para.h>
>
> +#include <arm64/gic_v3.h>
> +
> #include "test_util.h"
> #include "kvm_util.h"
> #include "processor.h"
> @@ -18,8 +20,6 @@
>
> #define REG_OFFSET(vcpu, offset) (((uint64_t)vcpu << 32) | offset)
>
> -#define GICR_TYPER 0x8
> -
> #define VGIC_DEV_IS_V2(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V2)
> #define VGIC_DEV_IS_V3(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V3)
>
> @@ -715,6 +715,220 @@ int test_kvm_device(uint32_t gic_dev_type)
> return 0;
> }
>
> +struct sr_def {
> + const char *name;
> + u32 encoding;
> +};
> +
> +#define PACK_SR(r) \
> + ((sys_reg_Op0(r) << 14) | \
> + (sys_reg_Op1(r) << 11) | \
> + (sys_reg_CRn(r) << 7) | \
> + (sys_reg_CRm(r) << 3) | \
> + (sys_reg_Op2(r)))
> +
> +#define SR(r) \
> + { \
> + .name = #r, \
> + .encoding = r, \
> + }
> +
> +static const struct sr_def sysregs_el1[] = {
> + SR(SYS_ICC_PMR_EL1),
> + SR(SYS_ICC_BPR0_EL1),
> + SR(SYS_ICC_AP0R0_EL1),
> + SR(SYS_ICC_AP0R1_EL1),
> + SR(SYS_ICC_AP0R2_EL1),
> + SR(SYS_ICC_AP0R3_EL1),
> + SR(SYS_ICC_AP1R0_EL1),
> + SR(SYS_ICC_AP1R1_EL1),
> + SR(SYS_ICC_AP1R2_EL1),
> + SR(SYS_ICC_AP1R3_EL1),
> + SR(SYS_ICC_BPR1_EL1),
> + SR(SYS_ICC_CTLR_EL1),
> + SR(SYS_ICC_SRE_EL1),
> + SR(SYS_ICC_IGRPEN0_EL1),
> + SR(SYS_ICC_IGRPEN1_EL1),
> +};
> +
> +static const struct sr_def sysregs_el2[] = {
> + SR(SYS_ICH_AP0R0_EL2),
> + SR(SYS_ICH_AP0R1_EL2),
> + SR(SYS_ICH_AP0R2_EL2),
> + SR(SYS_ICH_AP0R3_EL2),
> + SR(SYS_ICH_AP1R0_EL2),
> + SR(SYS_ICH_AP1R1_EL2),
> + SR(SYS_ICH_AP1R2_EL2),
> + SR(SYS_ICH_AP1R3_EL2),
> + SR(SYS_ICH_HCR_EL2),
> + SR(SYS_ICC_SRE_EL2),
> + SR(SYS_ICH_VTR_EL2),
> + SR(SYS_ICH_VMCR_EL2),
> + SR(SYS_ICH_LR0_EL2),
> + SR(SYS_ICH_LR1_EL2),
> + SR(SYS_ICH_LR2_EL2),
> + SR(SYS_ICH_LR3_EL2),
> + SR(SYS_ICH_LR4_EL2),
> + SR(SYS_ICH_LR5_EL2),
> + SR(SYS_ICH_LR6_EL2),
> + SR(SYS_ICH_LR7_EL2),
> + SR(SYS_ICH_LR8_EL2),
> + SR(SYS_ICH_LR9_EL2),
> + SR(SYS_ICH_LR10_EL2),
> + SR(SYS_ICH_LR11_EL2),
> + SR(SYS_ICH_LR12_EL2),
> + SR(SYS_ICH_LR13_EL2),
> + SR(SYS_ICH_LR14_EL2),
> + SR(SYS_ICH_LR15_EL2),
> +};
> +
> +static void test_sysreg_array(int gic, const struct sr_def *sr, int nr,
> + int (*check)(int, const struct sr_def *, const char *))
> +{
> + for (int i = 0; i < nr; i++) {
> + u64 val;
> + u64 attr;
> + int ret;
> +
> + /* Assume MPIDR_EL1.Aff*=0 */
> + attr = PACK_SR(sr[i].encoding);
> +
> + /*
> + * The API is braindead. A register can be advertised as
> + * available, and yet not be readable or writable.
> + * ICC_APnR{1,2,3}_EL1 are examples of such non-sense, and
> + * ICH_APnR{1,2,3}_EL2 do follow suit for consistency.
> + *
> + * On the bright side, no known HW is implementing more than
> + * 5 bits of priority, so we're safe. Sort of...
> + */
> + ret = __kvm_has_device_attr(gic, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + attr);
> + TEST_ASSERT(ret == 0, "%s unavailable", sr[i].name);
> +
> + /* Check that we can write back what we read */
> + ret = __kvm_device_attr_get(gic, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + attr, &val);
> + TEST_ASSERT(ret == 0 || !check(gic, &sr[i], "read"), "%s unreadable", sr[i].name);
> + ret = __kvm_device_attr_set(gic, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + attr, &val);
> + TEST_ASSERT(ret == 0 || !check(gic, &sr[i], "write"), "%s unwritable", sr[i].name);
> + }
> +}
> +
> +static u8 get_ctlr_pribits(int gic)
> +{
> + int ret;
> + u64 val;
> + u8 pri;
> +
> + ret = __kvm_device_attr_get(gic, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + PACK_SR(SYS_ICC_CTLR_EL1), &val);
> + TEST_ASSERT(ret == 0, "ICC_CTLR_EL1 unreadable");
> +
> + pri = FIELD_GET(ICC_CTLR_EL1_PRI_BITS_MASK, val) + 1;
> + TEST_ASSERT(pri >= 5 && pri <= 7, "Bad pribits %d", pri);
> +
> + return pri;
> +}
> +
> +static int check_unaccessible_el1_regs(int gic, const struct sr_def *sr, const char *what)
> +{
> + switch (sr->encoding) {
> + case SYS_ICC_AP0R1_EL1:
> + case SYS_ICC_AP1R1_EL1:
> + if (get_ctlr_pribits(gic) >= 6)
> + return -EINVAL;
> + break;
> + case SYS_ICC_AP0R2_EL1:
> + case SYS_ICC_AP0R3_EL1:
> + case SYS_ICC_AP1R2_EL1:
> + case SYS_ICC_AP1R3_EL1:
> + if (get_ctlr_pribits(gic) == 7)
> + return 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + pr_info("SKIP %s for %s\n", sr->name, what);
> + return 0;
> +}
> +
> +static u8 get_vtr_pribits(int gic)
> +{
> + int ret;
> + u64 val;
> + u8 pri;
> +
> + ret = __kvm_device_attr_get(gic, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + PACK_SR(SYS_ICH_VTR_EL2), &val);
> + TEST_ASSERT(ret == 0, "ICH_VTR_EL2 unreadable");
> +
> + pri = FIELD_GET(ICH_VTR_EL2_PRIbits, val) + 1;
> + TEST_ASSERT(pri >= 5 && pri <= 7, "Bad pribits %d", pri);
> +
> + return pri;
> +}
> +
> +static int check_unaccessible_el2_regs(int gic, const struct sr_def *sr, const char *what)
> +{
> + switch (sr->encoding) {
> + case SYS_ICH_AP0R1_EL2:
> + case SYS_ICH_AP1R1_EL2:
> + if (get_vtr_pribits(gic) >= 6)
> + return -EINVAL;
> + break;
> + case SYS_ICH_AP0R2_EL2:
> + case SYS_ICH_AP0R3_EL2:
> + case SYS_ICH_AP1R2_EL2:
> + case SYS_ICH_AP1R3_EL2:
> + if (get_vtr_pribits(gic) == 7)
> + return -EINVAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + pr_info("SKIP %s for %s\n", sr->name, what);
> + return 0;
> +}
> +
> +static void test_v3_sysregs(void)
> +{
> + struct kvm_vcpu_init init = {};
> + struct kvm_vcpu *vcpu;
> + struct kvm_vm *vm;
> + u32 feat = 0;
> + int gic;
> +
> + if (kvm_check_cap(KVM_CAP_ARM_EL2))
> + feat |= BIT(KVM_ARM_VCPU_HAS_EL2);
> +
> + vm = vm_create(1);
> +
> + vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
> + init.features[0] |= feat;
> +
> + vcpu = aarch64_vcpu_add(vm, 0, &init, NULL);
> + TEST_ASSERT(vcpu, "Can't create a vcpu?");
> +
> + gic = kvm_create_device(vm, KVM_DEV_TYPE_ARM_VGIC_V3);
> + TEST_ASSERT(gic >= 0, "No GIC???");
> +
> + kvm_device_attr_set(gic, KVM_DEV_ARM_VGIC_GRP_CTRL,
> + KVM_DEV_ARM_VGIC_CTRL_INIT, NULL);
> +
> + test_sysreg_array(gic, sysregs_el1, ARRAY_SIZE(sysregs_el1), check_unaccessible_el1_regs);
> + if (feat)
> + test_sysreg_array(gic, sysregs_el2, ARRAY_SIZE(sysregs_el2), check_unaccessible_el2_regs);
> + else
> + pr_info("SKIP EL2 registers, not available\n");
> +
> + close(gic);
> + kvm_vm_free(vm);
> +}
> +
> void run_tests(uint32_t gic_dev_type)
> {
> test_vcpus_then_vgic(gic_dev_type);
> @@ -730,6 +944,7 @@ void run_tests(uint32_t gic_dev_type)
> test_v3_last_bit_single_rdist();
> test_v3_redist_ipa_range_check_at_vcpu_run();
> test_v3_its_region();
> + test_v3_sysregs();
> }
> }
>
> --
> 2.39.2
>
next prev parent reply other threads:[~2025-07-23 5:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-18 11:11 [PATCH 0/4] KVM: arm64: Userspace GICv3 sysreg access fixes and testing Marc Zyngier
2025-07-18 11:11 ` [PATCH 1/4] KVM: arm64: vgic-v3: Fix ordering of ICH_HCR_EL2 Marc Zyngier
2025-07-23 15:38 ` Sebastian Ott
2025-07-18 11:11 ` [PATCH 2/4] KVM: arm64: Clarify the check for reset callback in check_sysreg_table() Marc Zyngier
2025-07-23 15:38 ` Sebastian Ott
2025-07-18 11:11 ` [PATCH 3/4] KVM: arm64: Enforce the sorting of the GICv3 system register table Marc Zyngier
2025-07-23 15:40 ` Sebastian Ott
2025-07-18 11:11 ` [PATCH 4/4] KVM: arm64: selftest: vgic-v3: Add basic GICv3 sysreg userspace access test Marc Zyngier
2025-07-23 5:00 ` Itaru Kitayama [this message]
2025-07-23 8:15 ` Marc Zyngier
2025-07-23 8:46 ` Itaru Kitayama
2025-07-23 15:56 ` Sebastian Ott
2025-07-28 17:15 ` [PATCH 0/4] KVM: arm64: Userspace GICv3 sysreg access fixes and testing Oliver Upton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aIBseJ3aO+hMVAee@vm4 \
--to=itaru.kitayama@linux.dev \
--cc=eric.auger@redhat.com \
--cc=joey.gouly@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=suzuki.poulose@arm.com \
--cc=yuzenghui@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).