Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 24/10/2023 10:12, Jan Beulich wrote:
> 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.
> 

Well, I don't know. It's up to the community to decide that. It can be 
an ad-hoc
justification, but I don't see much value in doing so. What do you 
propose to get this patch
approved (at least on your account)?.

>>>> --- 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

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