RE: [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion

["Michael Kelley (LINUX)" <mikelley@xxxxxxxxxxxxx>]

From: Vitaly Chikunov <vt@xxxxxxxxxxxx> Sent: Friday, February 4, 2022 5:05 AM
> 
> Clang 12.0.1 cannot understand that value 64 is excluded from the shift
> at compile time (for use in global context) resulting in build error:
> 
>   drivers/hv/vmbus_drv.c:2082:29: error: shift count >= width of type [-Werror,-
> Wshift-count-overflow]
>   static u64 vmbus_dma_mask = DMA_BIT_MASK(64);
> 			      ^~~~~~~~~~~~~~~~
>   ./include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK'
>   #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> 						       ^ ~~~
> 
> Avoid using DMA_BIT_MASK macro for that corner case.
> 
> Cc: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx>
> Cc: Michael Kelley <mikelley@xxxxxxxxxxxxx>
> Cc: Long Li <longli@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu@xxxxxxxxxx>
> Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
> ---
>  drivers/hv/vmbus_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 17bf55fe3169..a1306ca15d3f 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2079,7 +2079,8 @@ struct hv_device *vmbus_device_create(const guid_t *type,
>  	return child_device_obj;
>  }
> 
> -static u64 vmbus_dma_mask = DMA_BIT_MASK(64);
> +/* Use ~0ULL instead of DMA_BIT_MASK(64) to work around a bug in Clang. */
> +static u64 vmbus_dma_mask = ~0ULL;
>  /*
>   * vmbus_device_register - Register the child device
>   */
> --
> 2.33.0

Instead of the hack approach, does the following code rearrangement solve
the problem by eliminating the use of DMA_BIT_MASK(64) in a static initializer?
I don't have Clang handy to try it.

This approach also more closely follows the pattern used in other device types.

Michael


diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 17bf55f..0d96634 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2079,7 +2079,6 @@ struct hv_device *vmbus_device_create(const guid_t *type,
 	return child_device_obj;
 }
 
-static u64 vmbus_dma_mask = DMA_BIT_MASK(64);
 /*
  * vmbus_device_register - Register the child device
  */
@@ -2120,8 +2119,9 @@ int vmbus_device_register(struct hv_device *child_device_obj)
 	}
 	hv_debug_add_dev_dir(child_device_obj);
 
-	child_device_obj->device.dma_mask = &vmbus_dma_mask;
 	child_device_obj->device.dma_parms = &child_device_obj->dma_parms;
+	child_device_obj->device.dma_mask = &child_device_obj->dma_mask;
+	dma_set_mask(&child_device_obj->device, DMA_BIT_MASK(64));
 	return 0;
 
 err_kset_unregister:
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index f565a89..fe2e017 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1262,6 +1262,7 @@ struct hv_device {
 	struct vmbus_channel *channel;
 	struct kset	     *channels_kset;
 	struct device_dma_parameters dma_parms;
+	u64 dma_mask;
 
 	/* place holder to keep track of the dir for hv device in debugfs */
 	struct dentry *debug_dir;