Re: [PATCH 3/4] xen/arm: domain: Fix MISRA C 2012 Rule 8.7 violation

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

Hi Jan,

On 7/6/22 11:51, Jan Beulich wrote:
> On 06.07.2022 10:43, Xenia Ragiadakou wrote:
> > On 7/6/22 10:10, Jan Beulich wrote:
> > > On 05.07.2022 23:02, Xenia Ragiadakou wrote:
> > > > The function idle_loop() is referenced only in domain.c.
> > > > Change its linkage from external to internal by adding the storage-class
> > > > specifier static to its definitions.
> > > >
> > > > Since idle_loop() is referenced only in inline assembly, add the 'used'
> > > > attribute to suppress unused-function compiler warning.
> > > While I see that Julien has already acked the patch, I'd like to point
> > > out that using __used here is somewhat bogus. Imo the better approach
> > > is to properly make visible to the compiler that the symbol is used by
> > > the asm(), by adding a fake(?) input.
> > I 'm afraid I do not understand what do you mean by "adding a fake(?)
> > input". Can you please elaborate a little on your suggestion?
> Once the asm() in question takes the function as an input, the compiler
> will know that the function has a user (unless, of course, it finds a
> way to elide the asm() itself). The "fake(?)" was because I'm not deeply
> enough into Arm inline assembly to know whether the input could then
> also be used as an instruction operand (which imo would be preferable) -
> if it can't (e.g. because there's no suitable constraint or operand
> modifier), it still can be an input just to inform the compiler.
According to the following statement, your approach is the recommended one:
"To prevent the compiler from removing global data or functions which 
are referenced from inline assembly statements, you can:
-use __attribute__((used)) with the global data or functions.
-pass the reference to global data or functions as operands to inline 
assembly statements.
Arm recommends passing the reference to global data or functions as 
operands to inline assembly statements so that if the final image does 
not require the inline assembly statements and the referenced global 
data or function, then they can be removed."
IIUC, you are suggesting to change
asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
into
asm volatile ("mov sp,%0; b %1" : : "r" (stack), "S" (fn) : "memory" );

This gives, respectively:
reset_stack_and_jump(idle_loop);

     460:       9100001f        mov     sp, x0

     464:       14000007        b       480 <idle_loop>


reset_stack_and_jump(idle_loop);

     460:       9100001f        mov     sp, x0

     464:       14000000        b       600 <idle_loop>


With this change, the functions return_to_new_vcpu32 and 
return_to_new_vcpu64, implemented in assembly and called in the same way 
as idle_loop(), need to be declared.
--
Xenia