[v10,01/12] net: mana: Add support for auxiliary device
Commit Message
From: Long Li <longli@microsoft.com>
In preparation for supporting MANA RDMA driver, add support for auxiliary
device in the Ethernet driver. The RDMA device is modeled as an auxiliary
device to the Ethernet device.
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Long Li <longli@microsoft.com>
Acked-by: Haiyang Zhang <haiyangz@microsoft.com>
---
Change log:
v3: define mana_adev_idx_alloc and mana_adev_idx_free as static
v7: fix a bug that may assign a negative value to adev->id
drivers/net/ethernet/microsoft/Kconfig | 1 +
drivers/net/ethernet/microsoft/mana/gdma.h | 2 +
.../ethernet/microsoft/mana/mana_auxiliary.h | 10 +++
drivers/net/ethernet/microsoft/mana/mana_en.c | 83 ++++++++++++++++++-
4 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/microsoft/mana/mana_auxiliary.h
Comments
On Thu, Nov 03, 2022 at 12:16:19PM -0700, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> In preparation for supporting MANA RDMA driver, add support for auxiliary
> device in the Ethernet driver. The RDMA device is modeled as an auxiliary
> device to the Ethernet device.
>
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Long Li <longli@microsoft.com>
> Acked-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> Change log:
> v3: define mana_adev_idx_alloc and mana_adev_idx_free as static
> v7: fix a bug that may assign a negative value to adev->id
<...>
> int mana_probe(struct gdma_dev *gd, bool resuming)
> {
> struct gdma_context *gc = gd->gdma_context;
> @@ -2173,6 +2249,8 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
> break;
> }
> }
> +
> + err = add_adev(gd);
> out:
> if (err)
> mana_remove(gd, false);
> @@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> int err;
> int i;
>
> + /* adev currently doesn't support suspending, always remove it */
> + if (gd->adev)
This condition is always true, isn't it?
> + remove_adev(gd);
> +
Thanks
> > int mana_probe(struct gdma_dev *gd, bool resuming)
> > break;
> > }
> > }
> > +
> > + err = add_adev(gd);
> > out:
> > if (err)
> > mana_remove(gd, false);
> > @@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool
> suspending)
> > int err;
> > int i;
> >
> > + /* adev currently doesn't support suspending, always remove it */
> > + if (gd->adev)
>
> This condition is always true, isn't it?
I think the check is necessary. mana_probe() will call mana_remove() if it fails to
add this adev to gd. If this is the case, we can't call remove_adev().
Thanks,
Long
On Tue, Nov 08, 2022 at 09:33:21PM +0000, Long Li wrote:
>
> > > int mana_probe(struct gdma_dev *gd, bool resuming)
> > > break;
> > > }
> > > }
> > > +
> > > + err = add_adev(gd);
> > > out:
> > > if (err)
> > > mana_remove(gd, false);
> > > @@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool
> > suspending)
> > > int err;
> > > int i;
> > >
> > > + /* adev currently doesn't support suspending, always remove it */
> > > + if (gd->adev)
> >
> > This condition is always true, isn't it?
>
> I think the check is necessary. mana_probe() will call mana_remove() if it fails to
> add this adev to gd. If this is the case, we can't call remove_adev().
I'm sad to hear that. It is so anti-pattern to hide error unwind in one global function.
But ok, it is already there, so let's take this series as is.
Thanks
>
> Thanks,
> Long
@@ -19,6 +19,7 @@ config MICROSOFT_MANA
tristate "Microsoft Azure Network Adapter (MANA) support"
depends on PCI_MSI && X86_64
depends on PCI_HYPERV
+ select AUXILIARY_BUS
help
This driver supports Microsoft Azure Network Adapter (MANA).
So far, the driver is only supported on X86_64.
@@ -204,6 +204,8 @@ struct gdma_dev {
/* GDMA driver specific pointer */
void *driver_data;
+
+ struct auxiliary_device *adev;
};
#define MINIMUM_SUPPORTED_PAGE_SIZE PAGE_SIZE
new file mode 100644
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (c) 2022, Microsoft Corporation. */
+
+#include "mana.h"
+#include <linux/auxiliary_bus.h>
+
+struct mana_adev {
+ struct auxiliary_device adev;
+ struct gdma_dev *mdev;
+};
@@ -13,6 +13,19 @@
#include <net/ip6_checksum.h>
#include "mana.h"
+#include "mana_auxiliary.h"
+
+static DEFINE_IDA(mana_adev_ida);
+
+static int mana_adev_idx_alloc(void)
+{
+ return ida_alloc(&mana_adev_ida, GFP_KERNEL);
+}
+
+static void mana_adev_idx_free(int idx)
+{
+ ida_free(&mana_adev_ida, idx);
+}
/* Microsoft Azure Network Adapter (MANA) functions */
@@ -2106,6 +2119,69 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
return err;
}
+static void adev_release(struct device *dev)
+{
+ struct mana_adev *madev = container_of(dev, struct mana_adev, adev.dev);
+
+ kfree(madev);
+}
+
+static void remove_adev(struct gdma_dev *gd)
+{
+ struct auxiliary_device *adev = gd->adev;
+ int id = adev->id;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+
+ mana_adev_idx_free(id);
+ gd->adev = NULL;
+}
+
+static int add_adev(struct gdma_dev *gd)
+{
+ struct auxiliary_device *adev;
+ struct mana_adev *madev;
+ int ret;
+
+ madev = kzalloc(sizeof(*madev), GFP_KERNEL);
+ if (!madev)
+ return -ENOMEM;
+
+ adev = &madev->adev;
+ ret = mana_adev_idx_alloc();
+ if (ret < 0)
+ goto idx_fail;
+ adev->id = ret;
+
+ adev->name = "rdma";
+ adev->dev.parent = gd->gdma_context->dev;
+ adev->dev.release = adev_release;
+ madev->mdev = gd;
+
+ ret = auxiliary_device_init(adev);
+ if (ret)
+ goto init_fail;
+
+ ret = auxiliary_device_add(adev);
+ if (ret)
+ goto add_fail;
+
+ gd->adev = adev;
+ return 0;
+
+add_fail:
+ auxiliary_device_uninit(adev);
+
+init_fail:
+ mana_adev_idx_free(adev->id);
+
+idx_fail:
+ kfree(madev);
+
+ return ret;
+}
+
int mana_probe(struct gdma_dev *gd, bool resuming)
{
struct gdma_context *gc = gd->gdma_context;
@@ -2173,6 +2249,8 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
break;
}
}
+
+ err = add_adev(gd);
out:
if (err)
mana_remove(gd, false);
@@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
int err;
int i;
+ /* adev currently doesn't support suspending, always remove it */
+ if (gd->adev)
+ remove_adev(gd);
+
for (i = 0; i < ac->num_ports; i++) {
ndev = ac->ports[i];
if (!ndev) {
@@ -2221,7 +2303,6 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
}
mana_destroy_eq(ac);
-
out:
mana_gd_deregister_device(gd);