These variables are only used by asm code, and therefore
the lack of a declaration is justified by the corresponding
deviation comment.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/x86/include/asm/asm_defns.h | 1 +
 xen/arch/x86/setup.c                 | 1 +
 2 files changed, 2 insertions(+)

diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h
index baaaccb26e17..a2516de7749b 100644
--- 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
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 08ba1f95d635..7e2979f419af 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -153,6 +153,7 @@ char __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
 void *stack_start = cpu0_stack + STACK_SIZE - sizeof(struct cpu_info);

 /* Used by the boot asm to stash the relocated multiboot info pointer. */
+/* SAF-1-safe */
 unsigned int __initdata multiboot_ptr;

 struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
--
2.34.1

On 09.10.2023 08:54, Nicola Vetrini wrote:
> These variables are only used by asm code, and therefore
> the lack of a declaration is justified by the corresponding
> deviation comment.

Hmm, you say "declaration" here, but according to my understanding ...

> --- 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");

... this is a declaration, not a definition.

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -153,6 +153,7 @@ char __section(".init.bss.stack_aligned") __aligned(STACK_SIZE)
>  void *stack_start = cpu0_stack + STACK_SIZE - sizeof(struct cpu_info);
> 
>  /* Used by the boot asm to stash the relocated multiboot info pointer. */
> +/* SAF-1-safe */
>  unsigned int __initdata multiboot_ptr;

Imo such comments want folding; question is whether the tooling can
cope.

Jan

On 16/10/2023 16:58, Jan Beulich wrote:
> On 09.10.2023 08:54, Nicola Vetrini wrote:
>> These variables are only used by asm code, and therefore
>> the lack of a declaration is justified by the corresponding
>> deviation comment.
> 
> Hmm, you say "declaration" here, but according to my understanding ...
> 
>> --- 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");
> 
> ... this is a declaration, not a definition.
> 

It has automatic storage duration and it's not defined afterwards

>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -153,6 +153,7 @@ char __section(".init.bss.stack_aligned") 
>> __aligned(STACK_SIZE)
>>  void *stack_start = cpu0_stack + STACK_SIZE - sizeof(struct 
>> cpu_info);
>> 
>>  /* Used by the boot asm to stash the relocated multiboot info 
>> pointer. */
>> +/* SAF-1-safe */
>>  unsigned int __initdata multiboot_ptr;
> 
> Imo such comments want folding; question is whether the tooling can
> cope.
> 

As far as I know, it can't fold /* comment SAF-x-safe */, but /* 
SAF-x-safe comment */,
though the latter should be a justification, which this comment is not

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

On 18.10.2023 16:28, Nicola Vetrini wrote:
> On 16/10/2023 16:58, Jan Beulich wrote:
>> On 09.10.2023 08:54, Nicola Vetrini wrote:
>>> These variables are only used by asm code, and therefore
>>> the lack of a declaration is justified by the corresponding
>>> deviation comment.
>>
>> Hmm, you say "declaration" here, but according to my understanding ...
>>
>>> --- 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");
>>
>> ... this is a declaration, not a definition.
>>
> 
> It has automatic storage duration and it's not defined afterwards

Mind me asking what makes you derive "automatic storage duration"?
I also don't see how "not defined afterwards" matters here. This is
a special construct, not covered by the C standard.

Jan

On 18/10/2023 16:56, Jan Beulich wrote:
> On 18.10.2023 16:28, Nicola Vetrini wrote:
>> On 16/10/2023 16:58, Jan Beulich wrote:
>>> On 09.10.2023 08:54, Nicola Vetrini wrote:
>>>> These variables are only used by asm code, and therefore
>>>> the lack of a declaration is justified by the corresponding
>>>> deviation comment.
>>> 
>>> Hmm, you say "declaration" here, but according to my understanding 
>>> ...
>>> 
>>>> --- 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");
>>> 
>>> ... this is a declaration, not a definition.
>>> 
>> 
>> It has automatic storage duration and it's not defined afterwards
> 
> Mind me asking what makes you derive "automatic storage duration"?
> I also don't see how "not defined afterwards" matters here. This is
> a special construct, not covered by the C standard.
> 
> Jan

Oh, you're right. I was fooled by the fact that this is not a standard 
construct.
I see your point now.

Thanks,

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)