Re: [PATCH 2/2] x86/PV: address Misra C:2012 rule 16.2

["Roger Pau Monné" <roger.pau@citrix.com>]

On Wed, May 13, 2026 at 04:06:20PM +0200, Jan Beulich wrote:
> ... ("A switch label shall only be used when the most closely-enclosing
> compound statement is the body of a `switch' statement"). While I don't
> really like doing so, use a few "goto" instead. No change in generated
> code (somewhat to my surprise).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

With one alternative below if you would like to remove one of the
introduced labels.

> 
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -897,7 +897,7 @@ static int cf_check read_msr(
>      struct vcpu *curr = current;
>      const struct domain *currd = curr->domain;
>      const struct cpu_policy *cp = currd->arch.cpu_policy;
> -    bool vpmu_msr = false, warn = false;
> +    bool warn = false;
>      uint64_t tmp;
>      int ret;
>  
> @@ -996,21 +996,21 @@ static int cf_check read_msr(
>      case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
>      case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>          if ( boot_cpu_data.vendor == X86_VENDOR_INTEL )
> -        {
> -            vpmu_msr = true;
> -            /* fall through */
> +            goto vpmu;
> +        goto check_relaxed;
> +
>      case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5:
>      case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3:
> -            if ( vpmu_msr || (boot_cpu_data.vendor &
> -                              (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> -            {
> -                if ( vpmu_do_rdmsr(reg, val) )
> -                    break;
> -                return X86EMUL_OKAY;
> -            }
> +        if ( boot_cpu_data.vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
> +        {
> +    vpmu:
> +            if ( vpmu_do_rdmsr(reg, val) )
> +                break;
> +            return X86EMUL_OKAY;
>          }
>          /* fall through */
>      default:
> +    check_relaxed:

Not sure it's much better, but I think you could avoid the vpmu label
at the cost of keeping the vpmu_msr local variable:

    case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
    case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
        if ( boot_cpu_data.vendor != X86_VENDOR_INTEL )
            goto check_relaxed;
        vpmu_msr = true;
        fallthrough;

    case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5:
    case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3:
       if ( vpmu_msr || (boot_cpu_data.vendor &
                         (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )

Thanks, Roger.