Re: [XEN PATCH][for-4.19 8/9] xen/types: address Rule 10.1 for DECLARE_BITMAP use

[Julien Grall <julien@xxxxxxx>]

Hi Nicola,

On 06/10/2023 09:26, Nicola Vetrini wrote:
> 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'.
>
> A comment in BITS_TO_LONGS is added to make it clear that
> values passed are meant to be positive.
I am confused. If the value is meant to be positive. Then...

> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
>   xen/include/xen/iommu.h | 2 +-
>   xen/include/xen/types.h | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 0e747b0bbc1c..34aa0b9b5b81 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -360,7 +360,7 @@ struct domain_iommu {
>   #endif
>   
>       /* Features supported by the IOMMU */
> -    DECLARE_BITMAP(features, IOMMU_FEAT_count);
> +    DECLARE_BITMAP(features, (int)IOMMU_FEAT_count);
... why do we cast to (int) rather than (unsigned int)? Also, I think 
this cast deserve a comment on top because this is not a very obvious one.
>   
>       /* Does the guest share HAP mapping with the IOMMU? */
>       bool hap_pt_share;
> diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
> index aea259db1ef2..936e83d333a0 100644
> --- a/xen/include/xen/types.h
> +++ b/xen/include/xen/types.h
> @@ -22,6 +22,7 @@ typedef signed long ssize_t;
>   
>   typedef __PTRDIFF_TYPE__ ptrdiff_t;
>   
> +/* Users of this macro are expected to pass a positive value */
>   #define BITS_TO_LONGS(bits) \
>       (((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
>   #define DECLARE_BITMAP(name,bits) \
Cheers,

--
Julien Grall