Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On Thu, Sep 24, 2020 at 08:47:05AM -0700, Randy Dunlap wrote: > On 9/24/20 3:24 AM, Eli Cohen wrote: > > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote: > --- linux-next-20200917.orig/drivers/vdpa/Kconfig > +++ linux-next-20200917/drivers/vdpa/Kconfig > @@ -31,7 +31,7 @@ config IFCVF > > config MLX5_VDPA > bool "MLX5 VDPA support library for ConnectX devices" > -depends on MLX5_CORE > +depends on VHOST_IOTLB && MLX5_CORE > default n > >>> > >>> While we are here, can anyone who apply this patch delete the "default n" > >>> line? > >>> It is by default "n". > > > > I can do that > > > >>> > >>> Thanks > >> > >> Hmm other drivers select VHOST_IOTLB, why not do the same? > > v1 used select, but Saeed requested use of depends instead because > select can cause problems. OK I went over the history. v1 selected VHOST, that as the issue I think. A later version switched to VHOST_IOTLB, that is ok to select. > > I can't see another driver doing that. Perhaps I can set dependency on > > VHOST which by itself depends on VHOST_IOTLB? > >> > >> > help > Support library for Mellanox VDPA drivers. Provides code that > is > > >> > > > -- > ~Randy ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On Tue, Sep 29, 2020 at 09:01:42AM +0300, Eli Cohen wrote: > On Fri, Sep 25, 2020 at 06:20:45AM -0400, Michael S. Tsirkin wrote: > > > > > > > > Hmm other drivers select VHOST_IOTLB, why not do the same? > > > > > > I can't see another driver doing that. > > > > Well grep VHOST_IOTLB and you will see some examples. > > $ git grep -wn VHOST_IOTLB > drivers/vhost/Kconfig:2:config VHOST_IOTLB > drivers/vhost/Kconfig:11: select VHOST_IOTLB > drivers/vhost/Kconfig:18: select VHOST_IOTLB > > What am I missing here? Nothing, there's a select here as expected. > > > Perhaps I can set dependency on > > > VHOST which by itself depends on VHOST_IOTLB? > > > > VHOST is processing virtio in the kernel. You don't really need that > > for mlx, do you? > > > > > > > > > > > > > > > > help > > > > > > Support library for Mellanox VDPA drivers. Provides code that > > > > > > is > > > > > > > > > > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On 2020/9/25 下午6:19, Michael S. Tsirkin wrote: On Fri, Sep 25, 2020 at 10:20:05AM +0300, Leon Romanovsky wrote: On Thu, Sep 24, 2020 at 12:02:43PM -0400, Michael S. Tsirkin wrote: On Thu, Sep 24, 2020 at 08:47:05AM -0700, Randy Dunlap wrote: On 9/24/20 3:24 AM, Eli Cohen wrote: On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote: --- linux-next-20200917.orig/drivers/vdpa/Kconfig +++ linux-next-20200917/drivers/vdpa/Kconfig @@ -31,7 +31,7 @@ config IFCVF config MLX5_VDPA bool "MLX5 VDPA support library for ConnectX devices" - depends on MLX5_CORE + depends on VHOST_IOTLB && MLX5_CORE default n While we are here, can anyone who apply this patch delete the "default n" line? It is by default "n". I can do that Thanks Hmm other drivers select VHOST_IOTLB, why not do the same? v1 used select, but Saeed requested use of depends instead because select can cause problems. I can't see another driver doing that. Perhaps I can set dependency on VHOST which by itself depends on VHOST_IOTLB? help Support library for Mellanox VDPA drivers. Provides code that is Saeed what kind of problems? It's used with select in other places, isn't it? IMHO, "depends" is much more explicit than "select". Thanks This is now how VHOST_IOTLB has been designed though. If you want to change VHOST_IOTLB to depends I think we should do it consistently all over. config VHOST_IOTLB tristate help Generic IOTLB implementation for vhost and vringh. This option is selected by any driver which needs to support an IOMMU in software. Yes, since there's no prompt for VHOST_IOTLB which means, if there's no other symbol that select VHOST_IOTLB, you can't enable MLX5 at all. See kconfig-language.rst: In general use select only for non-visible symbols (no prompts anywhere) and for symbols with no dependencies. That will limit the usefulness but on the other hand avoid the illegal configurations all over. Thanks -- ~Randy ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On Thu, Sep 24, 2020 at 01:24:13PM +0300, Eli Cohen wrote: > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote: > > > > --- linux-next-20200917.orig/drivers/vdpa/Kconfig > > > > +++ linux-next-20200917/drivers/vdpa/Kconfig > > > > @@ -31,7 +31,7 @@ config IFCVF > > > > > > > > config MLX5_VDPA > > > > bool "MLX5 VDPA support library for ConnectX devices" > > > > - depends on MLX5_CORE > > > > + depends on VHOST_IOTLB && MLX5_CORE > > > > default n > > > > > > While we are here, can anyone who apply this patch delete the "default n" > > > line? > > > It is by default "n". > > I can do that > > > > > > > Thanks > > > > Hmm other drivers select VHOST_IOTLB, why not do the same? > > I can't see another driver doing that. Well grep VHOST_IOTLB and you will see some examples. > Perhaps I can set dependency on > VHOST which by itself depends on VHOST_IOTLB? VHOST is processing virtio in the kernel. You don't really need that for mlx, do you? > > > > > > > > help > > > > Support library for Mellanox VDPA drivers. Provides code that > > > > is > > > > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On Fri, Sep 25, 2020 at 10:20:05AM +0300, Leon Romanovsky wrote: > On Thu, Sep 24, 2020 at 12:02:43PM -0400, Michael S. Tsirkin wrote: > > On Thu, Sep 24, 2020 at 08:47:05AM -0700, Randy Dunlap wrote: > > > On 9/24/20 3:24 AM, Eli Cohen wrote: > > > > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote: > > > --- linux-next-20200917.orig/drivers/vdpa/Kconfig > > > +++ linux-next-20200917/drivers/vdpa/Kconfig > > > @@ -31,7 +31,7 @@ config IFCVF > > > > > > config MLX5_VDPA > > > bool "MLX5 VDPA support library for ConnectX devices" > > > -depends on MLX5_CORE > > > +depends on VHOST_IOTLB && MLX5_CORE > > > default n > > > >>> > > > >>> While we are here, can anyone who apply this patch delete the > > > >>> "default n" line? > > > >>> It is by default "n". > > > > > > > > I can do that > > > > > > > >>> > > > >>> Thanks > > > >> > > > >> Hmm other drivers select VHOST_IOTLB, why not do the same? > > > > > > v1 used select, but Saeed requested use of depends instead because > > > select can cause problems. > > > > > > > I can't see another driver doing that. Perhaps I can set dependency on > > > > VHOST which by itself depends on VHOST_IOTLB? > > > >> > > > >> > > > help > > > Support library for Mellanox VDPA drivers. Provides code that > > > is > > > > > > >> > > > > > > > Saeed what kind of problems? It's used with select in other places, > > isn't it? > > IMHO, "depends" is much more explicit than "select". > > Thanks This is now how VHOST_IOTLB has been designed though. If you want to change VHOST_IOTLB to depends I think we should do it consistently all over. config VHOST_IOTLB tristate help Generic IOTLB implementation for vhost and vringh. This option is selected by any driver which needs to support an IOMMU in software. > > > > > -- > > > ~Randy > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On Thu, Sep 24, 2020 at 08:47:05AM -0700, Randy Dunlap wrote: > On 9/24/20 3:24 AM, Eli Cohen wrote: > > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote: > --- linux-next-20200917.orig/drivers/vdpa/Kconfig > +++ linux-next-20200917/drivers/vdpa/Kconfig > @@ -31,7 +31,7 @@ config IFCVF > > config MLX5_VDPA > bool "MLX5 VDPA support library for ConnectX devices" > -depends on MLX5_CORE > +depends on VHOST_IOTLB && MLX5_CORE > default n > >>> > >>> While we are here, can anyone who apply this patch delete the "default n" > >>> line? > >>> It is by default "n". > > > > I can do that > > > >>> > >>> Thanks > >> > >> Hmm other drivers select VHOST_IOTLB, why not do the same? > > v1 used select, but Saeed requested use of depends instead because > select can cause problems. > > > I can't see another driver doing that. Perhaps I can set dependency on > > VHOST which by itself depends on VHOST_IOTLB? > >> > >> > help > Support library for Mellanox VDPA drivers. Provides code that > is > > >> > Saeed what kind of problems? It's used with select in other places, isn't it? > -- > ~Randy ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On 9/24/20 3:24 AM, Eli Cohen wrote: > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote: --- linux-next-20200917.orig/drivers/vdpa/Kconfig +++ linux-next-20200917/drivers/vdpa/Kconfig @@ -31,7 +31,7 @@ config IFCVF config MLX5_VDPA bool "MLX5 VDPA support library for ConnectX devices" - depends on MLX5_CORE + depends on VHOST_IOTLB && MLX5_CORE default n >>> >>> While we are here, can anyone who apply this patch delete the "default n" >>> line? >>> It is by default "n". > > I can do that > >>> >>> Thanks >> >> Hmm other drivers select VHOST_IOTLB, why not do the same? v1 used select, but Saeed requested use of depends instead because select can cause problems. > I can't see another driver doing that. Perhaps I can set dependency on > VHOST which by itself depends on VHOST_IOTLB? >> >> help Support library for Mellanox VDPA drivers. Provides code that is >> -- ~Randy ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors
On Fri, Sep 18, 2020 at 11:22:45AM +0300, Leon Romanovsky wrote: > On Thu, Sep 17, 2020 at 07:35:03PM -0700, Randy Dunlap wrote: > > From: Randy Dunlap > > > > drivers/vdpa/mlx5/ uses vhost_iotlb*() interfaces, so add a dependency > > on VHOST to eliminate build errors. > > > > ld: drivers/vdpa/mlx5/core/mr.o: in function `add_direct_chain': > > mr.c:(.text+0x106): undefined reference to `vhost_iotlb_itree_first' > > ld: mr.c:(.text+0x1cf): undefined reference to `vhost_iotlb_itree_next' > > ld: mr.c:(.text+0x30d): undefined reference to `vhost_iotlb_itree_first' > > ld: mr.c:(.text+0x3e8): undefined reference to `vhost_iotlb_itree_next' > > ld: drivers/vdpa/mlx5/core/mr.o: in function `_mlx5_vdpa_create_mr': > > mr.c:(.text+0x908): undefined reference to `vhost_iotlb_itree_first' > > ld: mr.c:(.text+0x9e6): undefined reference to `vhost_iotlb_itree_next' > > ld: drivers/vdpa/mlx5/core/mr.o: in function `mlx5_vdpa_handle_set_map': > > mr.c:(.text+0xf1d): undefined reference to `vhost_iotlb_itree_first' > > > > Signed-off-by: Randy Dunlap > > Cc: "Michael S. Tsirkin" > > Cc: Jason Wang > > Cc: virtualization@lists.linux-foundation.org > > Cc: Saeed Mahameed > > Cc: Leon Romanovsky > > Cc: net...@vger.kernel.org > > --- > > v2: change from select to depends on VHOST (Saeed) > > v3: change to depends on VHOST_IOTLB (Jason) > > > > drivers/vdpa/Kconfig |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- linux-next-20200917.orig/drivers/vdpa/Kconfig > > +++ linux-next-20200917/drivers/vdpa/Kconfig > > @@ -31,7 +31,7 @@ config IFCVF > > > > config MLX5_VDPA > > bool "MLX5 VDPA support library for ConnectX devices" > > - depends on MLX5_CORE > > + depends on VHOST_IOTLB && MLX5_CORE > > default n > > While we are here, can anyone who apply this patch delete the "default n" > line? > It is by default "n". > > Thanks Hmm other drivers select VHOST_IOTLB, why not do the same? > > help > > Support library for Mellanox VDPA drivers. Provides code that is > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
