| Message ID | 1667502990-2559-2-git-send-email-longli@linuxonhyperv.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a69839d4327d053b18d8e1b0e7ddeee78db78f4f |
| Headers | show |
| Series | Introduce Microsoft Azure Network Adapter (MANA) RDMA driver | expand |
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
diff --git a/drivers/net/ethernet/microsoft/Kconfig b/drivers/net/ethernet/microsoft/Kconfig index fe4e7a7d9c0b..090e6b983243 100644 --- a/drivers/net/ethernet/microsoft/Kconfig +++ b/drivers/net/ethernet/microsoft/Kconfig @@ -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. diff --git a/drivers/net/ethernet/microsoft/mana/gdma.h b/drivers/net/ethernet/microsoft/mana/gdma.h index 4a6efe6ada08..f321a2616d03 100644 --- a/drivers/net/ethernet/microsoft/mana/gdma.h +++ b/drivers/net/ethernet/microsoft/mana/gdma.h @@ -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 diff --git a/drivers/net/ethernet/microsoft/mana/mana_auxiliary.h b/drivers/net/ethernet/microsoft/mana/mana_auxiliary.h new file mode 100644 index 000000000000..373d59756846 --- /dev/null +++ b/drivers/net/ethernet/microsoft/mana/mana_auxiliary.h @@ -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; +}; diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c index 9259a74eca40..8751e475d1ba 100644 --- a/drivers/net/ethernet/microsoft/mana/mana_en.c +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c @@ -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);