| Message ID | 1462194141-8902-2-git-send-email-erezsh@mellanox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
I think the main point of this patch is the introduction of capabilitymask2 right? Should be in the title. And it fixes up qib use of the field repurposed to be used as capmask2. -- 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 Mon, May 2, 2016 at 6:14 PM, Christoph Lameter <cl@linux.com> wrote: > I think the main point of this patch is the introduction of > capabilitymask2 right? Should be in the title. And it fixes up qib use of > the field repurposed to be used as capmask2. OK, agree. will change accordingly. 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 -- 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 Mon, May 02, 2016 at 04:02:18PM +0300, Erez Shitrit wrote: > > Change struct ib_class_port_info to conform to IB Spec 1.3 > That in order to get specific capability mask from ClassPortInfo mad. > > From the IB Spec, ClassPortInfo section: > "CapabilityMask2 Bits 0-26: Additional class-specific capabilities... > RespTimeValue the rest 5 bits" > > The new struct now has one field that contains both of cap_mask2 (which was > reserved and resp_time which now is 27 bits for cap_mask2 and 5 bits for > resp_time in the new field) > > More changes to adjust the new structure: > IB/qib: Change pma_get_classportinfo > > Signed-off-by: Erez Shitrit <erezsh@mellanox.com> > Reviewed-by: Leon Romanovsky <leonro@mellanox.com> > --- > drivers/infiniband/hw/qib/qib_mad.c | 4 +++- > include/rdma/ib_mad.h | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c > index 0bd1837..c5f6248 100644 > --- a/drivers/infiniband/hw/qib/qib_mad.c > +++ b/drivers/infiniband/hw/qib/qib_mad.c > @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp, > struct ib_class_port_info *p = > (struct ib_class_port_info *)pmp->data; > struct qib_devdata *dd = dd_from_ibdev(ibdev); > + char *p_cap_mask2; > > memset(pmp->data, 0, sizeof(pmp->data)); > > @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp, > * Set the most significant bit of CM2 to indicate support for > * congestion statistics > */ > - p->reserved[0] = dd->psxmitwait_supported << 7; > + p_cap_mask2 = (char *)&p->capability_mask2; > + p_cap_mask2[0] = dd->psxmitwait_supported << 7; Why did this change from p_cap_mask2[3] to p_cap_mask2[0]? I think this supports my concerns that endianess is not correct here. Ira -- 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 Fri, May 13, 2016 at 3:00 AM, Ira Weiny <ira.weiny@intel.com> wrote: > On Mon, May 02, 2016 at 04:02:18PM +0300, Erez Shitrit wrote: >> >> Change struct ib_class_port_info to conform to IB Spec 1.3 >> That in order to get specific capability mask from ClassPortInfo mad. >> >> From the IB Spec, ClassPortInfo section: >> "CapabilityMask2 Bits 0-26: Additional class-specific capabilities... >> RespTimeValue the rest 5 bits" >> >> The new struct now has one field that contains both of cap_mask2 (which was >> reserved and resp_time which now is 27 bits for cap_mask2 and 5 bits for >> resp_time in the new field) >> >> More changes to adjust the new structure: >> IB/qib: Change pma_get_classportinfo >> >> Signed-off-by: Erez Shitrit <erezsh@mellanox.com> >> Reviewed-by: Leon Romanovsky <leonro@mellanox.com> >> --- >> drivers/infiniband/hw/qib/qib_mad.c | 4 +++- >> include/rdma/ib_mad.h | 2 +- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c >> index 0bd1837..c5f6248 100644 >> --- a/drivers/infiniband/hw/qib/qib_mad.c >> +++ b/drivers/infiniband/hw/qib/qib_mad.c >> @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp, >> struct ib_class_port_info *p = >> (struct ib_class_port_info *)pmp->data; >> struct qib_devdata *dd = dd_from_ibdev(ibdev); >> + char *p_cap_mask2; >> >> memset(pmp->data, 0, sizeof(pmp->data)); >> >> @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp, >> * Set the most significant bit of CM2 to indicate support for >> * congestion statistics >> */ >> - p->reserved[0] = dd->psxmitwait_supported << 7; >> + p_cap_mask2 = (char *)&p->capability_mask2; >> + p_cap_mask2[0] = dd->psxmitwait_supported << 7; > > Why did this change from p_cap_mask2[3] to p_cap_mask2[0]? > > I think this supports my concerns that endianess is not correct here. I don't think there is a problem in the endianess here. p_cap_mask2[0] is for the capmask2 val, p_cap_mask2[3] is for the resp_time variable. (in __be32 resp_time is in the MSB and capmask2 in the LSB area) But, anyway I took your comment and will add getter and setter for each value. > > Ira > > -- > 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
diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c index 0bd1837..c5f6248 100644 --- a/drivers/infiniband/hw/qib/qib_mad.c +++ b/drivers/infiniband/hw/qib/qib_mad.c @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp, struct ib_class_port_info *p = (struct ib_class_port_info *)pmp->data; struct qib_devdata *dd = dd_from_ibdev(ibdev); + char *p_cap_mask2; memset(pmp->data, 0, sizeof(pmp->data)); @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp, * Set the most significant bit of CM2 to indicate support for * congestion statistics */ - p->reserved[0] = dd->psxmitwait_supported << 7; + p_cap_mask2 = (char *)&p->capability_mask2; + p_cap_mask2[0] = dd->psxmitwait_supported << 7; /* * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec. */ diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h index 37dd534c..0761ab9 100644 --- a/include/rdma/ib_mad.h +++ b/include/rdma/ib_mad.h @@ -243,7 +243,7 @@ struct ib_class_port_info { u8 base_version; u8 class_version; __be16 capability_mask; - u8 reserved[3]; + __be32 capability_mask2; u8 resp_time_value; u8 redirect_gid[16]; __be32 redirect_tcslfl;
Change struct ib_class_port_info to conform to IB Spec 1.3 That in order to get specific capability mask from ClassPortInfo mad. From the IB Spec, ClassPortInfo section: "CapabilityMask2 Bits 0-26: Additional class-specific capabilities... RespTimeValue the rest 5 bits" The new struct now has one field that contains both of cap_mask2 (which was reserved and resp_time which now is 27 bits for cap_mask2 and 5 bits for resp_time in the new field) More changes to adjust the new structure: IB/qib: Change pma_get_classportinfo Signed-off-by: Erez Shitrit <erezsh@mellanox.com> Reviewed-by: Leon Romanovsky <leonro@mellanox.com> --- drivers/infiniband/hw/qib/qib_mad.c | 4 +++- include/rdma/ib_mad.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)