diff mbox

[dpdk-dev,02/16] nfp: add specific pf probe function

Message ID 1503591622-16232-3-git-send-email-alejandro.lucero@netronome.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Alejandro Lucero Aug. 24, 2017, 4:20 p.m. UTC 
  Configuring the NFP PMD for using the PF requires access through the
NSPU interface for device configuration. This patch adds a specific probe
function for the PF which uses the NSPU interface. Just basic NSPU access
is done by now reading the NSPU ABI version.

No ethernet port is created yet.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 drivers/net/nfp/nfp_net.c | 75 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 6 deletions(-)

Comments

Ferruh Yigit Aug. 28, 2017, 4:42 p.m. UTC | #1 
On 8/24/2017 5:20 PM, Alejandro Lucero wrote:
> Configuring the NFP PMD for using the PF requires access through the
> NSPU interface for device configuration. This patch adds a specific probe
> function for the PF which uses the NSPU interface. Just basic NSPU access
> is done by now reading the NSPU ABI version.
> 
> No ethernet port is created yet.
> 
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>

<...>

> +	/* Check NSP ABI version */
> +	if (nfp_nsp_get_abi_version(nspu_desc, &major, &minor) < 0) {
> +		RTE_LOG(INFO, PMD, "NFP NSP not present\n");
> +		goto no_abi;
> +	}
> +	PMD_INIT_LOG(INFO, "nspu ABI version: %d.%d\n", major, minor);
> +
> +	if (minor < 20) {
> +		RTE_LOG(INFO, PMD, "NFP NSP ABI version too old. Required 0.20 or higher\n");

I believe it worth documenting this detail in commit log and documentation.

<...>

>  
> -RTE_PMD_REGISTER_PCI(net_nfp, rte_nfp_net_pmd);
> -RTE_PMD_REGISTER_PCI_TABLE(net_nfp, pci_id_nfp_net_map);
> -RTE_PMD_REGISTER_KMOD_DEP(net_nfp, "* igb_uio | uio_pci_generic | vfio-pci");
> +RTE_PMD_REGISTER_PCI(net_nfp_pf, rte_nfp_net_pf_pmd);
> +RTE_PMD_REGISTER_PCI(net_nfp_vf, rte_nfp_net_vf_pmd);

Now pf and vf drivers are separated. For existing drivers this has been
documented in features file as another file (another column in table),
but we are looking for better representation for this.

What do you think, does two drivers has significant enough differences
to be documented as two different drivers?

> +RTE_PMD_REGISTER_PCI_TABLE(net_nfp_pf, pci_id_nfp_pf_net_map);
> +RTE_PMD_REGISTER_PCI_TABLE(net_nfp_vf, pci_id_nfp_vf_net_map);
> +RTE_PMD_REGISTER_KMOD_DEP(net_nfp_pf, "* igb_uio | uio_pci_generic | vfio");
> +RTE_PMD_REGISTER_KMOD_DEP(net_nfp_vf, "* igb_uio | uio_pci_generic | vfio");
>  
>  /*
>   * Local variables:
>
Alejandro Lucero Aug. 31, 2017, 9:23 a.m. UTC | #2 
On Mon, Aug 28, 2017 at 5:42 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 8/24/2017 5:20 PM, Alejandro Lucero wrote:
> > Configuring the NFP PMD for using the PF requires access through the
> > NSPU interface for device configuration. This patch adds a specific probe
> > function for the PF which uses the NSPU interface. Just basic NSPU access
> > is done by now reading the NSPU ABI version.
> >
> > No ethernet port is created yet.
> >
> > Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>
> <...>
>
> > +     /* Check NSP ABI version */
> > +     if (nfp_nsp_get_abi_version(nspu_desc, &major, &minor) < 0) {
> > +             RTE_LOG(INFO, PMD, "NFP NSP not present\n");
> > +             goto no_abi;
> > +     }
> > +     PMD_INIT_LOG(INFO, "nspu ABI version: %d.%d\n", major, minor);
> > +
> > +     if (minor < 20) {
> > +             RTE_LOG(INFO, PMD, "NFP NSP ABI version too old. Required
> 0.20 or higher\n");
>
> I believe it worth documenting this detail in commit log and documentation.
>

Ok.


>
> <...>
>
> >
> > -RTE_PMD_REGISTER_PCI(net_nfp, rte_nfp_net_pmd);
> > -RTE_PMD_REGISTER_PCI_TABLE(net_nfp, pci_id_nfp_net_map);
> > -RTE_PMD_REGISTER_KMOD_DEP(net_nfp, "* igb_uio | uio_pci_generic |
> vfio-pci");
> > +RTE_PMD_REGISTER_PCI(net_nfp_pf, rte_nfp_net_pf_pmd);
> > +RTE_PMD_REGISTER_PCI(net_nfp_vf, rte_nfp_net_vf_pmd);
>
> Now pf and vf drivers are separated. For existing drivers this has been
> documented in features file as another file (another column in table),
> but we are looking for better representation for this.
>
> What do you think, does two drivers has significant enough differences
> to be documented as two different drivers?
>
>
At this point PF and VF PMDs are exactly the same except for how
initialization is done. But, this will likely change in the near future.

I did not think about splitting out the features file, but I think it makes
sense. The existing one, just for VFs, has a problem with SRIOV. Obviously
VF support implies SRIOV, but I think the original idea of such a feature
was drivers being able to manage SRIOV, this is, creating and destroying
VFs. Also, firmware upload is just available with the PF, although such a
feature is not in the current features description list.

So, yes, I think I should have a file for the PF PMD and another one for
the VF.

I will add this in next patch set version.

Thanks



> > +RTE_PMD_REGISTER_PCI_TABLE(net_nfp_pf, pci_id_nfp_pf_net_map);
> > +RTE_PMD_REGISTER_PCI_TABLE(net_nfp_vf, pci_id_nfp_vf_net_map);
> > +RTE_PMD_REGISTER_KMOD_DEP(net_nfp_pf, "* igb_uio | uio_pci_generic |
> vfio");
> > +RTE_PMD_REGISTER_KMOD_DEP(net_nfp_vf, "* igb_uio | uio_pci_generic |
> vfio");
> >
> >  /*
> >   * Local variables:
> >
>
>
diff mbox

Patch

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index a3bf5e1..e2fe83a 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -59,6 +59,8 @@ 
 #include "nfp_net_logs.h"
 #include "nfp_net_ctrl.h"
 
+#include "nfp_nfpu.h"
+
 /* Prototypes */
 static void nfp_net_close(struct rte_eth_dev *dev);
 static int nfp_net_configure(struct rte_eth_dev *dev);
@@ -2632,12 +2634,63 @@  uint32_t nfp_net_txq_full(struct nfp_net_txq *txq)
 	return 0;
 }
 
-static const struct rte_pci_id pci_id_nfp_net_map[] = {
+static int nfp_pf_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
+			    struct rte_pci_device *dev)
+{
+	nfpu_desc_t *nfpu_desc;
+	nspu_desc_t *nspu_desc;
+	int major, minor;
+
+	if (!dev)
+		return -ENODEV;
+
+	nfpu_desc = rte_malloc("nfp nfpu", sizeof(nfpu_desc_t), 0);
+	if (!nfpu_desc)
+		return -ENOMEM;
+
+	if (nfpu_open(dev, nfpu_desc, 0) < 0) {
+		RTE_LOG(ERR, PMD,
+			"nfpu_open failed\n");
+		goto nfpu_error;
+	}
+
+	nspu_desc = nfpu_desc->nspu;
+
+
+	/* Check NSP ABI version */
+	if (nfp_nsp_get_abi_version(nspu_desc, &major, &minor) < 0) {
+		RTE_LOG(INFO, PMD, "NFP NSP not present\n");
+		goto no_abi;
+	}
+	PMD_INIT_LOG(INFO, "nspu ABI version: %d.%d\n", major, minor);
+
+	if (minor < 20) {
+		RTE_LOG(INFO, PMD, "NFP NSP ABI version too old. Required 0.20 or higher\n");
+		goto no_abi;
+	}
+
+	/* No port is created yet */
+
+no_abi:
+	nfpu_close(nfpu_desc);
+nfpu_error:
+	rte_free(nfpu_desc);
+
+	return -ENODEV;
+}
+
+static const struct rte_pci_id pci_id_nfp_pf_net_map[] = {
 	{
 		RTE_PCI_DEVICE(PCI_VENDOR_ID_NETRONOME,
 			       PCI_DEVICE_ID_NFP6000_PF_NIC)
 	},
 	{
+		.vendor_id = 0,
+	},
+};
+
+static const struct rte_pci_id pci_id_nfp_vf_net_map[] = {
+	{
 		RTE_PCI_DEVICE(PCI_VENDOR_ID_NETRONOME,
 			       PCI_DEVICE_ID_NFP6000_VF_NIC)
 	},
@@ -2658,16 +2711,26 @@  static int eth_nfp_pci_remove(struct rte_pci_device *pci_dev)
 	return rte_eth_dev_pci_generic_remove(pci_dev, NULL);
 }
 
-static struct rte_pci_driver rte_nfp_net_pmd = {
-	.id_table = pci_id_nfp_net_map,
+static struct rte_pci_driver rte_nfp_net_pf_pmd = {
+	.id_table = pci_id_nfp_pf_net_map,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+	.probe = nfp_pf_pci_probe,
+	.remove = eth_nfp_pci_remove,
+};
+
+static struct rte_pci_driver rte_nfp_net_vf_pmd = {
+	.id_table = pci_id_nfp_vf_net_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	.probe = eth_nfp_pci_probe,
 	.remove = eth_nfp_pci_remove,
 };
 
-RTE_PMD_REGISTER_PCI(net_nfp, rte_nfp_net_pmd);
-RTE_PMD_REGISTER_PCI_TABLE(net_nfp, pci_id_nfp_net_map);
-RTE_PMD_REGISTER_KMOD_DEP(net_nfp, "* igb_uio | uio_pci_generic | vfio-pci");
+RTE_PMD_REGISTER_PCI(net_nfp_pf, rte_nfp_net_pf_pmd);
+RTE_PMD_REGISTER_PCI(net_nfp_vf, rte_nfp_net_vf_pmd);
+RTE_PMD_REGISTER_PCI_TABLE(net_nfp_pf, pci_id_nfp_pf_net_map);
+RTE_PMD_REGISTER_PCI_TABLE(net_nfp_vf, pci_id_nfp_vf_net_map);
+RTE_PMD_REGISTER_KMOD_DEP(net_nfp_pf, "* igb_uio | uio_pci_generic | vfio");
+RTE_PMD_REGISTER_KMOD_DEP(net_nfp_vf, "* igb_uio | uio_pci_generic | vfio");
 
 /*
  * Local variables: