Re: [XEN PATCH 5/7] xen/arm: traps: add ASSERT_UNREACHABLE() where needed

[Julien Grall <julien@xxxxxxx>]

Hi,

On 15/12/2023 14:08, Nicola Vetrini wrote:
> > > Yes, I would go with 3., replace advance_pc with domain_crash. Assuming
> > > that it would also solve the violation in ECLAIR.
> > It needs to be prefixed with an ASSERT_UNREACHABLE(), though, because 
> > it's still a violation if there is no execution path leading to 
> > domain_crash(), but other than that it seems the safest choice.
> Assuming there are no objections to going forward with this proposal, 
> would you mind telling me how can I do the proper domain_crash call. 
> Most of the examples get a "struct domain *" from a parameter or from 
> the macro "current", so I was thinking of
> domain_crash(current->domain);
>
> but I'm not so sure about this, as there are no other uses in vcpreg.c. 
That would be correct. All the helpers in vcpreg.c are meant to work on 
the current vCPU.
> You can also submit the patch yourself, if you prefer.
I am not entirely sure about which justification you want to use for 
MISRA. So here the diff:
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index 39aeda9dab62..485b3cb63c86 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -708,7 +708,13 @@ void do_cp10(struct cpu_user_regs *regs, const 
union hsr hsr)
         return;
     }

-    advance_pc(regs, hsr);
+    /*
+     * All the cases in the switch should return. If this is not the
+     * case, then something went wrong and it is best to crash the
+     * domain.
+     */
+    ASSERT_UNREACHABLE();
+    domain_crash(current->domain);
 }

 void do_cp(struct cpu_user_regs *regs, const union hsr hsr)

Cheers,

--
Julien Grall