| Message ID | 73f7e48b-8d16-6b20-07d3-41dee0e3d3bd@infradead.org |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v3,-next] vdpa: mlx5: change Kconfig depends to fix build errors | expand |
On Thu, Sep 17, 2020 at 07:35:03PM -0700, Randy Dunlap wrote: > From: Randy Dunlap <rdunlap@infradead.org> > > 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 <rdunlap@infradead.org> > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: virtualization@lists.linux-foundation.org > Cc: Saeed Mahameed <saeedm@nvidia.com> > Cc: Leon Romanovsky <leonro@nvidia.com> > Cc: netdev@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 > help > Support library for Mellanox VDPA drivers. Provides code that is >
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 <rdunlap@infradead.org> > > > > 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 <rdunlap@infradead.org> > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > Cc: Jason Wang <jasowang@redhat.com> > > Cc: virtualization@lists.linux-foundation.org > > Cc: Saeed Mahameed <saeedm@nvidia.com> > > Cc: Leon Romanovsky <leonro@nvidia.com> > > Cc: netdev@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 > >
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. 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 > > > >
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 >>>> >>
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
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 > > > -- > > ~Randy >
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 > >
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 > > > > > >
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
On Fri, Sep 25, 2020 at 07:29:24PM +0800, Jason Wang wrote: > > 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, I wasn't aware of this clarification. > > Thanks > > > > > > > > > > > -- > > > > > ~Randy >
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? > > 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 > > > > > > > > >
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 > > > > > > > > > > > >
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
--- 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 help Support library for Mellanox VDPA drivers. Provides code that is