Michael, On Sat, Feb 05, 2022 at 02:41:31PM +0000, Michael Kelley (LINUX) wrote: > From: Vitaly Chikunov <vt@xxxxxxxxxxxx> Sent: Friday, February 4, 2022 8:06 PM > > > > On Fri, Feb 04, 2022 at 04:10:49PM +0000, Michael Kelley (LINUX) wrote: > > > 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. > > > > Yes this compiles well on Clang. > > > > Vitaly, > > > > Do you want to submit a patch with my approach, or should I do it? I'm > happy to do it, but I don't want to pre-empt what you have already > started without your OK. I'm OK if you send it! Thanks, > > Michael > > > > > > > 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;