On 31.07.2025 17:37, Andrew Cooper wrote: > On 31/07/2025 4:16 pm, Dmytro Prokopchuk1 wrote: >> MISRA Rule 13.1: Initializer lists shall not contain persistent side >> effects. >> >> The violations occur because both the `GVA_INFO` and `TRACE_TIME` macro >> expansions include expressions with persistent side effects introduced >> via inline assembly. >> >> In the case of `GVA_INFO`, the issue stems from the initializer list >> containing a direct call to `current`, which evaluates to >> `this_cpu(curr_vcpu)` and involves persistent side effects via the >> `asm` statement. To resolve this, the side-effect-producing expression >> is computed in a separate statement prior to the macro initialization: >> >> struct vcpu *current_vcpu = current; >> >> The computed value is passed into the `GVA_INFO(current_vcpu)` macro, >> ensuring that the initializer is clean and free of such side effects. >> >> Similarly, the `TRACE_TIME` macro violates this rule when accessing >> expressions like `current->vcpu_id` and `current->domain->domain_id`, >> which also depend on `current` and inline assembly. To fix this, the >> value of `current` is assigned to a temporary variable: >> >> struct vcpu *v = current; >> >> This temporary variable is then used to access `domain_id` and `vcpu_id`. >> This ensures that the arguments passed to the `TRACE_TIME` macro are >> simple expressions free of persistent side effects. >> >> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopch...@epam.com> > > The macro `current` specifically does not (and must not) have side > effects. It is expected to behave like a plain `struct vcpu *current;` > variable, and what Eclair is noticing is the thread-local machinery > under this_cpu() (or in x86's case, get_current()). > > In ARM's case, it's literally reading the hardware thread pointer > register. Can anything be done to tell Eclair that `this_cpu()` > specifically does not have side effects? > > The only reason that GVA_INFO() and TRACE_TIME() are picked out is > because they both contain embedded structure initialisation, and this is > is actually an example where trying to comply with MISRA interferes with > what is otherwise a standard pattern in Xen.
Irrespective of what you say, some of the changes here were eliminating multiple adjacent uses of current, which - iirc - often the compiler can't fold via CSE. Jan
