[PATCH 0/4] address remaining rule 13.1 violations; mark clean

[PATCH 0/4] address remaining rule 13.1 violations; mark clean

[Jan Beulich]

"Initializer lists shall not contain persistent side effects"

1: x86/pagewalk: avoid ACCESS_ONCE() in compound literals
2: Arm/guestcopy: avoid use of "current" in compound literals
3: sched/core: avoid use of "current" in initializer lists
4: automation/Eclair: tag rule 13.1 as clean

Jan

[PATCH 1/4] x86/pagewalk: avoid ACCESS_ONCE() in compound literals

[Jan Beulich]

Compound literals are also covered by Misra C:2012 rule 13.1
("Initializer lists shall not contain persistent side effects"), and
(sadly?) that rule also applies to lists with just a single element, or
more generally with just a single side effect. Use intermediate variables
to overcome this as well as ACCESS_ONCE()'s restriction to be usable on
scalar types only.

No functional change intended.

Fixes: 0345b835dc97 ("x86/pagewalk: Read guest PTEs with ACCESS_ONCE()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -129,7 +129,9 @@ guest_walk_tables(const struct vcpu *v,
             guest_l4_table_offset(va) * sizeof(gw->l4e);
     if ( !hvmemul_read_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e)) )
     {
-        gw->l4e = (guest_l4e_t){ ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4) };
+        guest_intpte_t l4e = ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4);
+
+        gw->l4e = (guest_l4e_t){ l4e };
         hvmemul_write_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e));
     }
     gflags = guest_l4e_get_flags(gw->l4e);
@@ -164,7 +166,9 @@ guest_walk_tables(const struct vcpu *v,
             guest_l3_table_offset(va) * sizeof(gw->l3e);
     if ( !hvmemul_read_cache(v, l3gpa, &gw->l3e, sizeof(gw->l3e)) )
     {
-        gw->l3e = (guest_l3e_t){ ACCESS_ONCE(l3p[guest_l3_table_offset(va)].l3) };
+        guest_intpte_t l3e = ACCESS_ONCE(l3p[guest_l3_table_offset(va)].l3);
+
+        gw->l3e = (guest_l3e_t){ l3e };
         hvmemul_write_cache(v, l3gpa, &gw->l3e, sizeof(gw->l3e));
     }
     gflags = guest_l3e_get_flags(gw->l3e);
@@ -264,7 +268,9 @@ guest_walk_tables(const struct vcpu *v,
     l2gpa += guest_l2_table_offset(va) * sizeof(gw->l2e);
     if ( !hvmemul_read_cache(v, l2gpa, &gw->l2e, sizeof(gw->l2e)) )
     {
-        gw->l2e = (guest_l2e_t){ ACCESS_ONCE(l2p[guest_l2_table_offset(va)].l2) };
+        guest_intpte_t l2e = ACCESS_ONCE(l2p[guest_l2_table_offset(va)].l2);
+
+        gw->l2e = (guest_l2e_t){ l2e };
         hvmemul_write_cache(v, l2gpa, &gw->l2e, sizeof(gw->l2e));
     }
 
@@ -353,7 +359,9 @@ guest_walk_tables(const struct vcpu *v,
             guest_l1_table_offset(va) * sizeof(gw->l1e);
     if ( !hvmemul_read_cache(v, l1gpa, &gw->l1e, sizeof(gw->l1e)) )
     {
-        gw->l1e = (guest_l1e_t){ ACCESS_ONCE(l1p[guest_l1_table_offset(va)].l1) };
+        guest_intpte_t l1e = ACCESS_ONCE(l1p[guest_l1_table_offset(va)].l1);
+
+        gw->l1e = (guest_l1e_t){ l1e };
         hvmemul_write_cache(v, l1gpa, &gw->l1e, sizeof(gw->l1e));
     }
 

Re: [PATCH 1/4] x86/pagewalk: avoid ACCESS_ONCE() in compound literals

[Roger Pau Monné]

On Thu, Sep 03, 2026 at 01:51:28PM +0200, Jan Beulich wrote:
> Compound literals are also covered by Misra C:2012 rule 13.1
> ("Initializer lists shall not contain persistent side effects"), and
> (sadly?) that rule also applies to lists with just a single element, or
> more generally with just a single side effect. Use intermediate variables
> to overcome this as well as ACCESS_ONCE()'s restriction to be usable on
> scalar types only.
> 
> No functional change intended.
> 
> Fixes: 0345b835dc97 ("x86/pagewalk: Read guest PTEs with ACCESS_ONCE()")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/mm/guest_walk.c
> +++ b/xen/arch/x86/mm/guest_walk.c
> @@ -129,7 +129,9 @@ guest_walk_tables(const struct vcpu *v,
>              guest_l4_table_offset(va) * sizeof(gw->l4e);
>      if ( !hvmemul_read_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e)) )
>      {
> -        gw->l4e = (guest_l4e_t){ ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4) };
> +        guest_intpte_t l4e = ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4);
> +
> +        gw->l4e = (guest_l4e_t){ l4e };

Did you consider using the l4e_from_intpte() and similar helpers here
and below?

Thanks, Roger.

Re: [PATCH 1/4] x86/pagewalk: avoid ACCESS_ONCE() in compound literals

[Jan Beulich]

On 21.09.2026 10:53, Roger Pau Monné wrote:
> On Thu, Sep 03, 2026 at 01:51:28PM +0200, Jan Beulich wrote:
>> Compound literals are also covered by Misra C:2012 rule 13.1
>> ("Initializer lists shall not contain persistent side effects"), and
>> (sadly?) that rule also applies to lists with just a single element, or
>> more generally with just a single side effect. Use intermediate variables
>> to overcome this as well as ACCESS_ONCE()'s restriction to be usable on
>> scalar types only.
>>
>> No functional change intended.
>>
>> Fixes: 0345b835dc97 ("x86/pagewalk: Read guest PTEs with ACCESS_ONCE()")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/mm/guest_walk.c
>> +++ b/xen/arch/x86/mm/guest_walk.c
>> @@ -129,7 +129,9 @@ guest_walk_tables(const struct vcpu *v,
>>              guest_l4_table_offset(va) * sizeof(gw->l4e);
>>      if ( !hvmemul_read_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e)) )
>>      {
>> -        gw->l4e = (guest_l4e_t){ ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4) };
>> +        guest_intpte_t l4e = ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4);
>> +
>> +        gw->l4e = (guest_l4e_t){ l4e };
> 
> Did you consider using the l4e_from_intpte() and similar helpers here
> and below?

No, I didn't, as they aren't applicable to guest_l<N>e_t. guest_intpte_t
aliases intpte_t only for GUEST_PAGING_LEVELS > 2.

Jan

Re: [PATCH 1/4] x86/pagewalk: avoid ACCESS_ONCE() in compound literals

[Roger Pau Monné]

On Mon, Sep 21, 2026 at 10:59:09AM +0200, Jan Beulich wrote:
> On 21.09.2026 10:53, Roger Pau Monné wrote:
> > On Thu, Sep 03, 2026 at 01:51:28PM +0200, Jan Beulich wrote:
> >> Compound literals are also covered by Misra C:2012 rule 13.1
> >> ("Initializer lists shall not contain persistent side effects"), and
> >> (sadly?) that rule also applies to lists with just a single element, or
> >> more generally with just a single side effect. Use intermediate variables
> >> to overcome this as well as ACCESS_ONCE()'s restriction to be usable on
> >> scalar types only.
> >>
> >> No functional change intended.
> >>
> >> Fixes: 0345b835dc97 ("x86/pagewalk: Read guest PTEs with ACCESS_ONCE()")
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>
> >> --- a/xen/arch/x86/mm/guest_walk.c
> >> +++ b/xen/arch/x86/mm/guest_walk.c
> >> @@ -129,7 +129,9 @@ guest_walk_tables(const struct vcpu *v,
> >>              guest_l4_table_offset(va) * sizeof(gw->l4e);
> >>      if ( !hvmemul_read_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e)) )
> >>      {
> >> -        gw->l4e = (guest_l4e_t){ ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4) };
> >> +        guest_intpte_t l4e = ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4);
> >> +
> >> +        gw->l4e = (guest_l4e_t){ l4e };
> > 
> > Did you consider using the l4e_from_intpte() and similar helpers here
> > and below?
> 
> No, I didn't, as they aren't applicable to guest_l<N>e_t. guest_intpte_t
> aliases intpte_t only for GUEST_PAGING_LEVELS > 2.

Right, as otherwise it's a 32bit PTE.

Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks, Roger.

[PATCH 2/4] Arm/guestcopy: avoid use of "current" in compound literals

[Jan Beulich]

Compound literals are also covered by Misra C:2012 rule 13.1
("Initializer lists shall not contain persistent side effects"), and
(sadly?) that rule also applies to lists with just a single element, or
more generally with just a single side effect. Use intermediate variables
to overcome this.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This is assumed to go on top of "Arm/guestcopy: deviate copy_guest() uses
just like their x86 counterparts", conflicting contextually but not
functionally.

--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -109,29 +109,37 @@ static unsigned long copy_guest(void *bu
 
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
 {
+    struct vcpu *curr = current;
+
     return copy_guest((void *)from, /* COPY_to_guest doesn't modify */
-                      (vaddr_t)to, len, GVA_INFO(current),
+                      (vaddr_t)to, len, GVA_INFO(curr),
                       COPY_to_guest | COPY_linear);
 }
 
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
                                              unsigned int len)
 {
+    struct vcpu *curr = current;
+
     return copy_guest((void *)from, /* COPY_to_guest doesn't modify */
-                      (vaddr_t)to, len, GVA_INFO(current),
+                      (vaddr_t)to, len, GVA_INFO(curr),
                       COPY_to_guest | COPY_flush_dcache | COPY_linear);
 }
 
 unsigned long raw_clear_guest(void *to, unsigned int len)
 {
-    return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(current),
+    struct vcpu *curr = current;
+
+    return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(curr),
                       COPY_to_guest | COPY_linear);
 }
 
 unsigned long raw_copy_from_guest(void *to, const void __user *from,
                                   unsigned int len)
 {
-    return copy_guest(to, (vaddr_t)from, len, GVA_INFO(current),
+    struct vcpu *curr = current;
+
+    return copy_guest(to, (vaddr_t)from, len, GVA_INFO(curr),
                       COPY_from_guest | COPY_linear);
 }
 

Re: [PATCH 2/4] Arm/guestcopy: avoid use of "current" in compound literals

[Orzel, Michal]

On 03-Sep-26 13:52, Jan Beulich wrote:
> Compound literals are also covered by Misra C:2012 rule 13.1
> ("Initializer lists shall not contain persistent side effects"), and
> (sadly?) that rule also applies to lists with just a single element, or
> more generally with just a single side effect. Use intermediate variables
> to overcome this.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

[PATCH 3/4] sched/core: avoid use of "current" in initializer lists

[Jan Beulich]

TRACE_TIME() uses an initializer list and hence is covered by Misra C:2012
rule 13.1 ("Initializer lists shall not contain persistent side effects").
Use intermediate variables to address this.

In vcpu_yield() take the opportunity to rename the existing variable which
better would have been used already before.

In do_sched_op() extend the scope of and rename an existing variable. Also
drop a pointless (and slightly malformed, style-wise) cast there, and
adjust to other one to by style conformant.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note that the violations presently are reported only for Arm. Similar
issues would exist on x86 in Clang builds, afaict.

--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -1531,20 +1531,20 @@ static long do_poll(const struct sched_p
 /* Voluntarily yield the processor for this allocation. */
 long vcpu_yield(void)
 {
-    struct vcpu * v=current;
+    struct vcpu *curr = current;
     spinlock_t *lock;
 
     rcu_read_lock(&sched_res_rculock);
 
-    lock = unit_schedule_lock_irq(v->sched_unit);
-    sched_yield(vcpu_scheduler(v), v->sched_unit);
-    unit_schedule_unlock_irq(lock, v->sched_unit);
+    lock = unit_schedule_lock_irq(curr->sched_unit);
+    sched_yield(vcpu_scheduler(curr), curr->sched_unit);
+    unit_schedule_unlock_irq(lock, curr->sched_unit);
 
     rcu_read_unlock(&sched_res_rculock);
 
     SCHED_STAT_CRANK(vcpu_yield);
 
-    TRACE_TIME(TRC_SCHED_YIELD, current->domain->domain_id, current->vcpu_id);
+    TRACE_TIME(TRC_SCHED_YIELD, curr->domain->domain_id, curr->vcpu_id);
     raise_softirq(SCHEDULE_SOFTIRQ);
     return 0;
 }
@@ -1914,6 +1914,8 @@ typedef long ret_t;
 
 ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
+    const struct vcpu *curr = current;
+    struct domain *currd = curr->domain;
     ret_t ret = 0;
 
     switch ( cmd )
@@ -1938,9 +1940,9 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
         if ( copy_from_guest(&sched_shutdown, arg, 1) )
             break;
 
-        TRACE_TIME(TRC_SCHED_SHUTDOWN, current->domain->domain_id,
-                   current->vcpu_id, sched_shutdown.reason);
-        ret = domain_shutdown(current->domain, (u8)sched_shutdown.reason);
+        TRACE_TIME(TRC_SCHED_SHUTDOWN, currd->domain_id, curr->vcpu_id,
+                   sched_shutdown.reason);
+        ret = domain_shutdown(currd, sched_shutdown.reason);
 
         break;
     }
@@ -1948,19 +1950,18 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
     case SCHEDOP_shutdown_code:
     {
         struct sched_shutdown sched_shutdown;
-        struct domain *d = current->domain;
 
         ret = -EFAULT;
         if ( copy_from_guest(&sched_shutdown, arg, 1) )
             break;
 
-        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, d->domain_id, current->vcpu_id,
+        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, currd->domain_id, curr->vcpu_id,
                    sched_shutdown.reason);
 
-        spin_lock(&d->shutdown_lock);
-        if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
-            d->shutdown_code = (u8)sched_shutdown.reason;
-        spin_unlock(&d->shutdown_lock);
+        spin_lock(&currd->shutdown_lock);
+        if ( currd->shutdown_code == SHUTDOWN_CODE_INVALID )
+            currd->shutdown_code = (uint8_t)sched_shutdown.reason;
+        spin_unlock(&currd->shutdown_lock);
 
         ret = 0;
         break;

Re: [PATCH 3/4] sched/core: avoid use of "current" in initializer lists

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 3870 bytes --]

On 03.09.26 13:53, Jan Beulich wrote:
> TRACE_TIME() uses an initializer list and hence is covered by Misra C:2012
> rule 13.1 ("Initializer lists shall not contain persistent side effects").
> Use intermediate variables to address this.
> 
> In vcpu_yield() take the opportunity to rename the existing variable which
> better would have been used already before.
> 
> In do_sched_op() extend the scope of and rename an existing variable. Also
> drop a pointless (and slightly malformed, style-wise) cast there, and
> adjust to other one to by style conformant.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Note that the violations presently are reported only for Arm. Similar
> issues would exist on x86 in Clang builds, afaict.
> 
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -1531,20 +1531,20 @@ static long do_poll(const struct sched_p
>   /* Voluntarily yield the processor for this allocation. */
>   long vcpu_yield(void)
>   {
> -    struct vcpu * v=current;
> +    struct vcpu *curr = current;
>       spinlock_t *lock;
>   
>       rcu_read_lock(&sched_res_rculock);
>   
> -    lock = unit_schedule_lock_irq(v->sched_unit);
> -    sched_yield(vcpu_scheduler(v), v->sched_unit);
> -    unit_schedule_unlock_irq(lock, v->sched_unit);
> +    lock = unit_schedule_lock_irq(curr->sched_unit);
> +    sched_yield(vcpu_scheduler(curr), curr->sched_unit);
> +    unit_schedule_unlock_irq(lock, curr->sched_unit);
>   
>       rcu_read_unlock(&sched_res_rculock);
>   
>       SCHED_STAT_CRANK(vcpu_yield);
>   
> -    TRACE_TIME(TRC_SCHED_YIELD, current->domain->domain_id, current->vcpu_id);
> +    TRACE_TIME(TRC_SCHED_YIELD, curr->domain->domain_id, curr->vcpu_id);
>       raise_softirq(SCHEDULE_SOFTIRQ);
>       return 0;
>   }
> @@ -1914,6 +1914,8 @@ typedef long ret_t;
>   
>   ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>   {
> +    const struct vcpu *curr = current;
> +    struct domain *currd = curr->domain;
>       ret_t ret = 0;
>   
>       switch ( cmd )
> @@ -1938,9 +1940,9 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
>           if ( copy_from_guest(&sched_shutdown, arg, 1) )
>               break;
>   
> -        TRACE_TIME(TRC_SCHED_SHUTDOWN, current->domain->domain_id,
> -                   current->vcpu_id, sched_shutdown.reason);
> -        ret = domain_shutdown(current->domain, (u8)sched_shutdown.reason);
> +        TRACE_TIME(TRC_SCHED_SHUTDOWN, currd->domain_id, curr->vcpu_id,
> +                   sched_shutdown.reason);
> +        ret = domain_shutdown(currd, sched_shutdown.reason);
>   
>           break;
>       }
> @@ -1948,19 +1950,18 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
>       case SCHEDOP_shutdown_code:
>       {
>           struct sched_shutdown sched_shutdown;
> -        struct domain *d = current->domain;
>   
>           ret = -EFAULT;
>           if ( copy_from_guest(&sched_shutdown, arg, 1) )
>               break;
>   
> -        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, d->domain_id, current->vcpu_id,
> +        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, currd->domain_id, curr->vcpu_id,
>                      sched_shutdown.reason);
>   
> -        spin_lock(&d->shutdown_lock);
> -        if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
> -            d->shutdown_code = (u8)sched_shutdown.reason;
> -        spin_unlock(&d->shutdown_lock);
> +        spin_lock(&currd->shutdown_lock);
> +        if ( currd->shutdown_code == SHUTDOWN_CODE_INVALID )
> +            currd->shutdown_code = (uint8_t)sched_shutdown.reason;
> +        spin_unlock(&currd->shutdown_lock);
>   
>           ret = 0;
>           break;
> 

What about the use of current in SCHEDOP_watchdog and SCHEDOP_pin_override
handling?


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 3/4] sched/core: avoid use of "current" in initializer lists

[Jan Beulich]

On 03.09.2026 15:26, Jürgen Groß wrote:
> On 03.09.26 13:53, Jan Beulich wrote:
>> @@ -1914,6 +1914,8 @@ typedef long ret_t;
>>   
>>   ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>   {
>> +    const struct vcpu *curr = current;
>> +    struct domain *currd = curr->domain;
>>       ret_t ret = 0;
>>   
>>       switch ( cmd )
>> @@ -1938,9 +1940,9 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
>>           if ( copy_from_guest(&sched_shutdown, arg, 1) )
>>               break;
>>   
>> -        TRACE_TIME(TRC_SCHED_SHUTDOWN, current->domain->domain_id,
>> -                   current->vcpu_id, sched_shutdown.reason);
>> -        ret = domain_shutdown(current->domain, (u8)sched_shutdown.reason);
>> +        TRACE_TIME(TRC_SCHED_SHUTDOWN, currd->domain_id, curr->vcpu_id,
>> +                   sched_shutdown.reason);
>> +        ret = domain_shutdown(currd, sched_shutdown.reason);
>>   
>>           break;
>>       }
>> @@ -1948,19 +1950,18 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
>>       case SCHEDOP_shutdown_code:
>>       {
>>           struct sched_shutdown sched_shutdown;
>> -        struct domain *d = current->domain;
>>   
>>           ret = -EFAULT;
>>           if ( copy_from_guest(&sched_shutdown, arg, 1) )
>>               break;
>>   
>> -        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, d->domain_id, current->vcpu_id,
>> +        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, currd->domain_id, curr->vcpu_id,
>>                      sched_shutdown.reason);
>>   
>> -        spin_lock(&d->shutdown_lock);
>> -        if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
>> -            d->shutdown_code = (u8)sched_shutdown.reason;
>> -        spin_unlock(&d->shutdown_lock);
>> +        spin_lock(&currd->shutdown_lock);
>> +        if ( currd->shutdown_code == SHUTDOWN_CODE_INVALID )
>> +            currd->shutdown_code = (uint8_t)sched_shutdown.reason;
>> +        spin_unlock(&currd->shutdown_lock);
>>   
>>           ret = 0;
>>           break;
> 
> What about the use of current in SCHEDOP_watchdog and SCHEDOP_pin_override
> handling?

I can change those too if that's wanted, also the one in SCHEDOP_remote_shutdown
handling then.

Jan

Re: [PATCH 3/4] sched/core: avoid use of "current" in initializer lists

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 2386 bytes --]

On 03.09.26 15:55, Jan Beulich wrote:
> On 03.09.2026 15:26, Jürgen Groß wrote:
>> On 03.09.26 13:53, Jan Beulich wrote:
>>> @@ -1914,6 +1914,8 @@ typedef long ret_t;
>>>    
>>>    ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>    {
>>> +    const struct vcpu *curr = current;
>>> +    struct domain *currd = curr->domain;
>>>        ret_t ret = 0;
>>>    
>>>        switch ( cmd )
>>> @@ -1938,9 +1940,9 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
>>>            if ( copy_from_guest(&sched_shutdown, arg, 1) )
>>>                break;
>>>    
>>> -        TRACE_TIME(TRC_SCHED_SHUTDOWN, current->domain->domain_id,
>>> -                   current->vcpu_id, sched_shutdown.reason);
>>> -        ret = domain_shutdown(current->domain, (u8)sched_shutdown.reason);
>>> +        TRACE_TIME(TRC_SCHED_SHUTDOWN, currd->domain_id, curr->vcpu_id,
>>> +                   sched_shutdown.reason);
>>> +        ret = domain_shutdown(currd, sched_shutdown.reason);
>>>    
>>>            break;
>>>        }
>>> @@ -1948,19 +1950,18 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
>>>        case SCHEDOP_shutdown_code:
>>>        {
>>>            struct sched_shutdown sched_shutdown;
>>> -        struct domain *d = current->domain;
>>>    
>>>            ret = -EFAULT;
>>>            if ( copy_from_guest(&sched_shutdown, arg, 1) )
>>>                break;
>>>    
>>> -        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, d->domain_id, current->vcpu_id,
>>> +        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, currd->domain_id, curr->vcpu_id,
>>>                       sched_shutdown.reason);
>>>    
>>> -        spin_lock(&d->shutdown_lock);
>>> -        if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
>>> -            d->shutdown_code = (u8)sched_shutdown.reason;
>>> -        spin_unlock(&d->shutdown_lock);
>>> +        spin_lock(&currd->shutdown_lock);
>>> +        if ( currd->shutdown_code == SHUTDOWN_CODE_INVALID )
>>> +            currd->shutdown_code = (uint8_t)sched_shutdown.reason;
>>> +        spin_unlock(&currd->shutdown_lock);
>>>    
>>>            ret = 0;
>>>            break;
>>
>> What about the use of current in SCHEDOP_watchdog and SCHEDOP_pin_override
>> handling?
> 
> I can change those too if that's wanted, also the one in SCHEDOP_remote_shutdown
> handling then.

I'd prefer that.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[PATCH 4/4] automation/Eclair: tag rule 13.1 as clean

[Jan Beulich]

Remaining violations were addressed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/automation/eclair_analysis/ECLAIR/tagging.ecl
+++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
@@ -66,6 +66,7 @@ MC3A2.R11.7||
 MC3A2.R11.9||
 MC3A2.R12.2||
 MC3A2.R12.5||
+MC3A2.R13.1||
 MC3A2.R13.2||
 MC3A2.R13.6||
 MC3A2.R14.1||

Re: [PATCH 4/4] automation/Eclair: tag rule 13.1 as clean

[Nicola Vetrini]

On 2026-09-03 13:54, Jan Beulich wrote:
> Remaining violations were addressed.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 

Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Supposing of course all previous patches are ready to go in

> --- a/automation/eclair_analysis/ECLAIR/tagging.ecl
> +++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
> @@ -66,6 +66,7 @@ MC3A2.R11.7||
>  MC3A2.R11.9||
>  MC3A2.R12.2||
>  MC3A2.R12.5||
> +MC3A2.R13.1||
>  MC3A2.R13.2||
>  MC3A2.R13.6||
>  MC3A2.R14.1||

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

[PATCH v1.1 3/4] sched/core: avoid use of "current" in initializer lists

[Jan Beulich]

TRACE_TIME() uses an initializer list and hence is covered by Misra C:2012
rule 13.1 ("Initializer lists shall not contain persistent side effects").
Use intermediate variables to address this.

In vcpu_yield() take the opportunity to rename the existing variable which
better would have been used already before.

In do_sched_op() extend the scope of and rename an existing variable. Use
both variables throughout the function. Also drop a pointless (and
slightly malformed, style-wise) cast there, and adjust to other one to by
style conformant.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note that the violations presently are reported only for Arm. Similar
issues would exist on x86 in Clang builds, afaict.
---
v1.1: Also replace further (unrelated) uses of current in do_sched_op().

--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -1531,20 +1531,20 @@ static long do_poll(const struct sched_p
 /* Voluntarily yield the processor for this allocation. */
 long vcpu_yield(void)
 {
-    struct vcpu * v=current;
+    struct vcpu *curr = current;
     spinlock_t *lock;
 
     rcu_read_lock(&sched_res_rculock);
 
-    lock = unit_schedule_lock_irq(v->sched_unit);
-    sched_yield(vcpu_scheduler(v), v->sched_unit);
-    unit_schedule_unlock_irq(lock, v->sched_unit);
+    lock = unit_schedule_lock_irq(curr->sched_unit);
+    sched_yield(vcpu_scheduler(curr), curr->sched_unit);
+    unit_schedule_unlock_irq(lock, curr->sched_unit);
 
     rcu_read_unlock(&sched_res_rculock);
 
     SCHED_STAT_CRANK(vcpu_yield);
 
-    TRACE_TIME(TRC_SCHED_YIELD, current->domain->domain_id, current->vcpu_id);
+    TRACE_TIME(TRC_SCHED_YIELD, curr->domain->domain_id, curr->vcpu_id);
     raise_softirq(SCHEDULE_SOFTIRQ);
     return 0;
 }
@@ -1914,6 +1914,8 @@ typedef long ret_t;
 
 ret_t do_sched_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
+    struct vcpu *curr = current;
+    struct domain *currd = curr->domain;
     ret_t ret = 0;
 
     switch ( cmd )
@@ -1938,9 +1940,9 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
         if ( copy_from_guest(&sched_shutdown, arg, 1) )
             break;
 
-        TRACE_TIME(TRC_SCHED_SHUTDOWN, current->domain->domain_id,
-                   current->vcpu_id, sched_shutdown.reason);
-        ret = domain_shutdown(current->domain, (u8)sched_shutdown.reason);
+        TRACE_TIME(TRC_SCHED_SHUTDOWN, currd->domain_id, curr->vcpu_id,
+                   sched_shutdown.reason);
+        ret = domain_shutdown(currd, sched_shutdown.reason);
 
         break;
     }
@@ -1948,19 +1950,18 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
     case SCHEDOP_shutdown_code:
     {
         struct sched_shutdown sched_shutdown;
-        struct domain *d = current->domain;
 
         ret = -EFAULT;
         if ( copy_from_guest(&sched_shutdown, arg, 1) )
             break;
 
-        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, d->domain_id, current->vcpu_id,
+        TRACE_TIME(TRC_SCHED_SHUTDOWN_CODE, currd->domain_id, curr->vcpu_id,
                    sched_shutdown.reason);
 
-        spin_lock(&d->shutdown_lock);
-        if ( d->shutdown_code == SHUTDOWN_CODE_INVALID )
-            d->shutdown_code = (u8)sched_shutdown.reason;
-        spin_unlock(&d->shutdown_lock);
+        spin_lock(&currd->shutdown_lock);
+        if ( currd->shutdown_code == SHUTDOWN_CODE_INVALID )
+            currd->shutdown_code = (uint8_t)sched_shutdown.reason;
+        spin_unlock(&currd->shutdown_lock);
 
         ret = 0;
         break;
@@ -1993,7 +1994,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
         if ( d == NULL )
             break;
 
-        ret = xsm_schedop_shutdown(XSM_DM_PRIV, current->domain, d);
+        ret = xsm_schedop_shutdown(XSM_DM_PRIV, currd, d);
         if ( likely(!ret) )
             domain_shutdown(d, sched_remote_shutdown.reason);
 
@@ -2010,8 +2011,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
         if ( copy_from_guest(&sched_watchdog, arg, 1) )
             break;
 
-        ret = domain_watchdog(
-            current->domain, sched_watchdog.id, sched_watchdog.timeout);
+        ret = domain_watchdog(currd, sched_watchdog.id, sched_watchdog.timeout);
         break;
     }
 
@@ -2021,7 +2021,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
         unsigned int cpu;
 
         ret = -EPERM;
-        if ( !is_hardware_domain(current->domain) )
+        if ( !is_hardware_domain(currd) )
             break;
 
         ret = -EFAULT;
@@ -2033,7 +2033,7 @@ ret_t do_sched_op(int cmd, XEN_GUEST_HAN
            break;
 
         cpu = sched_pin_override.pcpu < 0 ? NR_CPUS : sched_pin_override.pcpu;
-        ret = vcpu_temporary_affinity(current, cpu, VCPU_AFFINITY_OVERRIDE);
+        ret = vcpu_temporary_affinity(curr, cpu, VCPU_AFFINITY_OVERRIDE);
 
         break;
     }

Re: [PATCH v1.1 3/4] sched/core: avoid use of "current" in initializer lists

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 773 bytes --]

On 08.09.26 14:21, Jan Beulich wrote:
> TRACE_TIME() uses an initializer list and hence is covered by Misra C:2012
> rule 13.1 ("Initializer lists shall not contain persistent side effects").
> Use intermediate variables to address this.
> 
> In vcpu_yield() take the opportunity to rename the existing variable which
> better would have been used already before.
> 
> In do_sched_op() extend the scope of and rename an existing variable. Use
> both variables throughout the function. Also drop a pointless (and
> slightly malformed, style-wise) cast there, and adjust to other one to by
> style conformant.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v1.1 3/4] sched/core: avoid use of "current" in initializer lists

[Jan Beulich]

On 10.09.2026 14:07, Jürgen Groß wrote:
> On 08.09.26 14:21, Jan Beulich wrote:
>> TRACE_TIME() uses an initializer list and hence is covered by Misra C:2012
>> rule 13.1 ("Initializer lists shall not contain persistent side effects").
>> Use intermediate variables to address this.
>>
>> In vcpu_yield() take the opportunity to rename the existing variable which
>> better would have been used already before.
>>
>> In do_sched_op() extend the scope of and rename an existing variable. Use
>> both variables throughout the function. Also drop a pointless (and
>> slightly malformed, style-wise) cast there, and adjust to other one to by
>> style conformant.
>>
>> No functional change intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Turns out I applied this to a stale v1 I had sitting around, and then
committed that. I've now reverted the bad commit, putting in the intended
one on top.

Jan