On 3/2/2025 12:26 AM, Leon Romanovsky wrote:
On Wed, Feb 26, 2025 at 11:01:52PM +0000, Ertman, David M wrote:
-----Original Message-----
From: Leon Romanovsky <leon@xxxxxxxxxx>
Sent: Wednesday, February 26, 2025 10:50 AM
To: Ertman, David M <david.m.ertman@xxxxxxxxx>
Cc: Nikolova, Tatyana E <tatyana.e.nikolova@xxxxxxxxx>; jgg@xxxxxxxxxx;
intel-wired-lan@xxxxxxxxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx;
netdev@xxxxxxxxxxxxxxx
Subject: Re: [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to support multiple
consumers
On Wed, Feb 26, 2025 at 05:36:44PM +0000, Ertman, David M wrote:
-----Original Message-----
From: Leon Romanovsky <leon@xxxxxxxxxx>
Sent: Monday, February 24, 2025 11:56 PM
To: Nikolova, Tatyana E <tatyana.e.nikolova@xxxxxxxxx>
Cc: jgg@xxxxxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx; linux-
rdma@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; Ertman, David M
<david.m.ertman@xxxxxxxxx>
Subject: Re: [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to support
multiple
consumers
On Mon, Feb 24, 2025 at 11:04:28PM -0600, Tatyana Nikolova wrote:
From: Dave Ertman <david.m.ertman@xxxxxxxxx>
To support RDMA for E2000 product, the idpf driver will use the IDC
interface with the irdma auxiliary driver, thus becoming a second
consumer of it. This requires the IDC be updated to support multiple
consumers. The use of exported symbols no longer makes sense
because it
will require all core drivers (ice/idpf) that can interface with irdma
auxiliary driver to be loaded even if hardware is not present for those
drivers.
In auxiliary bus world, the code drivers (ice/idpf) need to created
auxiliary devices only if specific device present. That auxiliary device
will trigger the load of specific module (irdma in our case).
EXPORT_SYMBOL won't trigger load of irdma driver, but the opposite is
true, load of irdma will trigger load of ice/idpf drivers (depends on
their exported symbol).
To address this, implement an ops struct that will be universal set of
naked function pointers that will be populated by each core driver for
the irdma auxiliary driver to call.
No, we worked very hard to make proper HW discovery and driver
autoload,
let's not return back. For now, it is no-go.
Hi Leon,
I am a little confused about what the problem here is. The main issue I pull
from your response is: Removing exported symbols will stop ice/idpf from
autoloading when irdma loads. Is this correct or did I miss your point?
It is one of the main points.
But, if there is an ice or idpf supported device present in the system, the
appropriate driver will have already been loaded anyway (and gone
through its
probe flow to create auxiliary devices). If it is not loaded, then the system
owner
has either unloaded it manually or blacklisted it. This would not cause an
issue
anyway, since irdma and ice/idpf can load in any order.
There are two assumptions above, which both not true.
1. Users never issue "modprobe irdma" command alone and always will call
to whole chain "modprobe ice ..." before.
2. You open-code module subsystem properly with reference counters,
ownership and locks to protect from function pointers to be set/clear
dynamically.
Ah, I see your reasoning now. Our goal was to make the two modules independent,
with no prescribed load order mandated, and utilize the auxiliary bus and device subsystem
to handle load order and unload of one or the other module. The auxiliary device only has
the lifespan of the core PCI driver, so if the core driver unloads, then the auxiliary device gets
destroyed, and the associated link based off it will be gone. We wanted to be able to unload
and reload either of the modules (core or irdma) and have the interaction be able to restart with a
new probe. All our inter-driver function calls are protected by device lock on the auxiliary
device for the duration of the call.
Yes, you are trying to return to pre-aux era.
The main motivation to go with callbacks to the parent driver instead of
using exported symbols is to allow loading only the parent driver
required for a particular deployment. irdma is a common rdma auxilary
driver that supports multiple parent pci drivers(ice, idpf, i40e, iavf).
If we use exported symbols, all these modules will get loaded even on a
system with only idpf device.
The documentation for auxiliary bus
https://docs.kernel.org/driver-api/auxiliary_bus.html
shows an example on how shared data/callbacks can be used to establish
connection with the parent.
Auxiliary devices are created and registered by a subsystem-level core
device that needs to break up its functionality into smaller fragments.
One way to extend the scope of an auxiliary_device is to encapsulate it
within a domain-specific structure defined by the parent device. This
structure contains the auxiliary_device and any associated shared
data/callbacks needed to establish the connection with the parent.
An example is:
struct foo {
struct auxiliary_device auxdev;
void (*connect)(struct auxiliary_device *auxdev);
void (*disconnect)(struct auxiliary_device *auxdev);
void *data;
};
This example clearly shows that it is OK to use callbacks from the aux
driver. The aux driver is dependent on the parent driver and the parent
driver will guarantee that it will not get unloaded until all the
auxiliary devices are destroyed.
Hopefully you will understand our motivation of going with this design
and not force us to go with a solution that is not optimal.
Thanks
Sridhar
