Re: [PATCH 11/21] target/arm/kvm: Use a switch for kvm_arm_cpreg_level

[Gavin Shan]

On 11/23/23 15:42, Richard Henderson wrote:
> Use a switch instead of a linear search through data.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/kvm64.c | 32 +++++++++-----------------------
>   1 file changed, 9 insertions(+), 23 deletions(-)

With the following nits addressed:

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 504526b24c..61fb9dbde0 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -361,32 +361,18 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
>       }
>   }
>   
> -typedef struct CPRegStateLevel {
> -    uint64_t regidx;
> -    int level;
> -} CPRegStateLevel;
> -
> -/* All system registers not listed in the following table are assumed to be
> - * of the level KVM_PUT_RUNTIME_STATE. If a register should be written less
> - * often, you must add it to this table with a state of either
> - * KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
> - */
> -static const CPRegStateLevel non_runtime_cpregs[] = {
> -    { KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
> -    { KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
> -};
> -
>   int kvm_arm_cpreg_level(uint64_t regidx)
>   {
> -    int i;
> -
> -    for (i = 0; i < ARRAY_SIZE(non_runtime_cpregs); i++) {
> -        const CPRegStateLevel *l = &non_runtime_cpregs[i];
> -        if (l->regidx == regidx) {
> -            return l->level;
> -        }
> +    /*
> +     * All system registers are assumed to be level KVM_PUT_RUNTIME_STATE.
> +     * If a register should be written less often, you must add it here
> +     * with a state of either KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
> +     */
> +    switch (regidx) {
> +    case KVM_REG_ARM_TIMER_CNT:
> +    case KVM_REG_ARM_PTIMER_CNT:
> +        return KVM_PUT_FULL_STATE;
>       }
> -
  ^^^^

It is unrelated change and needs to be dropped?

>       return KVM_PUT_RUNTIME_STATE;
>   }
>   

Thanks,
Gavin