MISRA C:2012 Rule 11.3 states: "A cast shall not be performed between
a pointer to object type and a pointer to a different object type."

The function 'vcpu_mark_events_pending' contains a non-compliant cast
to (unsigned long*). Remove the explicit cast and pass the compatible
pointer type to the 'guest_test_and_set_bit' macro.

Fixes: c626aa1a5a (arm: implement event injection, 2012-06-01)
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1993054203
---
 xen/arch/arm/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 310c578909..6371e68cc7 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -1166,7 +1166,7 @@ void arch_dump_vcpu_info(struct vcpu *v)
 void vcpu_mark_events_pending(struct vcpu *v)
 {
     bool already_pending = guest_test_and_set_bit(v->domain,
-        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
+        0, &vcpu_info(v, evtchn_upcall_pending));
 
     if ( already_pending )
         return;
-- 
2.43.0

On 20.08.2025 10:00, Dmytro Prokopchuk1 wrote:
> MISRA C:2012 Rule 11.3 states: "A cast shall not be performed between
> a pointer to object type and a pointer to a different object type."
> 
> The function 'vcpu_mark_events_pending' contains a non-compliant cast
> to (unsigned long*). Remove the explicit cast and pass the compatible
> pointer type to the 'guest_test_and_set_bit' macro.

No-where up to here (incl the subject) it is said that this is an Arm-
only issue. Hence why I ended up looking in the first place.

> Fixes: c626aa1a5a (arm: implement event injection, 2012-06-01)
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
> ---
> Test CI pipeline:
> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1993054203
> ---
>  xen/arch/arm/domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 310c578909..6371e68cc7 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -1166,7 +1166,7 @@ void arch_dump_vcpu_info(struct vcpu *v)
>  void vcpu_mark_events_pending(struct vcpu *v)
>  {
>      bool already_pending = guest_test_and_set_bit(v->domain,
> -        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
> +        0, &vcpu_info(v, evtchn_upcall_pending));

It'll be Arm maintainers to judge, but my take is that the previously
questionable construct better wouldn't be made yet more questionable.
What's missing, at least for Arm32 aiui, is a (build-time) check that
the field actually lives at a 32-bit boundary and that read-modify-
write operations carried out on it at 32-bit width (this is true even
for Arm64) won't have an impact on what follows (this may be possible
to cover by just a comment, as you can't really check for the absence
of struct fields).

Jan

On 8/21/25 12:35, Jan Beulich wrote:
> On 20.08.2025 10:00, Dmytro Prokopchuk1 wrote:
>> MISRA C:2012 Rule 11.3 states: "A cast shall not be performed between
>> a pointer to object type and a pointer to a different object type."
>>
>> The function 'vcpu_mark_events_pending' contains a non-compliant cast
>> to (unsigned long*). Remove the explicit cast and pass the compatible
>> pointer type to the 'guest_test_and_set_bit' macro.
>
> No-where up to here (incl the subject) it is said that this is an Arm-
> only issue. Hence why I ended up looking in the first place.
>
>> Fixes: c626aa1a5a (arm: implement event injection, 2012-06-01)
>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
>> ---
>> Test CI pipeline:
>> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1993054203
>> ---
>>   xen/arch/arm/domain.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 310c578909..6371e68cc7 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -1166,7 +1166,7 @@ void arch_dump_vcpu_info(struct vcpu *v)
>>   void vcpu_mark_events_pending(struct vcpu *v)
>>   {
>>       bool already_pending = guest_test_and_set_bit(v->domain,
>> -        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
>> +        0, &vcpu_info(v, evtchn_upcall_pending));
>
> It'll be Arm maintainers to judge, but my take is that the previously
> questionable construct better wouldn't be made yet more questionable.
> What's missing, at least for Arm32 aiui, is a (build-time) check that
> the field actually lives at a 32-bit boundary and that read-modify-
> write operations carried out on it at 32-bit width (this is true even
> for Arm64) won't have an impact on what follows (this may be possible
> to cover by just a comment, as you can't really check for the absence
> of struct fields).
>
> Jan

The build-time assert could be placed there:

BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) % 4 != 0);

but member 'evtchn_upcall_pending' is the first in the struct
'vcpu_info', and offsetof() will always return zero.
Currently this assert is useless. Probably it can protect us in the
future when somebody will decide to move 'evtchn_upcall_pending' or add
new member in front of it.

Dmytro.