[PATCH] generic/altp2m: address violations of MISRA C Rule 2.1

Dmytro Prokopchuk1 posted 1 patch 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/8b5ca213a097f7b221b06b6173e4621563779a73.1775752149.git.dmytro._5Fprokopchuk1@epam.com
xen/include/asm-generic/altp2m.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
[PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Dmytro Prokopchuk1 3 months ago
MISRA C Rule 2.1 states: "A project shall not contain unreachable code".

In certain build configurations, the function 'altp2m_vcpu_idx()' is defined
as an inline function that contains the 'BUG()' macro. This resulted in a
violation because the 'BUG()' macro makes the function non-returning.

To ensure compliance with MISRA C Rule 2.1, this patch removes the inline
function implementation and its BUG()-based unreachable code. It is replaced
with an unconditional function declaration for 'altp2m_vcpu_idx()'. It relies
on the compiler's Dead Code Elimination (DCE) to remove the unused function
in builds where it is not needed.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2441424553
---
 xen/include/asm-generic/altp2m.h | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/xen/include/asm-generic/altp2m.h b/xen/include/asm-generic/altp2m.h
index 39865a842a..df6b9a9c01 100644
--- a/xen/include/asm-generic/altp2m.h
+++ b/xen/include/asm-generic/altp2m.h
@@ -15,12 +15,7 @@ static inline bool altp2m_active(const struct domain *d)
 }
 
 /* Alternate p2m VCPU */
-static inline unsigned int altp2m_vcpu_idx(const struct vcpu *v)
-{
-    /* Not implemented on GENERIC, should not be reached. */
-    BUG();
-    return 0;
-}
+uint16_t altp2m_vcpu_idx(const struct vcpu *v);
 
 #endif /* __ASM_GENERIC_ALTP2M_H */
 
-- 
2.43.0
Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Jan Beulich 3 months ago
On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
> 
> In certain build configurations,

Can you give an example where ...

> the function 'altp2m_vcpu_idx()' is defined
> as an inline function that contains the 'BUG()' macro. This resulted in a
> violation because the 'BUG()' macro makes the function non-returning.
> 
> To ensure compliance with MISRA C Rule 2.1, this patch removes the inline
> function implementation and its BUG()-based unreachable code. It is replaced
> with an unconditional function declaration for 'altp2m_vcpu_idx()'.

... a declaration is needed? The sole non-x86 reference I see is from
common/monitor.c, and the sole relevant Kconfig option I can spot is
VM_EVENT. When that's off, the file won't be built at all.

Further, BUG() and a few more constructs have a dedicated deviation
already in place. I don't mind a useless function to be shrunk (or, as
per above, perhaps even dropped), but the justification then needs to
be different.

Jan
Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Dmytro Prokopchuk1 2 weeks, 6 days ago
Hello, Jan

On 4/10/26 09:04, Jan Beulich wrote:
> On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
>>
>> In certain build configurations,
> 
> Can you give an example where ...
If CONFIG_VM_EVENT is enabled on ARM.
> 
>> the function 'altp2m_vcpu_idx()' is defined
>> as an inline function that contains the 'BUG()' macro. This resulted in a
>> violation because the 'BUG()' macro makes the function non-returning.
>>
>> To ensure compliance with MISRA C Rule 2.1, this patch removes the inline
>> function implementation and its BUG()-based unreachable code. It is replaced
>> with an unconditional function declaration for 'altp2m_vcpu_idx()'.
> 
> ... a declaration is needed? The sole non-x86 reference I see is from
Yes, declaration is needed. The file 'common/monitor.c' contains a usage 
of the 'altp2m_vcpu_idx()' and compiler must see it to avoid "error: 
implicit declaration of function ‘altp2m_vcpu_idx’".
> common/monitor.c, and the sole relevant Kconfig option I can spot is
> VM_EVENT. When that's off, the file won't be built at all.
But when 'CONFIG_VM_EVENT=y' this file is compiled on ARM.
> 
> Further, BUG() and a few more constructs have a dedicated deviation
> already in place. I don't mind a useless function to be shrunk (or, as
> per above, perhaps even dropped), but the justification then needs to
> be different.
Well, with Stefano's comment regarding return type, the commit subject 
and message could be rewritten like:

generic/altp2m: align and simplify altp2m_vcpu_idx()

The return type of 'altp2m_vcpu_idx()' in the generic altp2m.h header is
currently 'unsigned int', which is inconsistent with its 'uint16_t'
return type on x86 and the 'altp2m_idx' member of the monitor structures.

To fix this type inconsistency and simplify the header, this patch
replaces the static inline implementation of 'altp2m_vcpu_idx()' 
(contained a BUG() stub) with a simple function declaration returning
uint16_t.

For architectures using the generic altp2m.h header (such as ARM when
CONFIG_VM_EVENT is enabled), common code calls to 'altp2m_vcpu_idx()' in 
common/monitor.c are guarded by 'altp2m_active()', which
statically returns 'false'. The compiler's DCE will optimize out these 
calls, avoiding any linker issues for the missing definition.

> 
> Jan

BR, Dmytro.
Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Dmytro Prokopchuk1 3 days, 17 hours ago

On 6/22/26 13:29, Dmytro Prokopchuk wrote:
> Hello, Jan
> 
> On 4/10/26 09:04, Jan Beulich wrote:
>> On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
>>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
>>>
>>> In certain build configurations,
>>
>> Can you give an example where ...
> If CONFIG_VM_EVENT is enabled on ARM.
>>
>>> the function 'altp2m_vcpu_idx()' is defined
>>> as an inline function that contains the 'BUG()' macro. This resulted 
>>> in a
>>> violation because the 'BUG()' macro makes the function non-returning.
>>>
>>> To ensure compliance with MISRA C Rule 2.1, this patch removes the 
>>> inline
>>> function implementation and its BUG()-based unreachable code. It is 
>>> replaced
>>> with an unconditional function declaration for 'altp2m_vcpu_idx()'.
>>
>> ... a declaration is needed? The sole non-x86 reference I see is from
> Yes, declaration is needed. The file 'common/monitor.c' contains a usage 
> of the 'altp2m_vcpu_idx()' and compiler must see it to avoid "error: 
> implicit declaration of function ‘altp2m_vcpu_idx’".
>> common/monitor.c, and the sole relevant Kconfig option I can spot is
>> VM_EVENT. When that's off, the file won't be built at all.
> But when 'CONFIG_VM_EVENT=y' this file is compiled on ARM.
>>
>> Further, BUG() and a few more constructs have a dedicated deviation
>> already in place. I don't mind a useless function to be shrunk (or, as
>> per above, perhaps even dropped), but the justification then needs to
>> be different.
> Well, with Stefano's comment regarding return type, the commit subject 
> and message could be rewritten like:
> 
> generic/altp2m: align and simplify altp2m_vcpu_idx()
> 
> The return type of 'altp2m_vcpu_idx()' in the generic altp2m.h header is
> currently 'unsigned int', which is inconsistent with its 'uint16_t'
> return type on x86 and the 'altp2m_idx' member of the monitor structures.
> 
> To fix this type inconsistency and simplify the header, this patch
> replaces the static inline implementation of 
> 'altp2m_vcpu_idx()' (contained a BUG() stub) with a simple function 
> declaration returning
> uint16_t.
> 
> For architectures using the generic altp2m.h header (such as ARM when
> CONFIG_VM_EVENT is enabled), common code calls to 'altp2m_vcpu_idx()' in 
> common/monitor.c are guarded by 'altp2m_active()', which
> statically returns 'false'. The compiler's DCE will optimize out these 
> calls, avoiding any linker issues for the missing definition.
> 
>>
>> Jan
> 
> BR, Dmytro.

Hello Jan,

kind reminder. Please, take a look.
Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Jan Beulich 3 days, 16 hours ago
On 09.07.2026 12:00, Dmytro Prokopchuk1 wrote:
> On 6/22/26 13:29, Dmytro Prokopchuk wrote:
>> On 4/10/26 09:04, Jan Beulich wrote:
>>> On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
>>>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
>>>>
>>>> In certain build configurations,
>>>
>>> Can you give an example where ...
>> If CONFIG_VM_EVENT is enabled on ARM.
>>>
>>>> the function 'altp2m_vcpu_idx()' is defined
>>>> as an inline function that contains the 'BUG()' macro. This resulted 
>>>> in a
>>>> violation because the 'BUG()' macro makes the function non-returning.
>>>>
>>>> To ensure compliance with MISRA C Rule 2.1, this patch removes the 
>>>> inline
>>>> function implementation and its BUG()-based unreachable code. It is 
>>>> replaced
>>>> with an unconditional function declaration for 'altp2m_vcpu_idx()'.
>>>
>>> ... a declaration is needed? The sole non-x86 reference I see is from
>> Yes, declaration is needed. The file 'common/monitor.c' contains a usage 
>> of the 'altp2m_vcpu_idx()' and compiler must see it to avoid "error: 
>> implicit declaration of function ‘altp2m_vcpu_idx’".
>>> common/monitor.c, and the sole relevant Kconfig option I can spot is
>>> VM_EVENT. When that's off, the file won't be built at all.
>> But when 'CONFIG_VM_EVENT=y' this file is compiled on ARM.
>>>
>>> Further, BUG() and a few more constructs have a dedicated deviation
>>> already in place. I don't mind a useless function to be shrunk (or, as
>>> per above, perhaps even dropped), but the justification then needs to
>>> be different.
>> Well, with Stefano's comment regarding return type, the commit subject 
>> and message could be rewritten like:
>>
>> generic/altp2m: align and simplify altp2m_vcpu_idx()
>>
>> The return type of 'altp2m_vcpu_idx()' in the generic altp2m.h header is
>> currently 'unsigned int', which is inconsistent with its 'uint16_t'
>> return type on x86 and the 'altp2m_idx' member of the monitor structures.
>>
>> To fix this type inconsistency and simplify the header, this patch
>> replaces the static inline implementation of 
>> 'altp2m_vcpu_idx()' (contained a BUG() stub) with a simple function 
>> declaration returning
>> uint16_t.
>>
>> For architectures using the generic altp2m.h header (such as ARM when
>> CONFIG_VM_EVENT is enabled), common code calls to 'altp2m_vcpu_idx()' in 
>> common/monitor.c are guarded by 'altp2m_active()', which
>> statically returns 'false'. The compiler's DCE will optimize out these 
>> calls, avoiding any linker issues for the missing definition.
> 
> Hello Jan,
> 
> kind reminder. Please, take a look.

I did, back then already. No specific question was asked for me to answer,
so I was expecting a re-submission, allowing to see new description and
code changes together. That'll then make it easier / more natural to
comment on.

Jan

Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Dmytro Prokopchuk1 3 days, 15 hours ago

On 7/9/26 13:50, Jan Beulich wrote:
> On 09.07.2026 12:00, Dmytro Prokopchuk1 wrote:
>> On 6/22/26 13:29, Dmytro Prokopchuk wrote:
>>> On 4/10/26 09:04, Jan Beulich wrote:
>>>> On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
>>>>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
>>>>>
>>>>> In certain build configurations,
>>>>
>>>> Can you give an example where ...
>>> If CONFIG_VM_EVENT is enabled on ARM.
>>>>
>>>>> the function 'altp2m_vcpu_idx()' is defined
>>>>> as an inline function that contains the 'BUG()' macro. This resulted
>>>>> in a
>>>>> violation because the 'BUG()' macro makes the function non-returning.
>>>>>
>>>>> To ensure compliance with MISRA C Rule 2.1, this patch removes the
>>>>> inline
>>>>> function implementation and its BUG()-based unreachable code. It is
>>>>> replaced
>>>>> with an unconditional function declaration for 'altp2m_vcpu_idx()'.
>>>>
>>>> ... a declaration is needed? The sole non-x86 reference I see is from
>>> Yes, declaration is needed. The file 'common/monitor.c' contains a usage
>>> of the 'altp2m_vcpu_idx()' and compiler must see it to avoid "error:
>>> implicit declaration of function ‘altp2m_vcpu_idx’".
>>>> common/monitor.c, and the sole relevant Kconfig option I can spot is
>>>> VM_EVENT. When that's off, the file won't be built at all.
>>> But when 'CONFIG_VM_EVENT=y' this file is compiled on ARM.
>>>>
>>>> Further, BUG() and a few more constructs have a dedicated deviation
>>>> already in place. I don't mind a useless function to be shrunk (or, as
>>>> per above, perhaps even dropped), but the justification then needs to
>>>> be different.
>>> Well, with Stefano's comment regarding return type, the commit subject
>>> and message could be rewritten like:
>>>
>>> generic/altp2m: align and simplify altp2m_vcpu_idx()
>>>
>>> The return type of 'altp2m_vcpu_idx()' in the generic altp2m.h header is
>>> currently 'unsigned int', which is inconsistent with its 'uint16_t'
>>> return type on x86 and the 'altp2m_idx' member of the monitor structures.
>>>
>>> To fix this type inconsistency and simplify the header, this patch
>>> replaces the static inline implementation of
>>> 'altp2m_vcpu_idx()' (contained a BUG() stub) with a simple function
>>> declaration returning
>>> uint16_t.
>>>
>>> For architectures using the generic altp2m.h header (such as ARM when
>>> CONFIG_VM_EVENT is enabled), common code calls to 'altp2m_vcpu_idx()' in
>>> common/monitor.c are guarded by 'altp2m_active()', which
>>> statically returns 'false'. The compiler's DCE will optimize out these
>>> calls, avoiding any linker issues for the missing definition.
>>
>> Hello Jan,
>>
>> kind reminder. Please, take a look.
> 
> I did, back then already. No specific question was asked for me to answer,
> so I was expecting a re-submission, allowing to see new description and
> code changes together. That'll then make it easier / more natural to
> comment on.
> 
> Jan

Ack.
I'll submit a new version.
Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Dmytro Prokopchuk1 3 months ago
Hello Jan,

On 4/10/26 09:04, Jan Beulich wrote:
> On 09.04.2026 19:37, Dmytro Prokopchuk1 wrote:
>> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
>>
>> In certain build configurations,
> 
> Can you give an example where ...
When config VM_EVENT is enabled and compiled architecture (e.g. Arm64) 
doesn't have altp2m implementation.
> 
>> the function 'altp2m_vcpu_idx()' is defined
>> as an inline function that contains the 'BUG()' macro. This resulted in a
>> violation because the 'BUG()' macro makes the function non-returning.
>>
The call to "altp2m_vcpu_idx()" is guarded by the predicate "if 
(altp2m_active(d))", which always returns "false" 
(compile-time-constant) in current build config:

     if ( altp2m_active(d) )
     {
         req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
         req->altp2m_idx = altp2m_vcpu_idx(v);
     }

DCE removes (should remove) this entire branch, so the BUG() is never 
actually included in the final binary.

But, this code is still present after preprocessing and is analyzed by 
the Eclair tool (regardless of whether this code is later removed by the 
DCE).

No inline function --> no deviation (Eclair is happy).
>> To ensure compliance with MISRA C Rule 2.1, this patch removes the inline
>> function implementation and its BUG()-based unreachable code. It is replaced
>> with an unconditional function declaration for 'altp2m_vcpu_idx()'.
> 
> ... a declaration is needed? The sole non-x86 reference I see is from
> common/monitor.c, and the sole relevant Kconfig option I can spot is
> VM_EVENT. When that's off, the file won't be built at all.
> 
> Further, BUG() and a few more constructs have a dedicated deviation
> already in place.
If so, Eclair shouldn't report a violation...
  I don't mind a useless function to be shrunk (or, as
> per above, perhaps even dropped), but the justification then needs to
> be different.
> 
> Jan

BR, Dmytro.
Re: [PATCH] generic/altp2m: address violations of MISRA C Rule 2.1
Posted by Stefano Stabellini 3 months ago
On Thu, 9 Apr 2026, Dmytro Prokopchuk1 wrote:
> MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
> 
> In certain build configurations, the function 'altp2m_vcpu_idx()' is defined
> as an inline function that contains the 'BUG()' macro. This resulted in a
> violation because the 'BUG()' macro makes the function non-returning.
> 
> To ensure compliance with MISRA C Rule 2.1, this patch removes the inline
> function implementation and its BUG()-based unreachable code. It is replaced
> with an unconditional function declaration for 'altp2m_vcpu_idx()'. It relies
> on the compiler's Dead Code Elimination (DCE) to remove the unused function
> in builds where it is not needed.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
> ---
> Test CI pipeline:
> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2441424553
> ---
>  xen/include/asm-generic/altp2m.h | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/xen/include/asm-generic/altp2m.h b/xen/include/asm-generic/altp2m.h
> index 39865a842a..df6b9a9c01 100644
> --- a/xen/include/asm-generic/altp2m.h
> +++ b/xen/include/asm-generic/altp2m.h
> @@ -15,12 +15,7 @@ static inline bool altp2m_active(const struct domain *d)
>  }
>  
>  /* Alternate p2m VCPU */
> -static inline unsigned int altp2m_vcpu_idx(const struct vcpu *v)
> -{
> -    /* Not implemented on GENERIC, should not be reached. */
> -    BUG();
> -    return 0;
> -}
> +uint16_t altp2m_vcpu_idx(const struct vcpu *v);

The return type being changed to uint16_t is also a fix. It should be
mentioned in the commit message. Aside from that:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>  #endif /* __ASM_GENERIC_ALTP2M_H */
>  
> -- 
> 2.43.0
>