Re: [PATCH v5 31/49] KVM: arm64: gic-v5: Add GICv5 EL1 sysreg userspace accessors

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `gic_v5_icc_reg_descs` array is not sorted by sysreg encoding, which will cause KVM module initialization to fail.
--

--- Patch [31]: [PATCH v5 31/49] KVM: arm64: gic-v5: Add GICv5 EL1 sysreg userspace accessors ---
commit fc4bd92f943a451cf5400e9e01e75819d02ae623
Author: Sascha Bischoff <[email protected]>

KVM: arm64: gic-v5: Add GICv5 EL1 sysreg userspace accessors

This commit adds functions to handle userspace reads and writes of the GICv5
EL1 system registers to support VM migration. It wires these accessors into
KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS and enforces that register state can only
be written before a vCPU has run.

> diff --git a/arch/arm64/kvm/vgic-sys-reg-v5.c b/arch/arm64/kvm/vgic-sys-reg-v5.c
> new file mode 100644
> index 0000000000000..6b255eacbf60d
> --- /dev/null
> +++ b/arch/arm64/kvm/vgic-sys-reg-v5.c

[ ... ]

> +static const struct sys_reg_desc gic_v5_icc_reg_descs[] = {
> +	{ SYS_DESC(SYS_ICC_ICSR_EL1),
> +	  .set_user = set_gic_icsr, .get_user = get_gic_icsr, },
> +	{ SYS_DESC(SYS_ICC_PPI_ENABLER0_EL1),
> +	  .set_user = set_gic_ppi_enabler, .get_user = get_gic_ppi_enabler, },
> +	{ SYS_DESC(SYS_ICC_PPI_ENABLER1_EL1),
> +	  .set_user = set_gic_ppi_enabler, .get_user = get_gic_ppi_enabler, },

[ ... ]

> +	{ SYS_DESC(SYS_ICC_PPI_PRIORITYR14_EL1),
> +	  .set_user = set_gic_ppi_priorityr, .get_user = get_gic_ppi_priorityr, },
> +	{ SYS_DESC(SYS_ICC_PPI_PRIORITYR15_EL1),
> +	  .set_user = set_gic_ppi_priorityr, .get_user = get_gic_ppi_priorityr, },
> +	{ SYS_DESC(SYS_ICC_APR_EL1),
> +	  .set_user = set_gic_apr, .get_user = get_gic_apr, },
> +	{ SYS_DESC(SYS_ICC_CR0_EL1),
> +	  .set_user = set_gic_cr0, .get_user = get_gic_cr0, },
> +	{ SYS_DESC(SYS_ICC_PCR_EL1),
> +	  .set_user = set_gic_pcr, .get_user = get_gic_pcr, },
> +};

[Severity: High]
Will this cause KVM module initialization to fail?

During load, check_sysreg_table() verifies that the sys_reg_desc arrays are
strictly sorted by their sysreg encoding (Op0, Op1, CRn, CRm, Op2). If they
are out of order, check_sysreg_table() returns false and causes the init to
fail with -EINVAL.

Here, SYS_ICC_APR_EL1, SYS_ICC_CR0_EL1, and SYS_ICC_PCR_EL1 (which all have
CRm=0) are placed at the end of the array after SYS_ICC_PPI_PRIORITYR15_EL1
(which has CRm=9). Additionally, SYS_ICC_ICSR_EL1 is placed at the very
beginning of the array.

Does this array need to be reordered strictly by encoding so the validation
checks pass and the binary searches work correctly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=31