linux-kernel - Re: [PATCH 11/15] RDMA/hbl: add habanalabs RDMA driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 28 Jun 2024 10:24:32 +0000
From: Omer Shpigelman <oshpigelman@...ana.ai>
To: Leon Romanovsky <leon@...nel.org>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "ogabbay@...nel.org" <ogabbay@...nel.org>,
        Zvika Yehudai <zyehudai@...ana.ai>
Subject: Re: [PATCH 11/15] RDMA/hbl: add habanalabs RDMA driver

On 6/19/24 13:52, Leon Romanovsky wrote:
> On Wed, Jun 19, 2024 at 09:27:54AM +0000, Omer Shpigelman wrote:
>> On 6/18/24 15:58, Leon Romanovsky wrote:
>>> On Tue, Jun 18, 2024 at 11:08:34AM +0000, Omer Shpigelman wrote:
>>>> On 6/17/24 22:04, Leon Romanovsky wrote:
>>>>> [Some people who received this message don't often get email from leon@...nel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>>
>>>>> On Mon, Jun 17, 2024 at 05:43:49PM +0000, Omer Shpigelman wrote:
>>>>>> On 6/13/24 22:18, Leon Romanovsky wrote:
>>>>>>> [Some people who received this message don't often get email from leon@...nel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>>>>
>>>>>>> On Thu, Jun 13, 2024 at 11:22:04AM +0300, Omer Shpigelman wrote:
>>>>>>>> Add an RDMA driver of Gaudi ASICs family for AI scaling.
>>>>>>>> The driver itself is agnostic to the ASIC in action, it operates according
>>>>>>>> to the capabilities that were passed on device initialization.
>>>>>>>> The device is initialized by the hbl_cn driver via auxiliary bus.
>>>>>>>> The driver also supports QP resource tracking and port/device HW counters.
>>>>>>>>
>>>>>>>> Signed-off-by: Omer Shpigelman <oshpigelman@...ana.ai>
>>>>>>>> Co-developed-by: Abhilash K V <kvabhilash@...ana.ai>
>>>>>>>> Signed-off-by: Abhilash K V <kvabhilash@...ana.ai>
>>>>>>>> Co-developed-by: Andrey Agranovich <aagranovich@...ana.ai>
>>>>>>>> Signed-off-by: Andrey Agranovich <aagranovich@...ana.ai>
>>>>>>>> Co-developed-by: Bharat Jauhari <bjauhari@...ana.ai>
>>>>>>>> Signed-off-by: Bharat Jauhari <bjauhari@...ana.ai>
>>>>>>>> Co-developed-by: David Meriin <dmeriin@...ana.ai>
>>>>>>>> Signed-off-by: David Meriin <dmeriin@...ana.ai>
>>>>>>>> Co-developed-by: Sagiv Ozeri <sozeri@...ana.ai>
>>>>>>>> Signed-off-by: Sagiv Ozeri <sozeri@...ana.ai>
>>>>>>>> Co-developed-by: Zvika Yehudai <zyehudai@...ana.ai>
>>>>>>>> Signed-off-by: Zvika Yehudai <zyehudai@...ana.ai>
>>>>>>>

<...>

>> mlx5 IB driver doesn't export any symbol that is used by the core driver,
>> that's why the core driver can be loaded without the IB driver (althought
>> you'll get circular dependency if you would export).
> 
> Yes, IB and ETH drivers are "users" of core driver. As RDMA maintainer,
> I'm reluctant to accept code that exports symbols from IB drivers to
> other subsystems. We have drivers/infiniband/core/ for that.
> 

We need the core driver to access the IB driver (and to the ETH driver as
well). As you wrote, we can't use exported symbols from our IB driver nor
rely on function pointers, but what about providing the core driver an ops
structure? meaning exporting a register function from the core driver that
should be called by the IB driver during auxiliary device probe.
Something like:

int hbl_cn_register_ib_aux_dev(struct auxiliary_device *adev,
			       struct hbl_ib_ops *ops)
{
...
}
EXPORT_SYMBOL(hbl_cn_register_ib_aux_dev);

That's how only the parent driver exports symbols to the son driver so the
IB driver is a "user" of the core driver and so we count on the internal
module reference counter. But we also get the ability to access the IB
driver from the core driver (to report a HW error for example).

>> If relying on exported symbols only, then our IB and ETH drivers will need
>> to export symbols too because the core driver accesses them post probing.
> 
> So you should fix your core driver. This is exactly what auxbus model
> proposes.
> 
>> Hence we won't be able to load the core driver without both of them (or
>> loading anything due to circular dependency).
>> Unless we'll use dynamic symbol lookup and I don't think that's your
>> intention.
> 
> No it is not.
> 

<...>