Re: xen/x86: resolve the last 3 MISRA R16.6 violations

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 18 Feb 2025, Jan Beulich wrote:
> On 18.02.2025 00:12, Stefano Stabellini wrote:
> > On Mon, 17 Feb 2025, Jan Beulich wrote:
> >> On 15.02.2025 03:16, Stefano Stabellini wrote:
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -3797,22 +3797,14 @@ uint64_t hvm_get_reg(struct vcpu *v, unsigned int 
> >>> reg)
> >>>  {
> >>>      ASSERT(v == current || !vcpu_runnable(v));
> >>>  
> >>> -    switch ( reg )
> >>> -    {
> >>> -    default:
> >>> -        return alternative_call(hvm_funcs.get_reg, v, reg);
> >>> -    }
> >>> +    return alternative_call(hvm_funcs.get_reg, v, reg);
> >>>  }
> >>>  
> >>>  void hvm_set_reg(struct vcpu *v, unsigned int reg, uint64_t val)
> >>>  {
> >>>      ASSERT(v == current || !vcpu_runnable(v));
> >>>  
> >>> -    switch ( reg )
> >>> -    {
> >>> -    default:
> >>> -        return alternative_vcall(hvm_funcs.set_reg, v, reg, val);
> >>> -    }
> >>> +    return alternative_vcall(hvm_funcs.set_reg, v, reg, val);
> >>>  }
> >>
> >> Both of these were, iirc, deliberately written using switch(), to ease
> >> possible future changes.
> > 
> > To be honest, I do not see any value in the way they are currently
> > written. However, if you prefer, I can add a deviation for this, with
> > one SAF comment for each of these two. The reason for the deviation
> > would be "deliberate to ease possible future change". Please let me know
> > how you would like to proceed.
> 
> Well, best next thing you can do is seek input from the person who has
> written that code, i.e. Andrew.

Andrew wrote in chat that he is OK with a deviation and he can live with
a SAF deviation. Here is the patch.


---
xen/x86: resolve the last 3 MISRA R16.6 violations

MISRA R16.6 states that "Every switch statement shall have at least two
switch-clauses". There are only 3 violations left on x86 (zero on ARM).

One of them is only a violation depending on the kconfig configuration.
So deviate it instead with a SAF comment.

Two of them are deliberate to enable future additions. Deviate them as
such.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index b8a4f878ea..3d68b59169 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -92,6 +92,22 @@
         },
         {
             "id": "SAF-11-safe",
+            "analyser": {
+                "eclair": "MC3A2.R16.6"
+            },
+            "name": "Rule 16.6: single clause due to kconfig",
+            "text": "A switch statement with a single switch clause because 
other switch clauses are disabled in a given kconfig is safe."
+        },
+        {
+            "id": "SAF-12-safe",
+            "analyser": {
+                "eclair": "MC3A2.R16.6"
+            },
+            "name": "Rule 16.6: single clause due to future expansion",
+            "text": "A switch statement with a single switch clause to 
purposely enable future additions of new cases is safe."
+        },
+        {
+            "id": "SAF-13-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 39e39ce4ce..0f0630769b 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3797,6 +3797,7 @@ uint64_t hvm_get_reg(struct vcpu *v, unsigned int reg)
 {
     ASSERT(v == current || !vcpu_runnable(v));
 
+    /* SAF-12-safe */
     switch ( reg )
     {
     default:
@@ -3808,6 +3809,7 @@ void hvm_set_reg(struct vcpu *v, unsigned int reg, 
uint64_t val)
 {
     ASSERT(v == current || !vcpu_runnable(v));
 
+    /* SAF-12-safe */
     switch ( reg )
     {
     default:
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 87b30ce4df..dca11a613d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -436,6 +436,7 @@ unsigned long get_stack_trace_bottom(unsigned long sp)
 
 static unsigned long get_shstk_bottom(unsigned long sp)
 {
+    /* SAF-11-safe */
     switch ( get_stack_page(sp) )
     {
 #ifdef CONFIG_XEN_SHSTK