Re: [PATCH v9 10/22] KVM: arm64: Add Partitioned PMU register trap handlers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] arm64: range-check over enum vcpu_sysreg
--

commit 35ffc169e71bb4303b4d3e6f955bbc06a22fd20d
Author: Colton Lewis <coltonlewis@google.com>

KVM: arm64: Add Partitioned PMU register trap handlers

This commit adds handling for partitioned PMU register accesses that are 
trapped when FEAT_FGT is unavailable. It passes through or filters accesses 
to registers like PMCR_EL0, PMEVCNTR, and PMCCFILTR based on the partitioning 
configuration.

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f7f750b7a91c4..c8210a8b10389 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
[ ... ]
> @@ -1092,9 +1092,204 @@ static u64 reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  	return __vcpu_sys_reg(vcpu, r->reg);
>  }
>  
> +/**
> + * pmu_reg_write() - Register writes for Partitioned PMU
[ ... ]
> +static void pmu_reg_write(struct kvm_vcpu *vcpu, enum vcpu_sysreg reg, u64 val, bool set)
> +{
[ ... ]
> +	switch (reg) {
[ ... ]
> +	case PMEVCNTR0_EL0 ... PMCCNTR_EL0:
> +		idx = reg - PMEVCNTR0_EL0;

[Severity: Low]
Is it safe to use a numeric range and arithmetic over enum vcpu_sysreg values
here in pmu_reg_write()? Since the enum indexes sparse VNCR-mapped registers
by byte offset rather than declaration order, range checks and arithmetic
can cover unintended registers. Should this use an explicit per-register
allowlist instead?

> +
> +		if (kvm_pmu_is_partitioned(vcpu->kvm)) {
[ ... ]
> +	case PMEVTYPER0_EL0 ... PMCCFILTR_EL0:
> +		idx = reg - PMEVTYPER0_EL0;

[Severity: Low]
Does this range check and subtraction also rely on contiguous enum values?
Like the previous case, should this be converted to an explicit per-register
match to avoid issues with the sparse enum numbering?

[ ... ]
> @@ -1276,18 +1471,21 @@ static bool access_pmu_evcntr(struct kvm_vcpu *vcpu,
>  			      struct sys_reg_params *p,
>  			      const struct sys_reg_desc *r)
>  {
[ ... ]
>  	if (!pmu_counter_idx_valid(vcpu, idx))
>  		return false;
>  
> +	reg = PMEVCNTR0_EL0 + idx;
> +

[Severity: Low]
Does this arithmetic over the enum value in access_pmu_evcntr() assume a
contiguous layout? Using addition on enum vcpu_sysreg values might point to 
an unintended register due to the sparse byte-offset mapping. Should this 
be explicitly mapped instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924172928.2110956-1-coltonlewis@google.com?part=10