[PATCH 1/2] x86/mm: address Misra C:2012 rule 16.2

[PATCH 1/2] x86/mm: address Misra C:2012 rule 16.2

[Jan Beulich]

... ("A switch label shall only be used when the most closely-enclosing
compound statement is the body of a `switch' statement"). Use a form of
fall-through instead. No difference in generated code, except for some
line number changes.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2663,15 +2663,17 @@ static int validate_page(struct page_inf
                  get_gpfn_from_mfn(mfn_x(page_to_mfn(page))),
                  type, page->count_info, page->u.inuse.type_info);
         if ( page != current->arch.old_guest_table )
-            page->u.inuse.type_info = 0;
-        else
         {
-            ASSERT((page->u.inuse.type_info &
-                    (PGT_count_mask | PGT_validated)) == 1);
-    case -ERESTART:
-            get_page_light(page);
-            page->u.inuse.type_info |= PGT_partial;
+            page->u.inuse.type_info = 0;
+            break;
         }
+
+        ASSERT((page->u.inuse.type_info &
+                (PGT_count_mask | PGT_validated)) == 1);
+        fallthrough;
+    case -ERESTART:
+        get_page_light(page);
+        page->u.inuse.type_info |= PGT_partial;
         break;
     }
 

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

[Roger Pau Monné]

On Wed, May 13, 2026 at 04:05:55PM +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"). Use a form of
> fall-through instead. No difference in generated code, except for some
> line number changes.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

Thanks, Roger.

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

[Roger Pau Monné]

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.

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

[Jan Beulich]

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@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

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