1. Patchew
  2. Xen
  3. [XEN PATCH 0/3] x86: address violations of MISRA C Rule 16.3
  4. View patch

[XEN PATCH 2/3] x86/emul: use pseudo keyword fallthrough

Federico Serafini posted 3 patches 2 months, 4 weeks ago
There is a newer version of this series
[XEN PATCH 2/3] x86/emul: use pseudo keyword fallthrough
Posted by Federico Serafini 2 months, 4 weeks ago 
Make explicit the fallthrough intetion by adding the pseudo keyword
where missing and refactor comments not following the agreed syntax.

This satisfies the requirements to deviate violations of
MISRA C:2012 Rule 16.3 "An unconditional break statement shall
terminate every switch-clause".

No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/x86/x86_emulate/decode.c      | 6 ++++--
 xen/arch/x86/x86_emulate/x86_emulate.c | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c
index 32b9276dc5..0a0751f2ed 100644
--- a/xen/arch/x86/x86_emulate/decode.c
+++ b/xen/arch/x86/x86_emulate/decode.c
@@ -1356,7 +1356,8 @@ int x86emul_decode(struct x86_emulate_state *s,
                         --disp8scale;
                     break;
                 }
-                /* vcvt{,t}s{s,d}2usi need special casing: fall through */
+                /* vcvt{,t}s{s,d}2usi need special casing. */
+                fallthrough;
             case 0x2c: /* vcvtts{s,d}2si need special casing */
             case 0x2d: /* vcvts{s,d}2si need special casing */
                 if ( evex_encoded() )
@@ -1530,7 +1531,8 @@ int x86emul_decode(struct x86_emulate_state *s,
                         disp8scale -= 1 + (s->evex.pfx == vex_66);
                     break;
                 }
-                /* vcvt{,t}sh2usi needs special casing: fall through */
+                /* vcvt{,t}sh2usi needs special casing. */
+                fallthrough;
             case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */
                 disp8scale = 1;
                 break;
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 30674ec301..c38984b201 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1460,6 +1460,7 @@ x86_emulate(
 
         d = (d & ~DstMask) | DstMem;
         /* Becomes a normal DstMem operation from here on. */
+        fallthrough;
     case DstMem:
         generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
         if ( state->simd_size != simd_none )
@@ -1942,6 +1943,7 @@ x86_emulate(
             break;
         }
         generate_exception_if((modrm_reg & 7) != 0, X86_EXC_UD);
+        fallthrough;
     case 0x88 ... 0x8b: /* mov */
     case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
     case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
-- 
2.43.0
Re: [XEN PATCH 2/3] x86/emul: use pseudo keyword fallthrough
Posted by Stefano Stabellini 2 months, 3 weeks ago 
On Wed, 6 Nov 2024, Federico Serafini wrote:
> Make explicit the fallthrough intetion by adding the pseudo keyword
> where missing and refactor comments not following the agreed syntax.
> 
> This satisfies the requirements to deviate violations of
> MISRA C:2012 Rule 16.3 "An unconditional break statement shall
> terminate every switch-clause".
> 
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/x86/x86_emulate/decode.c      | 6 ++++--
>  xen/arch/x86/x86_emulate/x86_emulate.c | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c
> index 32b9276dc5..0a0751f2ed 100644
> --- a/xen/arch/x86/x86_emulate/decode.c
> +++ b/xen/arch/x86/x86_emulate/decode.c
> @@ -1356,7 +1356,8 @@ int x86emul_decode(struct x86_emulate_state *s,
>                          --disp8scale;
>                      break;
>                  }
> -                /* vcvt{,t}s{s,d}2usi need special casing: fall through */
> +                /* vcvt{,t}s{s,d}2usi need special casing. */
> +                fallthrough;
>              case 0x2c: /* vcvtts{s,d}2si need special casing */
>              case 0x2d: /* vcvts{s,d}2si need special casing */
>                  if ( evex_encoded() )
> @@ -1530,7 +1531,8 @@ int x86emul_decode(struct x86_emulate_state *s,
>                          disp8scale -= 1 + (s->evex.pfx == vex_66);
>                      break;
>                  }
> -                /* vcvt{,t}sh2usi needs special casing: fall through */
> +                /* vcvt{,t}sh2usi needs special casing. */
> +                fallthrough;
>              case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */
>                  disp8scale = 1;
>                  break;
> diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
> index 30674ec301..c38984b201 100644
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1460,6 +1460,7 @@ x86_emulate(
>  
>          d = (d & ~DstMask) | DstMem;
>          /* Becomes a normal DstMem operation from here on. */
> +        fallthrough;
>      case DstMem:
>          generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
>          if ( state->simd_size != simd_none )
> @@ -1942,6 +1943,7 @@ x86_emulate(
>              break;
>          }
>          generate_exception_if((modrm_reg & 7) != 0, X86_EXC_UD);
> +        fallthrough;
>      case 0x88 ... 0x8b: /* mov */
>      case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
>      case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */
> -- 
> 2.43.0
>
Re: [XEN PATCH 2/3] x86/emul: use pseudo keyword fallthrough
Posted by Jan Beulich 2 months, 4 weeks ago 
On 06.11.2024 10:04, Federico Serafini wrote:
> Make explicit the fallthrough intetion by adding the pseudo keyword
> where missing and refactor comments not following the agreed syntax.
> 
> This satisfies the requirements to deviate violations of
> MISRA C:2012 Rule 16.3 "An unconditional break statement shall
> terminate every switch-clause".
> 
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

We can certainly take this as is, as a tiny first step. Personally I'm
not overly happy though to see a mix of comment- and pseudo-keyword-
style annotations in individual files. After all going forward we'll
want to use the latter, now that this becomes possible. Yet for that I
view it a more than just helpful if bad precedents didn't exist
anymore. Being the one to touch the emulator files most, I can say
that here more than perhaps anywhere else new code is very often added
by copy-and-paste-then-edit.

Therefore question in particular to the other x86 maintainers: Won't
we be better off if we fully convert to pseudo-keyword-style right
away?

Jan

> --- a/xen/arch/x86/x86_emulate/decode.c
> +++ b/xen/arch/x86/x86_emulate/decode.c
> @@ -1356,7 +1356,8 @@ int x86emul_decode(struct x86_emulate_state *s,
>                          --disp8scale;
>                      break;
>                  }
> -                /* vcvt{,t}s{s,d}2usi need special casing: fall through */
> +                /* vcvt{,t}s{s,d}2usi need special casing. */
> +                fallthrough;
>              case 0x2c: /* vcvtts{s,d}2si need special casing */
>              case 0x2d: /* vcvts{s,d}2si need special casing */
>                  if ( evex_encoded() )
> @@ -1530,7 +1531,8 @@ int x86emul_decode(struct x86_emulate_state *s,
>                          disp8scale -= 1 + (s->evex.pfx == vex_66);
>                      break;
>                  }
> -                /* vcvt{,t}sh2usi needs special casing: fall through */
> +                /* vcvt{,t}sh2usi needs special casing. */
> +                fallthrough;
>              case 0x2c: case 0x2d: /* vcvt{,t}sh2si need special casing */
>                  disp8scale = 1;
>                  break;
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1460,6 +1460,7 @@ x86_emulate(
>  
>          d = (d & ~DstMask) | DstMem;
>          /* Becomes a normal DstMem operation from here on. */
> +        fallthrough;
>      case DstMem:
>          generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
>          if ( state->simd_size != simd_none )
> @@ -1942,6 +1943,7 @@ x86_emulate(
>              break;
>          }
>          generate_exception_if((modrm_reg & 7) != 0, X86_EXC_UD);
> +        fallthrough;
>      case 0x88 ... 0x8b: /* mov */
>      case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */
>      case 0xa2 ... 0xa3: /* mov {%al,%ax,%eax,%rax},mem.offs */

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.