Given its use in the declaration
'DECLARE_BITMAP(features, IOMMU_FEAT_count)' the argument
'bits' has essential type 'enum iommu_feature', which is not
allowed by the Rule as an operand to the addition operator
in macro 'BITS_TO_LONGS'.

This construct is deviated with a deviation comment.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 docs/misra/safe.json    | 8 ++++++++
 xen/include/xen/iommu.h | 1 +
 xen/include/xen/types.h | 8 ++++++++
 3 files changed, 17 insertions(+)

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 39c5c056c7d4..952324f85cf9 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -20,6 +20,14 @@
         },
         {
             "id": "SAF-2-safe",
+            "analyser": {
+                "eclair": "MC3R1.R10.1"
+            },
+            "name": "MC3R1.R10.1: use of an enumeration constant in an arithmetic operation",
+            "text": "This violation can be fixed with a cast to (int) of the enumeration constant, but a deviation was chosen due to code readability (see also the comment in BITS_TO_LONGS)."
+        },
+        {
+            "id": "SAF-3-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 0e747b0bbc1c..d5c25770915b 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -360,6 +360,7 @@ struct domain_iommu {
 #endif
 
     /* Features supported by the IOMMU */
+    /* SAF-2-safe enum constant in arithmetic operation */
     DECLARE_BITMAP(features, IOMMU_FEAT_count);
 
     /* Does the guest share HAP mapping with the IOMMU? */
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index aea259db1ef2..50171ea1ad28 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -22,6 +22,14 @@ typedef signed long ssize_t;
 
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 
+/*
+ * Users of this macro are expected to pass a positive value.
+ *
+ * Eventually, this should become an unsigned quantity, but this
+ * requires fixing various uses of this macro and BITS_PER_LONG in signed
+ * contexts, such as type-safe 'min' macro uses, which give rise to build errors
+ * when the arguments have differing signedness, due to the build flags used.
+ */
 #define BITS_TO_LONGS(bits) \
     (((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
 #define DECLARE_BITMAP(name,bits) \
-- 
2.34.1

On 12.10.2023 17:28, Nicola Vetrini wrote:
> --- a/xen/include/xen/types.h
> +++ b/xen/include/xen/types.h
> @@ -22,6 +22,14 @@ typedef signed long ssize_t;
>  
>  typedef __PTRDIFF_TYPE__ ptrdiff_t;
>  
> +/*
> + * Users of this macro are expected to pass a positive value.

Is passing 0 going to cause any issues?

> + * Eventually, this should become an unsigned quantity, but this
> + * requires fixing various uses of this macro and BITS_PER_LONG in signed
> + * contexts, such as type-safe 'min' macro uses, which give rise to build errors
> + * when the arguments have differing signedness, due to the build flags used.
> + */

I'm not convinced of the usefulness of this part of the comment.

Jan

>  #define BITS_TO_LONGS(bits) \
>      (((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
>  #define DECLARE_BITMAP(name,bits) \

On 16/10/2023 17:49, Jan Beulich wrote:
> On 12.10.2023 17:28, Nicola Vetrini wrote:
>> --- a/xen/include/xen/types.h
>> +++ b/xen/include/xen/types.h
>> @@ -22,6 +22,14 @@ typedef signed long ssize_t;
>> 
>>  typedef __PTRDIFF_TYPE__ ptrdiff_t;
>> 
>> +/*
>> + * Users of this macro are expected to pass a positive value.
> 
> Is passing 0 going to cause any issues?
> 

I don't think so, even if that wouldn't make much sense. Given that the 
usage of the
zero lenght array extension is documented, that shouldn't be a concern 
either.

>> + * Eventually, this should become an unsigned quantity, but this
>> + * requires fixing various uses of this macro and BITS_PER_LONG in 
>> signed
>> + * contexts, such as type-safe 'min' macro uses, which give rise to 
>> build errors
>> + * when the arguments have differing signedness, due to the build 
>> flags used.
>> + */
> 
> I'm not convinced of the usefulness of this part of the comment.
> 
> Jan
> 

Isn't it useful to record why it was left as-is, and what should be done 
about it?
If it's not, this can be dropped on commit, in my opinion.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)