[XEN PATCH 0/5] address violation of MISRA C Rule 5.5

[XEN PATCH 0/5] address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

This patch series fully eliminates MISRA C Rule 5.5
violations for ARM64.

The previous thread is here:
https://lore.kernel.org/xen-devel/48c7830931a98b2bf70ef1509f309b262b9e5792.1745427770.git.victorm.lira@amd.com/
where that violation was proposed to be deviated.

Dmytro Prokopchuk (5):
  gnttab: address violation of MISRA C Rule 5.5
  iommu: address violation of MISRA C Rule 5.5
  x86/irq: address violation of MISRA C Rule 5.5
  device-tree: address violation of MISRA C Rule 5.5
  xen/bitops: address violation of MISRA C Rule 5.5

 xen/arch/x86/irq.c                    |  2 +-
 xen/common/device-tree/domain-build.c |  9 ++++-----
 xen/common/grant_table.c              | 22 +++++++++++++---------
 xen/include/xen/bitops.h              | 24 ++++++++++++------------
 xen/include/xen/fdt-domain-build.h    |  4 ++--
 xen/include/xen/iommu.h               |  5 ++---
 xen/include/xen/irq.h                 |  4 ++--
 7 files changed, 36 insertions(+), 34 deletions(-)

-- 
2.43.0

[XEN PATCH 1/5] gnttab: address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".

Reports for service MC3A2.R5.5:
xen/common/grant_table.c: non-compliant macro 'update_gnttab_par'
xen/common/grant_table.c: non-compliant macro 'parse_gnttab_limit'

The macros above are intended to discard function arguments (unused1, unused2)
when compiling with different configurations of CONFIG_HYPFS.
This can lead to confusion and unexpected behavior
because the macro name and the function name are identical.
Split the code and create two distinct function signatures.
This ensures that the code behaves predictably and remains compliant.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
 xen/common/grant_table.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index cf131c43a1..f3282a1d7b 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -126,18 +126,12 @@ static void __init cf_check max_maptrack_frames_init(struct param_hypfs *par)
     update_gnttab_par(opt_max_maptrack_frames, par,
                       opt_max_maptrack_frames_val);
 }
-#else
-#define update_gnttab_par(v, unused1, unused2)     update_gnttab_par(v)
-#define parse_gnttab_limit(a, v, unused1, unused2) parse_gnttab_limit(a, v)
-
-static void update_gnttab_par(unsigned int val, struct param_hypfs *par,
-                              char *parval)
-{
-}
-#endif
 
 static int parse_gnttab_limit(const char *arg, unsigned int *valp,
                               struct param_hypfs *par, char *parval)
+#else
+static int parse_gnttab_limit(const char *arg, unsigned int *valp)
+#endif
 {
     const char *e;
     unsigned long val;
@@ -150,7 +144,9 @@ static int parse_gnttab_limit(const char *arg, unsigned int *valp,
         return -ERANGE;
 
     *valp = val;
+#ifdef CONFIG_HYPFS
     update_gnttab_par(val, par, parval);
+#endif
 
     return 0;
 }
@@ -161,9 +157,13 @@ custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames,
 
 static int cf_check parse_gnttab_max_frames(const char *arg)
 {
+#ifdef CONFIG_HYPFS
     return parse_gnttab_limit(arg, &opt_max_grant_frames,
                               param_2_parfs(parse_gnttab_max_frames),
                               opt_max_grant_frames_val);
+#else
+    return parse_gnttab_limit(arg, &opt_max_grant_frames);
+#endif
 }
 
 static int cf_check parse_gnttab_max_maptrack_frames(const char *arg);
@@ -173,9 +173,13 @@ custom_runtime_param("gnttab_max_maptrack_frames",
 
 static int cf_check parse_gnttab_max_maptrack_frames(const char *arg)
 {
+#ifdef CONFIG_HYPFS
     return parse_gnttab_limit(arg, &opt_max_maptrack_frames,
                               param_2_parfs(parse_gnttab_max_maptrack_frames),
                               opt_max_maptrack_frames_val);
+#else
+    return parse_gnttab_limit(arg, &opt_max_maptrack_frames);
+#endif
 }
 
 #ifndef GNTTAB_MAX_VERSION
-- 
2.43.0

[XEN PATCH 2/5] iommu: address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".

Reports for service MC3A2.R5.5:
xen/include/xen/iommu.h: non-compliant struct 'page_list_head'
xen/include/xen/mm.h: non-compliant macro 'page_list_head'
xen/drivers/passthrough/iommu.c: non-compliant macro 'iommu_quarantine'
xen/include/xen/iommu.h: non-compliant variable 'iommu_quarantine'

These external variables ('iommu_pt_cleanup_lock'
and 'iommu_pt_cleanup_list') are no longer used
in the codebase. Removing them eliminates dead
code and ensures compliance with coding standards.
Fixes: b5622eb627 (iommu: remove unused iommu_ops method and tasklet, 2020-09-22)

The variable 'iommu_quarantine' makes sence to use
only if 'CONFIG_HAS_PCI=y', so place it inside '#ifdef'.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
 xen/include/xen/iommu.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index ebfada1d88..57f338e2a0 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -53,7 +53,9 @@ static inline bool dfn_eq(dfn_t x, dfn_t y)
 extern bool iommu_enable, iommu_enabled;
 extern bool force_iommu, iommu_verbose;
 /* Boolean except for the specific purposes of drivers/passthrough/iommu.c. */
+#ifdef CONFIG_HAS_PCI
 extern uint8_t iommu_quarantine;
+#endif /* CONFIG_HAS_PCI */
 #else
 #define iommu_enabled false
 #endif
@@ -500,9 +502,6 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev);
  */
 DECLARE_PER_CPU(bool, iommu_dont_flush_iotlb);
 
-extern struct spinlock iommu_pt_cleanup_lock;
-extern struct page_list_head iommu_pt_cleanup_list;
-
 bool arch_iommu_use_permitted(const struct domain *d);
 
 #ifdef CONFIG_X86
-- 
2.43.0

[XEN PATCH 3/5] x86/irq: address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".

Reports for service MC3A2.R5.5:
xen/include/xen/irq.h: non-compliant function `pirq_cleanup_check(struct pirq*, struct domain*)'
xen/include/xen/irq.h: non-compliant macro `pirq_cleanup_check'

The primary issue stems from the macro and function
having identical names, which is confusing and
non-compliant with common coding standards.

Change the function name by adding two underscores at the end.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
 xen/arch/x86/irq.c    | 2 +-
 xen/include/xen/irq.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 556134f85a..d61720aa60 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1383,7 +1383,7 @@ struct pirq *alloc_pirq_struct(struct domain *d)
     return pirq;
 }
 
-void (pirq_cleanup_check)(struct pirq *pirq, struct domain *d)
+void pirq_cleanup_check__(struct pirq *pirq, struct domain *d)
 {
     /*
      * Check whether all fields have their default values, and delete
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 95034c0d6b..02f686a999 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -183,10 +183,10 @@ extern struct pirq *pirq_get_info(struct domain *d, int pirq);
 #define pirq_to_evtchn(d, pirq) pirq_field(d, pirq, evtchn, 0)
 #define pirq_masked(d, pirq) pirq_field(d, pirq, masked, 0)
 
-void pirq_cleanup_check(struct pirq *pirq, struct domain *d);
+void pirq_cleanup_check__(struct pirq *pirq, struct domain *d);
 
 #define pirq_cleanup_check(pirq, d) \
-    (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
+    (!(pirq)->evtchn ? pirq_cleanup_check__(pirq, d) : (void)0)
 
 extern void pirq_guest_eoi(struct pirq *pirq);
 extern void desc_guest_eoi(struct irq_desc *desc, struct pirq *pirq);
-- 
2.43.0

[XEN PATCH 4/5] device-tree: address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".

Reports for service MC3A2.R5.5:
xen/include/xen/fdt-domain-build.h: non-compliant parameter 'copy_to_guest'
xen/include/xen/guest_access.h: non-compliant macro 'copy_to_guest'

Rename 'copy_to_guest' function parameter to 'cb' for compliance.
No functional changes.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
 xen/common/device-tree/domain-build.c | 9 ++++-----
 xen/include/xen/fdt-domain-build.h    | 4 ++--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
index cd01a8b4bc..2b009547d0 100644
--- a/xen/common/device-tree/domain-build.c
+++ b/xen/common/device-tree/domain-build.c
@@ -331,7 +331,7 @@ void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
 }
 
 void __init dtb_load(struct kernel_info *kinfo,
-                     copy_to_guest_phys_cb copy_to_guest)
+                     copy_to_guest_phys_cb cb)
 {
     unsigned long left;
 
@@ -339,7 +339,7 @@ void __init dtb_load(struct kernel_info *kinfo,
            kinfo->d, kinfo->dtb_paddr,
            kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
 
-    left = copy_to_guest(kinfo->d, kinfo->dtb_paddr,
+    left = cb(kinfo->d, kinfo->dtb_paddr,
                          kinfo->fdt,
                          fdt_totalsize(kinfo->fdt));
 
@@ -350,7 +350,7 @@ void __init dtb_load(struct kernel_info *kinfo,
 }
 
 void __init initrd_load(struct kernel_info *kinfo,
-                        copy_to_guest_phys_cb copy_to_guest)
+                        copy_to_guest_phys_cb cb)
 {
     const struct boot_module *mod = kinfo->initrd;
     paddr_t load_addr = kinfo->initrd_paddr;
@@ -393,8 +393,7 @@ void __init initrd_load(struct kernel_info *kinfo,
     if ( !initrd )
         panic("Unable to map the %pd initrd\n", kinfo->d);
 
-    res = copy_to_guest(kinfo->d, load_addr,
-                        initrd, len);
+    res = cb(kinfo->d, load_addr, initrd, len);
     if ( res != 0 )
         panic("Unable to copy the initrd in the %pd memory\n", kinfo->d);
 
diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
index 45981dbec0..3a20623cf5 100644
--- a/xen/include/xen/fdt-domain-build.h
+++ b/xen/include/xen/fdt-domain-build.h
@@ -50,10 +50,10 @@ typedef unsigned long (*copy_to_guest_phys_cb)(struct domain *d,
                                                unsigned int len);
 
 void initrd_load(struct kernel_info *kinfo,
-                 copy_to_guest_phys_cb copy_to_guest);
+                 copy_to_guest_phys_cb cb);
 
 void dtb_load(struct kernel_info *kinfo,
-              copy_to_guest_phys_cb copy_to_guest);
+              copy_to_guest_phys_cb cb);
 
 int find_unallocated_memory(const struct kernel_info *kinfo,
                             const struct membanks *mem_banks[],
-- 
2.43.0

[XEN PATCH 5/5] xen/bitops: address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

Address a violation of MISRA C:2012 Rule 5.5:
"Identifiers shall be distinct from macro names".

Reports for service MC3A2.R5.5:
xen/include/xen/bitops.h: non-compliant function '__test_and_set_bit(int, volatile void*)'
xen/include/xen/bitops.h: non-compliant macro '__test_and_set_bit'
xen/include/xen/bitops.h: non-compliant function '__test_and_clear_bit(int, volatile void*)'
xen/include/xen/bitops.h: non-compliant macro '__test_and_clear_bit'
xen/include/xen/bitops.h: non-compliant function '__test_and_change_bit(int, volatile void*)'
xen/include/xen/bitops.h: non-compliant macro '__test_and_change_bit'
xen/include/xen/bitops.h: non-compliant function 'test_bit(int, const volatile void*)'
xen/include/xen/bitops.h: non-compliant macro 'test_bit'

The primary issue stems from the macro and function
having identical names, which is confusing and
non-compliant with common coding standards.
Change the functions names by adding two underscores at the end.
No functional changes.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
 xen/include/xen/bitops.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index a4d31ec02a..b292470ad7 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -120,7 +120,7 @@ static always_inline bool generic_test_bit(int nr, const volatile void *addr)
 }
 
 /**
- * __test_and_set_bit - Set a bit and return its old value
+ * __test_and_set_bit__ - Set a bit and return its old value
  * @nr: Bit to set
  * @addr: Address to count from
  *
@@ -129,7 +129,7 @@ static always_inline bool generic_test_bit(int nr, const volatile void *addr)
  * but actually fail.  You must protect multiple accesses with a lock.
  */
 static always_inline bool
-__test_and_set_bit(int nr, volatile void *addr)
+__test_and_set_bit__(int nr, volatile void *addr)
 {
 #ifndef arch__test_and_set_bit
 #define arch__test_and_set_bit generic__test_and_set_bit
@@ -139,11 +139,11 @@ __test_and_set_bit(int nr, volatile void *addr)
 }
 #define __test_and_set_bit(nr, addr) ({             \
     if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
-    __test_and_set_bit(nr, addr);                   \
+    __test_and_set_bit__(nr, addr);                 \
 })
 
 /**
- * __test_and_clear_bit - Clear a bit and return its old value
+ * __test_and_clear_bit__ - Clear a bit and return its old value
  * @nr: Bit to clear
  * @addr: Address to count from
  *
@@ -152,7 +152,7 @@ __test_and_set_bit(int nr, volatile void *addr)
  * but actually fail.  You must protect multiple accesses with a lock.
  */
 static always_inline bool
-__test_and_clear_bit(int nr, volatile void *addr)
+__test_and_clear_bit__(int nr, volatile void *addr)
 {
 #ifndef arch__test_and_clear_bit
 #define arch__test_and_clear_bit generic__test_and_clear_bit
@@ -162,11 +162,11 @@ __test_and_clear_bit(int nr, volatile void *addr)
 }
 #define __test_and_clear_bit(nr, addr) ({           \
     if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
-    __test_and_clear_bit(nr, addr);                 \
+    __test_and_clear_bit__(nr, addr);               \
 })
 
 /**
- * __test_and_change_bit - Change a bit and return its old value
+ * __test_and_change_bit__ - Change a bit and return its old value
  * @nr: Bit to change
  * @addr: Address to count from
  *
@@ -175,7 +175,7 @@ __test_and_clear_bit(int nr, volatile void *addr)
  * but actually fail.  You must protect multiple accesses with a lock.
  */
 static always_inline bool
-__test_and_change_bit(int nr, volatile void *addr)
+__test_and_change_bit__(int nr, volatile void *addr)
 {
 #ifndef arch__test_and_change_bit
 #define arch__test_and_change_bit generic__test_and_change_bit
@@ -185,11 +185,11 @@ __test_and_change_bit(int nr, volatile void *addr)
 }
 #define __test_and_change_bit(nr, addr) ({              \
     if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
-    __test_and_change_bit(nr, addr);                    \
+    __test_and_change_bit__(nr, addr);                  \
 })
 
 /**
- * test_bit - Determine whether a bit is set
+ * test_bit__ - Determine whether a bit is set
  * @nr: bit number to test
  * @addr: Address to start counting from
  *
@@ -197,7 +197,7 @@ __test_and_change_bit(int nr, volatile void *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static always_inline bool test_bit(int nr, const volatile void *addr)
+static always_inline bool test_bit__(int nr, const volatile void *addr)
 {
 #ifndef arch_test_bit
 #define arch_test_bit generic_test_bit
@@ -207,7 +207,7 @@ static always_inline bool test_bit(int nr, const volatile void *addr)
 }
 #define test_bit(nr, addr) ({                           \
     if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
-    test_bit(nr, addr);                                 \
+    test_bit__(nr, addr);                               \
 })
 
 /* --------------------- Please tidy above here --------------------- */
-- 
2.43.0

Re: [XEN PATCH 0/5] address violation of MISRA C Rule 5.5

[Dmytro Prokopchuk1]

Pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1907188989

On 7/4/25 23:39, Dmytro Prokopchuk1 wrote:
> This patch series fully eliminates MISRA C Rule 5.5
> violations for ARM64.
>
> The previous thread is here:
> https://lore.kernel.org/xen-devel/48c7830931a98b2bf70ef1509f309b262b9e5792.1745427770.git.victorm.lira@amd.com/
> where that violation was proposed to be deviated.
>
> Dmytro Prokopchuk (5):
>    gnttab: address violation of MISRA C Rule 5.5
>    iommu: address violation of MISRA C Rule 5.5
>    x86/irq: address violation of MISRA C Rule 5.5
>    device-tree: address violation of MISRA C Rule 5.5
>    xen/bitops: address violation of MISRA C Rule 5.5
>
>   xen/arch/x86/irq.c                    |  2 +-
>   xen/common/device-tree/domain-build.c |  9 ++++-----
>   xen/common/grant_table.c              | 22 +++++++++++++---------
>   xen/include/xen/bitops.h              | 24 ++++++++++++------------
>   xen/include/xen/fdt-domain-build.h    |  4 ++--
>   xen/include/xen/iommu.h               |  5 ++---
>   xen/include/xen/irq.h                 |  4 ++--
>   7 files changed, 36 insertions(+), 34 deletions(-)
>

Re: [XEN PATCH 2/5] iommu: address violation of MISRA C Rule 5.5

[Jan Beulich]

On 04.07.2025 22:39, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/include/xen/iommu.h: non-compliant struct 'page_list_head'
> xen/include/xen/mm.h: non-compliant macro 'page_list_head'

What is this about? There's no code change that I could see which would
alter the situation here.

> xen/drivers/passthrough/iommu.c: non-compliant macro 'iommu_quarantine'
> xen/include/xen/iommu.h: non-compliant variable 'iommu_quarantine'
> 
> These external variables ('iommu_pt_cleanup_lock'
> and 'iommu_pt_cleanup_list') are no longer used
> in the codebase. Removing them eliminates dead
> code and ensures compliance with coding standards.
> Fixes: b5622eb627 (iommu: remove unused iommu_ops method and tasklet, 2020-09-22)

That's a different Misra rule, so may better be put in a separate patch?
Otherwise please at least mention the rule number as well.

> The variable 'iommu_quarantine' makes sence to use
> only if 'CONFIG_HAS_PCI=y', so place it inside '#ifdef'.

Hmm, yes - not nice, but what do you do. I question "makes sense" though:
Quarantining is possible also without PCI, aiui. Just we don't that right
now.

Jan

Re: [XEN PATCH 3/5] x86/irq: address violation of MISRA C Rule 5.5

[Jan Beulich]

On 04.07.2025 22:39, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/include/xen/irq.h: non-compliant function `pirq_cleanup_check(struct pirq*, struct domain*)'
> xen/include/xen/irq.h: non-compliant macro `pirq_cleanup_check'
> 
> The primary issue stems from the macro and function
> having identical names, which is confusing and
> non-compliant with common coding standards.
> 
> Change the function name by adding two underscores at the end.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>

I'm not going to NAK this, but I dislike the transformation done. The aliasing
in this case was intentional, to avoid any caller appearing that would bypass
the macro. Yes, the double underscores will also stand out (as much as the
parenthesization that would have been needed to override the protection), but
still ...

Jan

Re: [XEN PATCH 2/5] iommu: address violation of MISRA C Rule 5.5

[Stefano Stabellini]

On Mon, 7 Jul 2025, Jan Beulich wrote:
> On 04.07.2025 22:39, Dmytro Prokopchuk1 wrote:
> > Address a violation of MISRA C:2012 Rule 5.5:
> > "Identifiers shall be distinct from macro names".
> > 
> > Reports for service MC3A2.R5.5:
> > xen/include/xen/iommu.h: non-compliant struct 'page_list_head'
> > xen/include/xen/mm.h: non-compliant macro 'page_list_head'
> 
> What is this about? There's no code change that I could see which would
> alter the situation here.
> 
> > xen/drivers/passthrough/iommu.c: non-compliant macro 'iommu_quarantine'
> > xen/include/xen/iommu.h: non-compliant variable 'iommu_quarantine'
> > 
> > These external variables ('iommu_pt_cleanup_lock'
> > and 'iommu_pt_cleanup_list') are no longer used
> > in the codebase. Removing them eliminates dead
> > code and ensures compliance with coding standards.
> > Fixes: b5622eb627 (iommu: remove unused iommu_ops method and tasklet, 2020-09-22)
> 
> That's a different Misra rule, so may better be put in a separate patch?
> Otherwise please at least mention the rule number as well.
> 
> > The variable 'iommu_quarantine' makes sence to use
> > only if 'CONFIG_HAS_PCI=y', so place it inside '#ifdef'.
> 
> Hmm, yes - not nice, but what do you do. I question "makes sense" though:
> Quarantining is possible also without PCI, aiui. Just we don't that right
> now.

Hi Jan,

As far as I can tell the change to #ifdef iommu_quarantine is necessary
to resolve a R5.5 violation here:

#ifdef CONFIG_HAS_PCI
uint8_t __read_mostly iommu_quarantine =
# if defined(CONFIG_IOMMU_QUARANTINE_NONE)
    IOMMU_quarantine_none;
# elif defined(CONFIG_IOMMU_QUARANTINE_BASIC)
    IOMMU_quarantine_basic;
# elif defined(CONFIG_IOMMU_QUARANTINE_SCRATCH_PAGE)
    IOMMU_quarantine_scratch_page;
# endif
#else
# define iommu_quarantine IOMMU_quarantine_none       <<< violation
#endif /* CONFIG_HAS_PCI */

As you can see from the patch series, often it is not nice to find a
resoltution for these R5.5 violations. This is the reason why I
originally suggested to deviate R5.5 entirely.

https://lore.kernel.org/xen-devel/139aa595-8b41-44e7-b205-415443c8c357@suse.com/](https://lore.kernel.org/xen-devel/139aa595-8b41-44e7-b205-415443c8c357@suse.com/

That said, this patch is one of the nicer changes in this series, I
think it is OK.

Re: [XEN PATCH 3/5] x86/irq: address violation of MISRA C Rule 5.5

[Stefano Stabellini]

On Mon, 7 Jul 2025, Jan Beulich wrote:
> On 04.07.2025 22:39, Dmytro Prokopchuk1 wrote:
> > Address a violation of MISRA C:2012 Rule 5.5:
> > "Identifiers shall be distinct from macro names".
> > 
> > Reports for service MC3A2.R5.5:
> > xen/include/xen/irq.h: non-compliant function `pirq_cleanup_check(struct pirq*, struct domain*)'
> > xen/include/xen/irq.h: non-compliant macro `pirq_cleanup_check'
> > 
> > The primary issue stems from the macro and function
> > having identical names, which is confusing and
> > non-compliant with common coding standards.
> > 
> > Change the function name by adding two underscores at the end.
> > 
> > Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
> 
> I'm not going to NAK this, but I dislike the transformation done. The aliasing
> in this case was intentional, to avoid any caller appearing that would bypass
> the macro. Yes, the double underscores will also stand out (as much as the
> parenthesization that would have been needed to override the protection), but
> still ...

Maybe you can suggest a different name? Looking at the diff, this patch
also seems OKish.

It is possible but difficult to deviate specific instances like this: if
a SAF in-code comment works, then great, otherwise we have to resort to
a regex which makes thing harder to maintain.

Unless a SAF in-code comment works, I think this patch is the best way
to go.

Re: [XEN PATCH 1/5] gnttab: address violation of MISRA C Rule 5.5

[Stefano Stabellini]

On Fri, 4 Jul 2025, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/common/grant_table.c: non-compliant macro 'update_gnttab_par'
> xen/common/grant_table.c: non-compliant macro 'parse_gnttab_limit'
> 
> The macros above are intended to discard function arguments (unused1, unused2)
> when compiling with different configurations of CONFIG_HYPFS.
> This can lead to confusion and unexpected behavior
> because the macro name and the function name are identical.
> Split the code and create two distinct function signatures.
> This ensures that the code behaves predictably and remains compliant.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>


I realize you tried to address Jan's comment about the global deviation.
In my opinion patch #2 and #3 are still OK, but I think this patch makes
things more confusing and error prone.

Can we find a way to deviate update_gnttab_par and parse_gnttab_limit
either with a SAF in-code comment (docs/misra/safe.json) or with a new
regex deviation (docs/misra/deviations.rst,
automation/eclair_analysis/ECLAIR/deviations.ecl)?


> ---
>  xen/common/grant_table.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index cf131c43a1..f3282a1d7b 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -126,18 +126,12 @@ static void __init cf_check max_maptrack_frames_init(struct param_hypfs *par)
>      update_gnttab_par(opt_max_maptrack_frames, par,
>                        opt_max_maptrack_frames_val);
>  }
> -#else
> -#define update_gnttab_par(v, unused1, unused2)     update_gnttab_par(v)
> -#define parse_gnttab_limit(a, v, unused1, unused2) parse_gnttab_limit(a, v)
> -
> -static void update_gnttab_par(unsigned int val, struct param_hypfs *par,
> -                              char *parval)
> -{
> -}
> -#endif
>  
>  static int parse_gnttab_limit(const char *arg, unsigned int *valp,
>                                struct param_hypfs *par, char *parval)
> +#else
> +static int parse_gnttab_limit(const char *arg, unsigned int *valp)
> +#endif
>  {
>      const char *e;
>      unsigned long val;
> @@ -150,7 +144,9 @@ static int parse_gnttab_limit(const char *arg, unsigned int *valp,
>          return -ERANGE;
>  
>      *valp = val;
> +#ifdef CONFIG_HYPFS
>      update_gnttab_par(val, par, parval);
> +#endif
>  
>      return 0;
>  }
> @@ -161,9 +157,13 @@ custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames,
>  
>  static int cf_check parse_gnttab_max_frames(const char *arg)
>  {
> +#ifdef CONFIG_HYPFS
>      return parse_gnttab_limit(arg, &opt_max_grant_frames,
>                                param_2_parfs(parse_gnttab_max_frames),
>                                opt_max_grant_frames_val);
> +#else
> +    return parse_gnttab_limit(arg, &opt_max_grant_frames);
> +#endif
>  }
>  
>  static int cf_check parse_gnttab_max_maptrack_frames(const char *arg);
> @@ -173,9 +173,13 @@ custom_runtime_param("gnttab_max_maptrack_frames",
>  
>  static int cf_check parse_gnttab_max_maptrack_frames(const char *arg)
>  {
> +#ifdef CONFIG_HYPFS
>      return parse_gnttab_limit(arg, &opt_max_maptrack_frames,
>                                param_2_parfs(parse_gnttab_max_maptrack_frames),
>                                opt_max_maptrack_frames_val);
> +#else
> +    return parse_gnttab_limit(arg, &opt_max_maptrack_frames);
> +#endif
>  }
>  
>  #ifndef GNTTAB_MAX_VERSION
> -- 
> 2.43.0
> 

Re: [XEN PATCH 4/5] device-tree: address violation of MISRA C Rule 5.5

[Stefano Stabellini]

On Fri, 4 Jul 2025, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/include/xen/fdt-domain-build.h: non-compliant parameter 'copy_to_guest'
> xen/include/xen/guest_access.h: non-compliant macro 'copy_to_guest'
> 
> Rename 'copy_to_guest' function parameter to 'cb' for compliance.
> No functional changes.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>

Nice!


> ---
>  xen/common/device-tree/domain-build.c | 9 ++++-----
>  xen/include/xen/fdt-domain-build.h    | 4 ++--
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/common/device-tree/domain-build.c b/xen/common/device-tree/domain-build.c
> index cd01a8b4bc..2b009547d0 100644
> --- a/xen/common/device-tree/domain-build.c
> +++ b/xen/common/device-tree/domain-build.c
> @@ -331,7 +331,7 @@ void __init allocate_memory(struct domain *d, struct kernel_info *kinfo)
>  }
>  
>  void __init dtb_load(struct kernel_info *kinfo,
> -                     copy_to_guest_phys_cb copy_to_guest)
> +                     copy_to_guest_phys_cb cb)
>  {
>      unsigned long left;
>  
> @@ -339,7 +339,7 @@ void __init dtb_load(struct kernel_info *kinfo,
>             kinfo->d, kinfo->dtb_paddr,
>             kinfo->dtb_paddr + fdt_totalsize(kinfo->fdt));
>  
> -    left = copy_to_guest(kinfo->d, kinfo->dtb_paddr,
> +    left = cb(kinfo->d, kinfo->dtb_paddr,
>                           kinfo->fdt,
>                           fdt_totalsize(kinfo->fdt));

NIT: code style, alignment

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


>  
> @@ -350,7 +350,7 @@ void __init dtb_load(struct kernel_info *kinfo,
>  }
>  
>  void __init initrd_load(struct kernel_info *kinfo,
> -                        copy_to_guest_phys_cb copy_to_guest)
> +                        copy_to_guest_phys_cb cb)
>  {
>      const struct boot_module *mod = kinfo->initrd;
>      paddr_t load_addr = kinfo->initrd_paddr;
> @@ -393,8 +393,7 @@ void __init initrd_load(struct kernel_info *kinfo,
>      if ( !initrd )
>          panic("Unable to map the %pd initrd\n", kinfo->d);
>  
> -    res = copy_to_guest(kinfo->d, load_addr,
> -                        initrd, len);
> +    res = cb(kinfo->d, load_addr, initrd, len);
>      if ( res != 0 )
>          panic("Unable to copy the initrd in the %pd memory\n", kinfo->d);
>  
> diff --git a/xen/include/xen/fdt-domain-build.h b/xen/include/xen/fdt-domain-build.h
> index 45981dbec0..3a20623cf5 100644
> --- a/xen/include/xen/fdt-domain-build.h
> +++ b/xen/include/xen/fdt-domain-build.h
> @@ -50,10 +50,10 @@ typedef unsigned long (*copy_to_guest_phys_cb)(struct domain *d,
>                                                 unsigned int len);
>  
>  void initrd_load(struct kernel_info *kinfo,
> -                 copy_to_guest_phys_cb copy_to_guest);
> +                 copy_to_guest_phys_cb cb);
>  
>  void dtb_load(struct kernel_info *kinfo,
> -              copy_to_guest_phys_cb copy_to_guest);
> +              copy_to_guest_phys_cb cb);
>  
>  int find_unallocated_memory(const struct kernel_info *kinfo,
>                              const struct membanks *mem_banks[],
> -- 
> 2.43.0
> 

Re: [XEN PATCH 5/5] xen/bitops: address violation of MISRA C Rule 5.5

[Stefano Stabellini]

On Fri, 4 Jul 2025, Dmytro Prokopchuk1 wrote:
> Address a violation of MISRA C:2012 Rule 5.5:
> "Identifiers shall be distinct from macro names".
> 
> Reports for service MC3A2.R5.5:
> xen/include/xen/bitops.h: non-compliant function '__test_and_set_bit(int, volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro '__test_and_set_bit'
> xen/include/xen/bitops.h: non-compliant function '__test_and_clear_bit(int, volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro '__test_and_clear_bit'
> xen/include/xen/bitops.h: non-compliant function '__test_and_change_bit(int, volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro '__test_and_change_bit'
> xen/include/xen/bitops.h: non-compliant function 'test_bit(int, const volatile void*)'
> xen/include/xen/bitops.h: non-compliant macro 'test_bit'
> 
> The primary issue stems from the macro and function
> having identical names, which is confusing and
> non-compliant with common coding standards.
> Change the functions names by adding two underscores at the end.
> No functional changes.
> 
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>

I think these should also be deviated either using a SAF in-code comment
if possible or a regex in
automation/eclair_analysis/ECLAIR/deviations.ecl and
docs/misra/deviations.rst



> ---
>  xen/include/xen/bitops.h | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
> index a4d31ec02a..b292470ad7 100644
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -120,7 +120,7 @@ static always_inline bool generic_test_bit(int nr, const volatile void *addr)
>  }
>  
>  /**
> - * __test_and_set_bit - Set a bit and return its old value
> + * __test_and_set_bit__ - Set a bit and return its old value
>   * @nr: Bit to set
>   * @addr: Address to count from
>   *
> @@ -129,7 +129,7 @@ static always_inline bool generic_test_bit(int nr, const volatile void *addr)
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
>  static always_inline bool
> -__test_and_set_bit(int nr, volatile void *addr)
> +__test_and_set_bit__(int nr, volatile void *addr)
>  {
>  #ifndef arch__test_and_set_bit
>  #define arch__test_and_set_bit generic__test_and_set_bit
> @@ -139,11 +139,11 @@ __test_and_set_bit(int nr, volatile void *addr)
>  }
>  #define __test_and_set_bit(nr, addr) ({             \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
> -    __test_and_set_bit(nr, addr);                   \
> +    __test_and_set_bit__(nr, addr);                 \
>  })
>  
>  /**
> - * __test_and_clear_bit - Clear a bit and return its old value
> + * __test_and_clear_bit__ - Clear a bit and return its old value
>   * @nr: Bit to clear
>   * @addr: Address to count from
>   *
> @@ -152,7 +152,7 @@ __test_and_set_bit(int nr, volatile void *addr)
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
>  static always_inline bool
> -__test_and_clear_bit(int nr, volatile void *addr)
> +__test_and_clear_bit__(int nr, volatile void *addr)
>  {
>  #ifndef arch__test_and_clear_bit
>  #define arch__test_and_clear_bit generic__test_and_clear_bit
> @@ -162,11 +162,11 @@ __test_and_clear_bit(int nr, volatile void *addr)
>  }
>  #define __test_and_clear_bit(nr, addr) ({           \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size(); \
> -    __test_and_clear_bit(nr, addr);                 \
> +    __test_and_clear_bit__(nr, addr);               \
>  })
>  
>  /**
> - * __test_and_change_bit - Change a bit and return its old value
> + * __test_and_change_bit__ - Change a bit and return its old value
>   * @nr: Bit to change
>   * @addr: Address to count from
>   *
> @@ -175,7 +175,7 @@ __test_and_clear_bit(int nr, volatile void *addr)
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
>  static always_inline bool
> -__test_and_change_bit(int nr, volatile void *addr)
> +__test_and_change_bit__(int nr, volatile void *addr)
>  {
>  #ifndef arch__test_and_change_bit
>  #define arch__test_and_change_bit generic__test_and_change_bit
> @@ -185,11 +185,11 @@ __test_and_change_bit(int nr, volatile void *addr)
>  }
>  #define __test_and_change_bit(nr, addr) ({              \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
> -    __test_and_change_bit(nr, addr);                    \
> +    __test_and_change_bit__(nr, addr);                  \
>  })
>  
>  /**
> - * test_bit - Determine whether a bit is set
> + * test_bit__ - Determine whether a bit is set
>   * @nr: bit number to test
>   * @addr: Address to start counting from
>   *
> @@ -197,7 +197,7 @@ __test_and_change_bit(int nr, volatile void *addr)
>   * If two examples of this operation race, one can appear to succeed
>   * but actually fail.  You must protect multiple accesses with a lock.
>   */
> -static always_inline bool test_bit(int nr, const volatile void *addr)
> +static always_inline bool test_bit__(int nr, const volatile void *addr)
>  {
>  #ifndef arch_test_bit
>  #define arch_test_bit generic_test_bit
> @@ -207,7 +207,7 @@ static always_inline bool test_bit(int nr, const volatile void *addr)
>  }
>  #define test_bit(nr, addr) ({                           \
>      if ( bitop_bad_size(addr) ) __bitop_bad_size();     \
> -    test_bit(nr, addr);                                 \
> +    test_bit__(nr, addr);                               \
>  })
>  
>  /* --------------------- Please tidy above here --------------------- */
> -- 
> 2.43.0
> 

Re: [XEN PATCH 2/5] iommu: address violation of MISRA C Rule 5.5

[Jan Beulich]

On 07.07.2025 23:21, Stefano Stabellini wrote:
> On Mon, 7 Jul 2025, Jan Beulich wrote:
>> On 04.07.2025 22:39, Dmytro Prokopchuk1 wrote:
>>> Address a violation of MISRA C:2012 Rule 5.5:
>>> "Identifiers shall be distinct from macro names".
>>>
>>> Reports for service MC3A2.R5.5:
>>> xen/include/xen/iommu.h: non-compliant struct 'page_list_head'
>>> xen/include/xen/mm.h: non-compliant macro 'page_list_head'
>>
>> What is this about? There's no code change that I could see which would
>> alter the situation here.
>>
>>> xen/drivers/passthrough/iommu.c: non-compliant macro 'iommu_quarantine'
>>> xen/include/xen/iommu.h: non-compliant variable 'iommu_quarantine'
>>>
>>> These external variables ('iommu_pt_cleanup_lock'
>>> and 'iommu_pt_cleanup_list') are no longer used
>>> in the codebase. Removing them eliminates dead
>>> code and ensures compliance with coding standards.
>>> Fixes: b5622eb627 (iommu: remove unused iommu_ops method and tasklet, 2020-09-22)
>>
>> That's a different Misra rule, so may better be put in a separate patch?
>> Otherwise please at least mention the rule number as well.
>>
>>> The variable 'iommu_quarantine' makes sence to use
>>> only if 'CONFIG_HAS_PCI=y', so place it inside '#ifdef'.
>>
>> Hmm, yes - not nice, but what do you do. I question "makes sense" though:
>> Quarantining is possible also without PCI, aiui. Just we don't that right
>> now.
> 
> As far as I can tell the change to #ifdef iommu_quarantine is necessary
> to resolve a R5.5 violation here:
> 
> #ifdef CONFIG_HAS_PCI
> uint8_t __read_mostly iommu_quarantine =
> # if defined(CONFIG_IOMMU_QUARANTINE_NONE)
>     IOMMU_quarantine_none;
> # elif defined(CONFIG_IOMMU_QUARANTINE_BASIC)
>     IOMMU_quarantine_basic;
> # elif defined(CONFIG_IOMMU_QUARANTINE_SCRATCH_PAGE)
>     IOMMU_quarantine_scratch_page;
> # endif
> #else
> # define iommu_quarantine IOMMU_quarantine_none       <<< violation
> #endif /* CONFIG_HAS_PCI */

Yes. And I expressed that I accept the need to this change. I merely
questioned the wording used in the description.

What I can't derive is why page_list_head is mentioned in the
description, but then there's no related code change.

> As you can see from the patch series, often it is not nice to find a
> resoltution for these R5.5 violations. This is the reason why I
> originally suggested to deviate R5.5 entirely.
> 
> https://lore.kernel.org/xen-devel/139aa595-8b41-44e7-b205-415443c8c357@suse.com/](https://lore.kernel.org/xen-devel/139aa595-8b41-44e7-b205-415443c8c357@suse.com/
> 
> That said, this patch is one of the nicer changes in this series, I
> think it is OK.

With some adjustment(s) to the description, perhaps.

Jan

Re: [XEN PATCH 3/5] x86/irq: address violation of MISRA C Rule 5.5

[Jan Beulich]

On 07.07.2025 23:25, Stefano Stabellini wrote:
> On Mon, 7 Jul 2025, Jan Beulich wrote:
>> On 04.07.2025 22:39, Dmytro Prokopchuk1 wrote:
>>> Address a violation of MISRA C:2012 Rule 5.5:
>>> "Identifiers shall be distinct from macro names".
>>>
>>> Reports for service MC3A2.R5.5:
>>> xen/include/xen/irq.h: non-compliant function `pirq_cleanup_check(struct pirq*, struct domain*)'
>>> xen/include/xen/irq.h: non-compliant macro `pirq_cleanup_check'
>>>
>>> The primary issue stems from the macro and function
>>> having identical names, which is confusing and
>>> non-compliant with common coding standards.
>>>
>>> Change the function name by adding two underscores at the end.
>>>
>>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
>>
>> I'm not going to NAK this, but I dislike the transformation done. The aliasing
>> in this case was intentional, to avoid any caller appearing that would bypass
>> the macro. Yes, the double underscores will also stand out (as much as the
>> parenthesization that would have been needed to override the protection), but
>> still ...
> 
> Maybe you can suggest a different name?

As per my earlier reply, using the same name was intentional here. Hence
it's not a matter of what (different) name to pick, but the mere fact that
a different name is being suggested to be used. Yet as said - I'm not
going to NAK this, but I also don't like the change.

Jan

> Looking at the diff, this patch also seems OKish.
> 
> It is possible but difficult to deviate specific instances like this: if
> a SAF in-code comment works, then great, otherwise we have to resort to
> a regex which makes thing harder to maintain.
> 
> Unless a SAF in-code comment works, I think this patch is the best way
> to go.