On 24.10.2023 09:58, Nicola Vetrini wrote:
> On 24/10/2023 09:32, Jan Beulich wrote:
>> On 23.10.2023 11:56, Nicola Vetrini wrote:
>>> --- a/xen/arch/x86/include/asm/asm_defns.h
>>> +++ b/xen/arch/x86/include/asm/asm_defns.h
>>> @@ -31,6 +31,7 @@ asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
>>> * gets set up by the containing function.
>>> */
>>> #ifdef CONFIG_FRAME_POINTER
>>> +/* SAF-1-safe */
>>> register unsigned long current_stack_pointer asm("rsp");
>>> # define ASM_CALL_CONSTRAINT , "+r" (current_stack_pointer)
>>> #else
>>
>> SAF-1-safe is about symbols "used only by asm modules". This doesn't
>> apply
>> to the declaration here.
>>
>
> The wording could change to "asm code" if that is deemed clearer.
Question is what would be meant by "asm code"; "asm modules" is quite
clear.
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -75,6 +75,7 @@ static bool __initdata opt_invpcid = true;
>>> boolean_param("invpcid", opt_invpcid);
>>> bool __read_mostly use_invpcid;
>>>
>>> +/* SAF-1-safe Only used in asm code and within this source file */
>>> unsigned long __read_mostly cr4_pv32_mask;
>>>
>>> /* **** Linux config option: propagated to domain0. */
>>> @@ -147,12 +148,13 @@ cpumask_t __read_mostly cpu_present_map;
>>> unsigned long __read_mostly xen_phys_start;
>>>
>>> char __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
>>> - cpu0_stack[STACK_SIZE];
>>> + cpu0_stack[STACK_SIZE]; /* SAF-1-safe Only used in asm code and
>>> below */
>>
>> Wasn't it that such comments need to live on the earlier line?
>
> On the same line is fine as well. I personally found it less clear
> putting that in the
> line above.
But please recall that these comments are intended to cover other
scanners as well. Iirc only Eclair accepts comments on the same line.
Nevertheless I realize that putting the comment on the earlier line
is problematic (and maybe also scanner dependent) when that ends up
in the middle of a declaration / definition.
Jan
