Re: [PATCH 6/8] KVM: arm64: Remove dead PMU sysreg decoding code

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Marc,

I checked and indeed the remaining cases cover all registers that use this accessor.

However, I'm a bit torn here. The warning that I got when trying to run a guest
with the PMU feature flag set, but not initialized (reported at [1]) was also not
supposed to ever be reached:

static u32 kvm_pmu_event_mask(struct kvm *kvm)
{
    switch (kvm->arch.pmuver) {
    case 1:            /* ARMv8.0 */
        return GENMASK(9, 0);
    case 4:            /* ARMv8.1 */
    case 5:            /* ARMv8.4 */
    case 6:            /* ARMv8.5 */
        return GENMASK(15, 0);
    default:        /* Shouldn't be here, just for sanity */
        WARN_ONCE(1, "Unknown PMU version %d\n", kvm->arch.pmuver);
        return 0;
    }
}

I realize it's not exactly the same thing and I'll leave it up to you if you want
to add a warning for the cases that should never happen. I'm fine either way:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

[1] https://www.spinics.net/lists/arm-kernel/msg857927.html

On 11/13/20 6:26 PM, Marc Zyngier wrote:
> The handling of traps in access_pmu_evcntr() has a couple of
> omminous "else return false;" statements that don't make any sense:
> the decoding tree coverse all the registers that trap to this handler,
> and returning false implies that we change PC, which we don't.
>
> Get rid of what is evidently dead code.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 3bd4cc40536b..f878d71484d8 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -733,8 +733,6 @@ static bool access_pmu_evcntr(struct kvm_vcpu *vcpu,
>  				return false;
>  
>  			idx = ARMV8_PMU_CYCLE_IDX;
> -		} else {
> -			return false;
>  		}
>  	} else if (r->CRn == 0 && r->CRm == 9) {
>  		/* PMCCNTR */
> @@ -748,8 +746,6 @@ static bool access_pmu_evcntr(struct kvm_vcpu *vcpu,
>  			return false;
>  
>  		idx = ((r->CRm & 3) << 3) | (r->Op2 & 7);
> -	} else {
> -		return false;
>  	}
>  
>  	if (!pmu_counter_idx_valid(vcpu, idx))
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm