Re: [PATCH] misra: fix Rule 11.3 violation in 'vcpu_mark_events_pending'

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

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@xxxxxxxx>
>> ---
>> 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.