All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion
@ 2022-02-04 13:05 Vitaly Chikunov
  2022-02-04 16:10 ` Michael Kelley (LINUX)
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Chikunov @ 2022-02-04 13:05 UTC (permalink / raw)
  To: linux-hyperv, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Wei Liu, Dexuan Cui
  Cc: Vitaly Chikunov, Tianyu Lan, Michael Kelley, Long Li

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@microsoft.com>
Cc: Michael Kelley <mikelley@microsoft.com>
Cc: Long Li <longli@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion
  2022-02-04 13:05 [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion Vitaly Chikunov
@ 2022-02-04 16:10 ` Michael Kelley (LINUX)
  2022-02-05  4:05   ` Vitaly Chikunov
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Kelley (LINUX) @ 2022-02-04 16:10 UTC (permalink / raw)
  To: Vitaly Chikunov, linux-hyperv, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, Dexuan Cui, Jakub Kicinski
  Cc: Tianyu Lan, Long Li

From: Vitaly Chikunov <vt@altlinux.org> 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@microsoft.com>
> Cc: Michael Kelley <mikelley@microsoft.com>
> Cc: Long Li <longli@microsoft.com>
> Cc: Wei Liu <wei.liu@kernel.org>
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> ---
>  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;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion
  2022-02-04 16:10 ` Michael Kelley (LINUX)
@ 2022-02-05  4:05   ` Vitaly Chikunov
  2022-02-05  8:06     ` Vitaly Chikunov
  2022-02-05 14:41     ` Michael Kelley (LINUX)
  0 siblings, 2 replies; 6+ messages in thread
From: Vitaly Chikunov @ 2022-02-05  4:05 UTC (permalink / raw)
  To: Michael Kelley (LINUX)
  Cc: linux-hyperv, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Wei Liu, Dexuan Cui, Jakub Kicinski, Tianyu Lan, Long Li

On Fri, Feb 04, 2022 at 04:10:49PM +0000, Michael Kelley (LINUX) wrote:
> From: Vitaly Chikunov <vt@altlinux.org> 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@microsoft.com>
> > Cc: Michael Kelley <mikelley@microsoft.com>
> > Cc: Long Li <longli@microsoft.com>
> > Cc: Wei Liu <wei.liu@kernel.org>
> > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > ---
> >  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,

> 
> 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;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion
  2022-02-05  4:05   ` Vitaly Chikunov
@ 2022-02-05  8:06     ` Vitaly Chikunov
  2022-02-05 14:41     ` Michael Kelley (LINUX)
  1 sibling, 0 replies; 6+ messages in thread
From: Vitaly Chikunov @ 2022-02-05  8:06 UTC (permalink / raw)
  To: Michael Kelley (LINUX)
  Cc: linux-hyperv, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Wei Liu, Dexuan Cui, Jakub Kicinski, Tianyu Lan, Long Li

On Sat, Feb 05, 2022 at 07:05:53AM +0300, Vitaly Chikunov wrote:
> On Fri, Feb 04, 2022 at 04:10:49PM +0000, Michael Kelley (LINUX) wrote:
> > From: Vitaly Chikunov <vt@altlinux.org> 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@microsoft.com>
> > > Cc: Michael Kelley <mikelley@microsoft.com>
> > > Cc: Long Li <longli@microsoft.com>
> > > Cc: Wei Liu <wei.liu@kernel.org>
> > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > ---
> > >  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.

How to reproduce in Docker (as in 'totally non-secure'):

  kernel-src$ make mrproper
  kernel-src$ docker run --rm -it -v $PWD:/src -w /src kernelci/clang-12
  in-docker# make CC=clang LLVM=1 O=/tmp/k allyesconfig drivers/hv/

`make mrproper` is required for `O=`, where `O=` is to not pollute
kernel tree (which will be rw bind mounted into /src) with root owned
files (since Docker runs under real root by default), `allyesconfig` is
to enable `HYPERV=y` the easy way. There are also other clang versions up
to clang-14 (they all fail with that global DMA_BIT_MASK(64) error).

Instead of `-v $PWD:/src -w /src` options to docker you can do git clone
inside, this is perhaps will be slightly more secure, and you don't need
to mrproper your tree (which will destroy .config).

Vitaly,

> 
> Vitaly,
> 
> > 
> > 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;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion
  2022-02-05  4:05   ` Vitaly Chikunov
  2022-02-05  8:06     ` Vitaly Chikunov
@ 2022-02-05 14:41     ` Michael Kelley (LINUX)
  2022-02-06  1:02       ` Vitaly Chikunov
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Kelley (LINUX) @ 2022-02-05 14:41 UTC (permalink / raw)
  To: Vitaly Chikunov
  Cc: linux-hyperv, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Wei Liu, Dexuan Cui, Jakub Kicinski, Tianyu Lan, Long Li

From: Vitaly Chikunov <vt@altlinux.org> 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@altlinux.org> 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@microsoft.com>
> > > Cc: Michael Kelley <mikelley@microsoft.com>
> > > Cc: Long Li <longli@microsoft.com>
> > > Cc: Wei Liu <wei.liu@kernel.org>
> > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > ---
> > >  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.

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;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion
  2022-02-05 14:41     ` Michael Kelley (LINUX)
@ 2022-02-06  1:02       ` Vitaly Chikunov
  0 siblings, 0 replies; 6+ messages in thread
From: Vitaly Chikunov @ 2022-02-06  1:02 UTC (permalink / raw)
  To: Michael Kelley (LINUX)
  Cc: linux-hyperv, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Wei Liu, Dexuan Cui, Jakub Kicinski, Tianyu Lan, Long Li

Michael,

On Sat, Feb 05, 2022 at 02:41:31PM +0000, Michael Kelley (LINUX) wrote:
> From: Vitaly Chikunov <vt@altlinux.org> 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@altlinux.org> 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@microsoft.com>
> > > > Cc: Michael Kelley <mikelley@microsoft.com>
> > > > Cc: Long Li <longli@microsoft.com>
> > > > Cc: Wei Liu <wei.liu@kernel.org>
> > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > > > ---
> > > >  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;

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-02-06  1:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-04 13:05 [PATCH v3] drivers: hv: vmbus: Fix build on Clang 12 failed by DMA_BIT_MASK(64) expansion Vitaly Chikunov
2022-02-04 16:10 ` Michael Kelley (LINUX)
2022-02-05  4:05   ` Vitaly Chikunov
2022-02-05  8:06     ` Vitaly Chikunov
2022-02-05 14:41     ` Michael Kelley (LINUX)
2022-02-06  1:02       ` Vitaly Chikunov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.