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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.05.2026 12:49, Roger Pau Monné wrote:
> 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@xxxxxxxx>
> 
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

> 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)) )

I was actually happy to see this last construct go away, which your
variant would retain.

Jan