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