| Message ID | a31ee424df9ad51371ff38e4f8239a390df13947.1503468979.git.aditr@vmware.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > and the IP_BASED_GIDS port cap flag. > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > Reviewed-by: Bryan Tan <bryantan@vmware.com> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > Signed-off-by: Adit Ranadive <aditr@vmware.com> > --- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > index 3a308ff..df0a6b5 100644 > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > @@ -149,6 +149,13 @@ > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > +/* > + * Get capability values based on device version. > + */ > + > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > + The current macro implementation will require you to go and update all the places once you will move to new version. In simple case, everything is supported and you will change your check of PVRDMA_IS_VERSION18 to something PVRDMA_IS_VERSIONXXX and it will work. In more complex case, when one of the features is supported in one version but isn't supported in another you will need to add "if(specific_feature)" magic into it. > enum pvrdma_pci_resource { > PVRDMA_PCI_RESOURCE_MSIX, /* BAR0: MSI-X, MMIO. */ > PVRDMA_PCI_RESOURCE_REG, /* BAR1: Registers, MMIO. */ > @@ -251,7 +258,7 @@ struct pvrdma_device_caps { > u8 atomic_ops; /* PVRDMA_ATOMIC_OP_* bits */ > u8 bmme_flags; /* FRWR Mem Mgmt Extensions */ > u8 gid_types; /* PVRDMA_GID_TYPE_FLAG_ */ > - u8 reserved[4]; > + u32 max_fast_reg_page_list_len; > }; > > struct pvrdma_ring_page_info { > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > index 2851704..48776f5 100644 > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > @@ -83,6 +83,8 @@ int pvrdma_query_device(struct ib_device *ibdev, > props->max_qp_wr = dev->dsr->caps.max_qp_wr; > props->device_cap_flags = dev->dsr->caps.device_cap_flags; > props->max_sge = dev->dsr->caps.max_sge; > + props->max_sge_rd = PVRDMA_GET_CAP(dev, dev->dsr->caps.max_sge, > + dev->dsr->caps.max_sge_rd); > props->max_cq = dev->dsr->caps.max_cq; > props->max_cqe = dev->dsr->caps.max_cqe; > props->max_mr = dev->dsr->caps.max_mr; > @@ -101,8 +103,14 @@ int pvrdma_query_device(struct ib_device *ibdev, > (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_REMOTE_INV) && > (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_FAST_REG_WR)) { > props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS; > + props->max_fast_reg_page_list_len = PVRDMA_GET_CAP(dev, > + PVRDMA_MAX_FAST_REG_PAGES, > + dev->dsr->caps.max_fast_reg_page_list_len); > } > > + props->device_cap_flags |= IB_DEVICE_PORT_ACTIVE_EVENT | > + IB_DEVICE_RC_RNR_NAK_GEN; > + > return 0; > } > > @@ -143,6 +151,7 @@ int pvrdma_query_port(struct ib_device *ibdev, u8 port, > props->gid_tbl_len = resp->attrs.gid_tbl_len; > props->port_cap_flags = > pvrdma_port_cap_flags_to_ib(resp->attrs.port_cap_flags); > + props->port_cap_flags |= IB_PORT_CM_SUP | IB_PORT_IP_BASED_GIDS; > props->max_msg_sz = resp->attrs.max_msg_sz; > props->bad_pkey_cntr = resp->attrs.bad_pkey_cntr; > props->qkey_viol_cntr = resp->attrs.qkey_viol_cntr; > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 23, 2017 at 12:09:20AM +0300, Leon Romanovsky wrote: > On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > > and the IP_BASED_GIDS port cap flag. > > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > Reviewed-by: Bryan Tan <bryantan@vmware.com> > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > Signed-off-by: Adit Ranadive <aditr@vmware.com> > > --- > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > index 3a308ff..df0a6b5 100644 > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > @@ -149,6 +149,13 @@ > > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > > > +/* > > + * Get capability values based on device version. > > + */ > > + > > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > > + > > The current macro implementation will require you to go and update all > the places once you will move to new version. In simple case, everything > is supported and you will change your check of PVRDMA_IS_VERSION18 to > something PVRDMA_IS_VERSIONXXX and it will work. Thanks for taking a look. That's the intention I wrote the macro with so it should pick up the value based on the current device version. If you are concerned that we will change device caps without changing the PVRDMA version, that's something we avoid for compatibility reasons. > In more complex case, when one of the features is supported in one version > but isn't supported in another you will need to add "if(specific_feature)" magic > into it. This will not happen based on how our version model currently works. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 23, 2017 at 11:08:01AM -0700, Adit Ranadive wrote: > On Wed, Aug 23, 2017 at 12:09:20AM +0300, Leon Romanovsky wrote: > > On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > > > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > > > and the IP_BASED_GIDS port cap flag. > > > > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > > Reviewed-by: Bryan Tan <bryantan@vmware.com> > > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > > Signed-off-by: Adit Ranadive <aditr@vmware.com> > > > --- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > index 3a308ff..df0a6b5 100644 > > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > @@ -149,6 +149,13 @@ > > > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > > > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > > > > > +/* > > > + * Get capability values based on device version. > > > + */ > > > + > > > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > > > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > > > + > > > > The current macro implementation will require you to go and update all > > the places once you will move to new version. In simple case, everything > > is supported and you will change your check of PVRDMA_IS_VERSION18 to > > something PVRDMA_IS_VERSIONXXX and it will work. > > Thanks for taking a look. That's the intention I wrote the macro with so it > should pick up the value based on the current device version. If you are > concerned that we will change device caps without changing the PVRDMA > version, that's something we avoid for compatibility reasons. I have hard time to understand how will you extend this macro once new feature is introduced. Let's for an example conduct the following experiment: 1. IB/core gets new exciting ROCE_V3 2. You are adding support to it and increasing the version to be VERSION19. 3. You now have code for ROCE_V2 and ROCE_V3. So the question is: how will your VRDMA_GET_CAP() look in such case? RECE_V2 is supported in VERSION18 and above, while ROCE_V3 is supported in VERSION19 only. And if we add more than one new feature, how will it look? Thanks > > > In more complex case, when one of the features is supported in one version > > but isn't supported in another you will need to add "if(specific_feature)" magic > > into it. > > This will not happen based on how our version model currently works.
On Wed, Aug 23, 2017 at 11:53:26AM -0700, Leon Romanovsky wrote: > On Wed, Aug 23, 2017 at 11:08:01AM -0700, Adit Ranadive wrote: > > On Wed, Aug 23, 2017 at 12:09:20AM +0300, Leon Romanovsky wrote: > > > On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > > > > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > > > > and the IP_BASED_GIDS port cap flag. > > > > > > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > > > Reviewed-by: Bryan Tan <bryantan@vmware.com> > > > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > > > Signed-off-by: Adit Ranadive <aditr@vmware.com> > > > > --- > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > > index 3a308ff..df0a6b5 100644 > > > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > > @@ -149,6 +149,13 @@ > > > > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > > > > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > > > > > > > +/* > > > > + * Get capability values based on device version. > > > > + */ > > > > + > > > > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > > > > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > > > > + > > > > > > The current macro implementation will require you to go and update all > > > the places once you will move to new version. In simple case, everything > > > is supported and you will change your check of PVRDMA_IS_VERSION18 to > > > something PVRDMA_IS_VERSIONXXX and it will work. > > > > Thanks for taking a look. That's the intention I wrote the macro with so it > > should pick up the value based on the current device version. If you are > > concerned that we will change device caps without changing the PVRDMA > > version, that's something we avoid for compatibility reasons. > > I have hard time to understand how will you extend this macro once new > feature is introduced. > > Let's for an example conduct the following experiment: > 1. IB/core gets new exciting ROCE_V3 > 2. You are adding support to it and increasing the version to be VERSION19. > 3. You now have code for ROCE_V2 and ROCE_V3. > > So the question is: how will your VRDMA_GET_CAP() look in such case? > RECE_V2 is supported in VERSION18 and above, while ROCE_V3 is supported > in VERSION19 only. The values for the capabilities come from our DSR (device shared region) itself. This would have nothing to do with the exact RoCE version but is based on the device version itself. So, if we add a VERSION19, we can change the macro to something like PVRDMA_IS_VERSION_NEXT_GEN, it will just pick up the new values defined in the DSR rather than based on RoCE versions. If we have removed support for that capability for VERSION19, that should simply be 0. To make this more clear, I can put the RoCE v1/v2 type checks, each in own macro. Drop them from the PVRDMA_IS_VERSION checks. Rename the PVRDMA_IS_VERSION18 to PVRDMA_IS_NEXTVERSION so we will always pick up the cap values correctly for future versions and we are not dependent on the RoCE version itself. > And if we add more than one new feature, how will it look? > > Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 23, 2017 at 12:29:21PM -0700, Adit Ranadive wrote: > On Wed, Aug 23, 2017 at 11:53:26AM -0700, Leon Romanovsky wrote: > > On Wed, Aug 23, 2017 at 11:08:01AM -0700, Adit Ranadive wrote: > > > On Wed, Aug 23, 2017 at 12:09:20AM +0300, Leon Romanovsky wrote: > > > > On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > > > > > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > > > > > and the IP_BASED_GIDS port cap flag. > > > > > > > > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > > > > Reviewed-by: Bryan Tan <bryantan@vmware.com> > > > > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > > > > Signed-off-by: Adit Ranadive <aditr@vmware.com> > > > > > --- > > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > > > > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > > > > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > > > index 3a308ff..df0a6b5 100644 > > > > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > > > > @@ -149,6 +149,13 @@ > > > > > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > > > > > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > > > > > > > > > +/* > > > > > + * Get capability values based on device version. > > > > > + */ > > > > > + > > > > > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > > > > > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > > > > > + > > > > > > > > The current macro implementation will require you to go and update all > > > > the places once you will move to new version. In simple case, everything > > > > is supported and you will change your check of PVRDMA_IS_VERSION18 to > > > > something PVRDMA_IS_VERSIONXXX and it will work. > > > > > > Thanks for taking a look. That's the intention I wrote the macro with so it > > > should pick up the value based on the current device version. If you are > > > concerned that we will change device caps without changing the PVRDMA > > > version, that's something we avoid for compatibility reasons. > > > > I have hard time to understand how will you extend this macro once new > > feature is introduced. > > > > Let's for an example conduct the following experiment: > > 1. IB/core gets new exciting ROCE_V3 > > 2. You are adding support to it and increasing the version to be VERSION19. > > 3. You now have code for ROCE_V2 and ROCE_V3. > > > > So the question is: how will your VRDMA_GET_CAP() look in such case? > > RECE_V2 is supported in VERSION18 and above, while ROCE_V3 is supported > > in VERSION19 only. > > The values for the capabilities come from our DSR (device shared region) itself. > This would have nothing to do with the exact RoCE version but is based > on the device version itself. So, if we add a VERSION19, we can change the > macro to something like PVRDMA_IS_VERSION_NEXT_GEN, it will just pick up the > new values defined in the DSR rather than based on RoCE versions. > If we have removed support for that capability for VERSION19, that should > simply be 0. Thanks, I got the idea and looking forward to see how it handles over time. > > To make this more clear, I can put the RoCE v1/v2 type checks, each in own > macro. Drop them from the PVRDMA_IS_VERSION checks. Rename the > PVRDMA_IS_VERSION18 to PVRDMA_IS_NEXTVERSION so we will always pick up the > cap values correctly for future versions and we are not dependent on the > RoCE version itself. I don't think that it is needed at this stage. > > > And if we add more than one new feature, how will it look? > > > > Thanks > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > and the IP_BASED_GIDS port cap flag. > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > Reviewed-by: Bryan Tan <bryantan@vmware.com> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > Signed-off-by: Adit Ranadive <aditr@vmware.com> > --- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > Thanks, Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > and the IP_BASED_GIDS port cap flag. And IB_PORT_CM_SUP > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > Reviewed-by: Bryan Tan <bryantan@vmware.com> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > Signed-off-by: Adit Ranadive <aditr@vmware.com> > --- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > index 3a308ff..df0a6b5 100644 > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > @@ -149,6 +149,13 @@ > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > +/* > + * Get capability values based on device version. > + */ > + > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > + > enum pvrdma_pci_resource { > PVRDMA_PCI_RESOURCE_MSIX, /* BAR0: MSI-X, MMIO. */ > PVRDMA_PCI_RESOURCE_REG, /* BAR1: Registers, MMIO. */ > @@ -251,7 +258,7 @@ struct pvrdma_device_caps { > u8 atomic_ops; /* PVRDMA_ATOMIC_OP_* bits */ > u8 bmme_flags; /* FRWR Mem Mgmt Extensions */ > u8 gid_types; /* PVRDMA_GID_TYPE_FLAG_ */ > - u8 reserved[4]; > + u32 max_fast_reg_page_list_len; > }; > > struct pvrdma_ring_page_info { > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > index 2851704..48776f5 100644 > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > @@ -83,6 +83,8 @@ int pvrdma_query_device(struct ib_device *ibdev, > props->max_qp_wr = dev->dsr->caps.max_qp_wr; > props->device_cap_flags = dev->dsr->caps.device_cap_flags; > props->max_sge = dev->dsr->caps.max_sge; > + props->max_sge_rd = PVRDMA_GET_CAP(dev, dev->dsr->caps.max_sge, > + dev->dsr->caps.max_sge_rd); I tend to agree with Leon on the other thread, i think that the following code is much more readable: props->max_sge_rd = dev->dsr->caps.max_sge; if (PVRDMA_IS_VERSION18(dev)) props->max_sge_rd = dev->dsr->caps.max_sge_rd; And then in the future add something like this: if (PVRDMA_IS_VERSION19(dev)) props->max_sge_rd = dev->dsr->caps.max_sge_19; But it is totally up to you. > props->max_cq = dev->dsr->caps.max_cq; > props->max_cqe = dev->dsr->caps.max_cqe; > props->max_mr = dev->dsr->caps.max_mr; > @@ -101,8 +103,14 @@ int pvrdma_query_device(struct ib_device *ibdev, > (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_REMOTE_INV) && > (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_FAST_REG_WR)) { > props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS; > + props->max_fast_reg_page_list_len = PVRDMA_GET_CAP(dev, > + PVRDMA_MAX_FAST_REG_PAGES, > + dev->dsr->caps.max_fast_reg_page_list_len); > } > > + props->device_cap_flags |= IB_DEVICE_PORT_ACTIVE_EVENT | > + IB_DEVICE_RC_RNR_NAK_GEN; > + Is it left over from version 17 or a new HW support? > return 0; > } > > @@ -143,6 +151,7 @@ int pvrdma_query_port(struct ib_device *ibdev, u8 port, > props->gid_tbl_len = resp->attrs.gid_tbl_len; > props->port_cap_flags = > pvrdma_port_cap_flags_to_ib(resp->attrs.port_cap_flags); > + props->port_cap_flags |= IB_PORT_CM_SUP | IB_PORT_IP_BASED_GIDS; Ditto. > props->max_msg_sz = resp->attrs.max_msg_sz; > props->bad_pkey_cntr = resp->attrs.bad_pkey_cntr; > props->qkey_viol_cntr = resp->attrs.qkey_viol_cntr; > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thur, Aug 24, 2017 at 07:33:50AM 0700, Yuval Shaia wrote: > On Tue, Aug 22, 2017 at 11:19:01PM -0700, Adit Ranadive wrote: > > Added support for two device caps - max_sge_rd, max_fast_reg_page_list_len > > and the IP_BASED_GIDS port cap flag. > > And IB_PORT_CM_SUP > > > > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com> > > Reviewed-by: Bryan Tan <bryantan@vmware.com> > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com> > > Signed-off-by: Adit Ranadive <aditr@vmware.com> > > --- > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h | 9 ++++++++- > > drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 9 +++++++++ > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > index 3a308ff..df0a6b5 100644 > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > > @@ -149,6 +149,13 @@ > > ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ > > (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) > > > > +/* > > + * Get capability values based on device version. > > + */ > > + > > +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ > > + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) > > + > > enum pvrdma_pci_resource { > > PVRDMA_PCI_RESOURCE_MSIX, /* BAR0: MSI-X, MMIO. */ > > PVRDMA_PCI_RESOURCE_REG, /* BAR1: Registers, MMIO. */ > > @@ -251,7 +258,7 @@ struct pvrdma_device_caps { > > u8 atomic_ops; /* PVRDMA_ATOMIC_OP_* bits */ > > u8 bmme_flags; /* FRWR Mem Mgmt Extensions */ > > u8 gid_types; /* PVRDMA_GID_TYPE_FLAG_ */ > > - u8 reserved[4]; > > + u32 max_fast_reg_page_list_len; > > }; > > > > struct pvrdma_ring_page_info { > > diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > > index 2851704..48776f5 100644 > > --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > > +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c > > @@ -83,6 +83,8 @@ int pvrdma_query_device(struct ib_device *ibdev, > > props->max_qp_wr = dev->dsr->caps.max_qp_wr; > > props->device_cap_flags = dev->dsr->caps.device_cap_flags; > > props->max_sge = dev->dsr->caps.max_sge; > > + props->max_sge_rd = PVRDMA_GET_CAP(dev, dev->dsr->caps.max_sge, > > + dev->dsr->caps.max_sge_rd); > > I tend to agree with Leon on the other thread, i think that the following > code is much more readable: > > props->max_sge_rd = dev->dsr->caps.max_sge; > if (PVRDMA_IS_VERSION18(dev)) > props->max_sge_rd = dev->dsr->caps.max_sge_rd; > > And then in the future add something like this: > if (PVRDMA_IS_VERSION19(dev)) > props->max_sge_rd = dev->dsr->caps.max_sge_19; > > But it is totally up to you. Thanks for taking a look and your suggestion. > > > props->max_cq = dev->dsr->caps.max_cq; > > props->max_cqe = dev->dsr->caps.max_cqe; > > props->max_mr = dev->dsr->caps.max_mr; > > @@ -101,8 +103,14 @@ int pvrdma_query_device(struct ib_device *ibdev, > > (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_REMOTE_INV) && > > (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_FAST_REG_WR)) { > > props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS; > > + props->max_fast_reg_page_list_len = PVRDMA_GET_CAP(dev, > > + PVRDMA_MAX_FAST_REG_PAGES, > > + dev->dsr->caps.max_fast_reg_page_list_len); > > } > > > > + props->device_cap_flags |= IB_DEVICE_PORT_ACTIVE_EVENT | > > + IB_DEVICE_RC_RNR_NAK_GEN; > > + > > Is it left over from version 17 or a new HW support? No, we have always implicitly supported those. This just makes it more explicit to users. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h index 3a308ff..df0a6b5 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h @@ -149,6 +149,13 @@ ((_dev->dsr->caps.mode == PVRDMA_DEVICE_MODE_ROCE) && \ (PVRDMA_IS_VERSION17(_dev) || PVRDMA_IS_VERSION18(_dev))) +/* + * Get capability values based on device version. + */ + +#define PVRDMA_GET_CAP(_dev, _old_val, _val) \ + ((PVRDMA_IS_VERSION18(_dev)) ? _val : _old_val) + enum pvrdma_pci_resource { PVRDMA_PCI_RESOURCE_MSIX, /* BAR0: MSI-X, MMIO. */ PVRDMA_PCI_RESOURCE_REG, /* BAR1: Registers, MMIO. */ @@ -251,7 +258,7 @@ struct pvrdma_device_caps { u8 atomic_ops; /* PVRDMA_ATOMIC_OP_* bits */ u8 bmme_flags; /* FRWR Mem Mgmt Extensions */ u8 gid_types; /* PVRDMA_GID_TYPE_FLAG_ */ - u8 reserved[4]; + u32 max_fast_reg_page_list_len; }; struct pvrdma_ring_page_info { diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c index 2851704..48776f5 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c @@ -83,6 +83,8 @@ int pvrdma_query_device(struct ib_device *ibdev, props->max_qp_wr = dev->dsr->caps.max_qp_wr; props->device_cap_flags = dev->dsr->caps.device_cap_flags; props->max_sge = dev->dsr->caps.max_sge; + props->max_sge_rd = PVRDMA_GET_CAP(dev, dev->dsr->caps.max_sge, + dev->dsr->caps.max_sge_rd); props->max_cq = dev->dsr->caps.max_cq; props->max_cqe = dev->dsr->caps.max_cqe; props->max_mr = dev->dsr->caps.max_mr; @@ -101,8 +103,14 @@ int pvrdma_query_device(struct ib_device *ibdev, (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_REMOTE_INV) && (dev->dsr->caps.bmme_flags & PVRDMA_BMME_FLAG_FAST_REG_WR)) { props->device_cap_flags |= IB_DEVICE_MEM_MGT_EXTENSIONS; + props->max_fast_reg_page_list_len = PVRDMA_GET_CAP(dev, + PVRDMA_MAX_FAST_REG_PAGES, + dev->dsr->caps.max_fast_reg_page_list_len); } + props->device_cap_flags |= IB_DEVICE_PORT_ACTIVE_EVENT | + IB_DEVICE_RC_RNR_NAK_GEN; + return 0; } @@ -143,6 +151,7 @@ int pvrdma_query_port(struct ib_device *ibdev, u8 port, props->gid_tbl_len = resp->attrs.gid_tbl_len; props->port_cap_flags = pvrdma_port_cap_flags_to_ib(resp->attrs.port_cap_flags); + props->port_cap_flags |= IB_PORT_CM_SUP | IB_PORT_IP_BASED_GIDS; props->max_msg_sz = resp->attrs.max_msg_sz; props->bad_pkey_cntr = resp->attrs.bad_pkey_cntr; props->qkey_viol_cntr = resp->attrs.qkey_viol_cntr;