1. Patchew
  2. Xen
  3. [XEN PATCH][for-4.19 v4 0/8] Fix or deviate various instances of missing declarations
  4. View patch

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

Nicola Vetrini posted 8 patches 2 years, 3 months ago
There is a newer version of this series
[XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Nicola Vetrini 2 years, 3 months ago 
To avoid a violation of MISRA C:2012 Rule 8.4, as permitted
by docs/misra/rules.rst.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
Changes in v3:
- Edited commit message
- Add two new variables
---
 xen/arch/x86/include/asm/asm_defns.h | 1 +
 xen/arch/x86/setup.c                 | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

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..4480f08de718 100644
--- 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 */
 
 /* Used by the BSP/AP paths to find the higher half stack mapping to use. */
 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
Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Jan Beulich 2 years, 3 months ago 
On 23.10.2023 11:56, Nicola Vetrini wrote:
> To avoid a violation of MISRA C:2012 Rule 8.4, as permitted
> by docs/misra/rules.rst.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> Changes in v3:
> - Edited commit message
> - Add two new variables

Oh, also: The R-b dates back to v1. Imo any addition invalidates prior
R-b, unless indicated otherwise by the person offering it (which isn't
the case here afaict).

Jan
Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Jan Beulich 2 years, 3 months ago 
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.

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

Jan
Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Nicola Vetrini 2 years, 3 months ago 
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.

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

On the same line is fine as well. I personally found it less clear 
putting that in the
line above.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Jan Beulich 2 years, 3 months ago 
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
Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Nicola Vetrini 2 years, 3 months ago 
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)
Re: [XEN PATCH][for-4.19 v4 2/8] x86: add deviations for variables only used in asm code
Posted by Jan Beulich 2 years, 3 months ago 
On 24.10.2023 15:40, Nicola Vetrini wrote:
> 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)?.

Drop this change and have Eclair recognize that what we're talking
about here is just a declaration, not a definition.

Jan

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.