Re: [XEN PATCH v2 07/13] x86/hvm: address violations of MISRA C Rule 16.3

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 24 Jun 2024, Federico Serafini wrote:
> MISRA C Rule 16.3 states that "An unconditional `break' statement shall
> terminate every switch-clause".
> 
> Add pseudo keyword fallthrough or missing break statement
> to address violations of the rule.
> 
> As a defensive measure, return -EOPNOTSUPP in case an unreachable
> return statement is reached.
> 
> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> ---
> Changes in v2:
> - replace hypened fallthrough with the pseudo keyword.
> ---
>  xen/arch/x86/hvm/emulate.c   | 9 ++++++---
>  xen/arch/x86/hvm/hvm.c       | 6 ++++++
>  xen/arch/x86/hvm/hypercall.c | 1 +
>  xen/arch/x86/hvm/irq.c       | 1 +
>  4 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 02e378365b..859c21a5ab 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -339,7 +339,7 @@ static int hvmemul_do_io(
>      }
>      case X86EMUL_UNIMPLEMENTED:
>          ASSERT_UNREACHABLE();
> -        /* Fall-through */
> +        fallthrough;
>      default:
>          BUG();
>      }
> @@ -2674,6 +2674,7 @@ static int _hvm_emulate_one(struct hvm_emulate_ctxt 
> *hvmemul_ctxt,
>  
>      default:
>          ASSERT_UNREACHABLE();
> +        break;

same here


>      }
>  
>      if ( hvmemul_ctxt->ctxt.retire.singlestep )
> @@ -2764,6 +2765,7 @@ int hvm_emulate_one_mmio(unsigned long mfn, unsigned 
> long gla)
>          /* fallthrough */
>      default:
>          hvm_emulate_writeback(&ctxt);
> +        break;
>      }
>  
>      return rc;
> @@ -2799,10 +2801,11 @@ void hvm_emulate_one_vm_event(enum emul_kind kind, 
> unsigned int trapnr,
>          memcpy(hvio->mmio_insn, curr->arch.vm_event->emul.insn.data,
>                 hvio->mmio_insn_bytes);
>      }
> -    /* Fall-through */
> +    fallthrough;
>      default:
>          ctx.set_context = (kind == EMUL_KIND_SET_CONTEXT_DATA);
>          rc = hvm_emulate_one(&ctx, VIO_no_completion);
> +        break;
>      }
>  
>      switch ( rc )
> @@ -2818,7 +2821,7 @@ void hvm_emulate_one_vm_event(enum emul_kind kind, 
> unsigned int trapnr,
>      case X86EMUL_UNIMPLEMENTED:
>          if ( hvm_monitor_emul_unimplemented() )
>              return;
> -        /* fall-through */
> +        fallthrough;
>      case X86EMUL_UNHANDLEABLE:
>          hvm_dump_emulation_state(XENLOG_G_DEBUG, "Mem event", &ctx, rc);
>          hvm_inject_hw_exception(trapnr, errcode);
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 7f4b627b1f..c263e562ff 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4919,6 +4919,8 @@ static int do_altp2m_op(
>  
>      default:
>          ASSERT_UNREACHABLE();
> +        rc = -EOPNOTSUPP;
> +        break;

same here


>      }
>  
>   out:
> @@ -5020,6 +5022,8 @@ static int compat_altp2m_op(
>  
>      default:
>          ASSERT_UNREACHABLE();
> +        rc = -EOPNOTSUPP;
> +        break;

same here


>      }
>  
>      return rc;
> @@ -5283,6 +5287,8 @@ void hvm_get_segment_register(struct vcpu *v, enum 
> x86_segment seg,
>           * %cs and %tr are unconditionally present.  SVM ignores these 
> present
>           * bits and will happily run without them set.
>           */
> +        fallthrough;
> +
>      case x86_seg_cs:
>          reg->p = 1;
>          break;
> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
> index 7fb3136f0c..2271afe02a 100644
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -111,6 +111,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>      case 8:
>          eax = regs->rax;
>          /* Fallthrough to permission check. */
> +        fallthrough;
>      case 4:
>      case 2:
>          if ( currd->arch.monitor.guest_request_userspace_enabled &&
> diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> index 210cebb0e6..1eab44defc 100644
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -282,6 +282,7 @@ static void hvm_set_callback_irq_level(struct vcpu *v)
>              __hvm_pci_intx_assert(d, pdev, pintx);
>          else
>              __hvm_pci_intx_deassert(d, pdev, pintx);
> +        break;
>      default:
>          break;
>      }
> -- 
> 2.34.1
> 
>