| Message ID | 20201028231945.6533-1-aditr@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-rc] RDMA/vmw_pvrdma: Fix the active_speed and phys_state value | expand |
On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: > The PVRDMA device still reports the active_speed in u8. > Lets use the ib_eth_get_speed to report the speed and > width. Unfortunately, phys_state gets stored as msb of > the new u16 active_speed. This explanation is not clear, I have no idea what this is fixing Jason
On 10/29/20 4:57 AM, Jason Gunthorpe wrote: > On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: >> The PVRDMA device still reports the active_speed in u8. >> Lets use the ib_eth_get_speed to report the speed and >> width. Unfortunately, phys_state gets stored as msb of >> the new u16 active_speed. > > This explanation is not clear, I have no idea what this is fixing It seemed more clear to me in my head, I guess :). After commit 376ceb31ff87 changed the active_speed attribute to u16, both the active_speed and phys_state attributes in the pvrdma_port_attr struct are getting stored in this u16. As a result, these show up as invalid values in ibv_devinfo. Our device still gives us back a u8 active_speed so both these are getting stored in the u16. This fix I proposed simply gets the active_speed from the netdev while the phys_state still needs to come from the pvrdma device, i.e. the msb the of the u16. I also removed some unused functions as a result. Alternatively, I could change the u8 active_width and u16 active_speed to reserved now that we're getting the active_speed and active_width from the ib_get_eth_speed function. > > Jason >
On 10/29/20 9:16 AM, Adit Ranadive wrote: > On 10/29/20 4:57 AM, Jason Gunthorpe wrote: >> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: >>> The PVRDMA device still reports the active_speed in u8. >>> Lets use the ib_eth_get_speed to report the speed and >>> width. Unfortunately, phys_state gets stored as msb of >>> the new u16 active_speed. >> >> This explanation is not clear, I have no idea what this is fixing > > It seemed more clear to me in my head, I guess :). > > After commit 376ceb31ff87 changed the active_speed attribute to > u16, both the active_speed and phys_state attributes in the > pvrdma_port_attr struct are getting stored in this u16. As a > result, these show up as invalid values in ibv_devinfo. > > Our device still gives us back a u8 active_speed so both these > are getting stored in the u16. This fix I proposed simply gets > the active_speed from the netdev while the phys_state still > needs to come from the pvrdma device, i.e. the msb the of the > u16. I also removed some unused functions as a result. > > Alternatively, I could change the u8 active_width and u16 > active_speed to reserved now that we're getting the active_speed > and active_width from the ib_get_eth_speed function. > Jason, did you have any comments on this or did you want me to just send v1 with an updated description? >> >> Jason >>
On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: > On 10/29/20 9:16 AM, Adit Ranadive wrote: > > On 10/29/20 4:57 AM, Jason Gunthorpe wrote: > >> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: > >>> The PVRDMA device still reports the active_speed in u8. > >>> Lets use the ib_eth_get_speed to report the speed and > >>> width. Unfortunately, phys_state gets stored as msb of > >>> the new u16 active_speed. > >> > >> This explanation is not clear, I have no idea what this is fixing > > > > It seemed more clear to me in my head, I guess :). > > > > After commit 376ceb31ff87 changed the active_speed attribute to > > u16, both the active_speed and phys_state attributes in the > > pvrdma_port_attr struct are getting stored in this u16. As a > > result, these show up as invalid values in ibv_devinfo. > > > > Our device still gives us back a u8 active_speed so both these > > are getting stored in the u16. This fix I proposed simply gets > > the active_speed from the netdev while the phys_state still > > needs to come from the pvrdma device, i.e. the msb the of the > > u16. I also removed some unused functions as a result. > > > > Alternatively, I could change the u8 active_width and u16 > > active_speed to reserved now that we're getting the active_speed > > and active_width from the ib_get_eth_speed function. > > Jason, did you have any comments on this or did you want me > to just send v1 with an updated description? I still haven't figured out what this is fixing. Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix to revert the type? Jason
On 11/2/20 10:02 AM, Jason Gunthorpe wrote: > On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: >> On 10/29/20 9:16 AM, Adit Ranadive wrote: >>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: >>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: >>>>> The PVRDMA device still reports the active_speed in u8. >>>>> Lets use the ib_eth_get_speed to report the speed and >>>>> width. Unfortunately, phys_state gets stored as msb of >>>>> the new u16 active_speed. >>>> >>>> This explanation is not clear, I have no idea what this is fixing >>> >>> It seemed more clear to me in my head, I guess :). >>> >>> After commit 376ceb31ff87 changed the active_speed attribute to >>> u16, both the active_speed and phys_state attributes in the >>> pvrdma_port_attr struct are getting stored in this u16. As a >>> result, these show up as invalid values in ibv_devinfo. >>> >>> Our device still gives us back a u8 active_speed so both these >>> are getting stored in the u16. This fix I proposed simply gets >>> the active_speed from the netdev while the phys_state still >>> needs to come from the pvrdma device, i.e. the msb the of the >>> u16. I also removed some unused functions as a result. >>> >>> Alternatively, I could change the u8 active_width and u16 >>> active_speed to reserved now that we're getting the active_speed >>> and active_width from the ib_get_eth_speed function. >> >> Jason, did you have any comments on this or did you want me >> to just send v1 with an updated description? > > I still haven't figured out what this is fixing. > > Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix > to revert the type? I can revert it but I thought that it had to a u16 based on the IBTA, no? Or does that not apply to device-level stuff? Also, instead of reverting it seemed better to use the ib_get_eth_speed function to get the active_speed based on the netdev.
On Mon, Nov 02, 2020 at 10:21:21AM -0800, Adit Ranadive wrote: > On 11/2/20 10:02 AM, Jason Gunthorpe wrote: > > On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: > >> On 10/29/20 9:16 AM, Adit Ranadive wrote: > >>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: > >>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: > >>>>> The PVRDMA device still reports the active_speed in u8. > >>>>> Lets use the ib_eth_get_speed to report the speed and > >>>>> width. Unfortunately, phys_state gets stored as msb of > >>>>> the new u16 active_speed. > >>>> > >>>> This explanation is not clear, I have no idea what this is fixing > >>> > >>> It seemed more clear to me in my head, I guess :). > >>> > >>> After commit 376ceb31ff87 changed the active_speed attribute to > >>> u16, both the active_speed and phys_state attributes in the > >>> pvrdma_port_attr struct are getting stored in this u16. As a > >>> result, these show up as invalid values in ibv_devinfo. > >>> > >>> Our device still gives us back a u8 active_speed so both these > >>> are getting stored in the u16. This fix I proposed simply gets > >>> the active_speed from the netdev while the phys_state still > >>> needs to come from the pvrdma device, i.e. the msb the of the > >>> u16. I also removed some unused functions as a result. > >>> > >>> Alternatively, I could change the u8 active_width and u16 > >>> active_speed to reserved now that we're getting the active_speed > >>> and active_width from the ib_get_eth_speed function. > >> > >> Jason, did you have any comments on this or did you want me > >> to just send v1 with an updated description? > > > > I still haven't figured out what this is fixing. > > > > Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix > > to revert the type? > > I can revert it but I thought that it had to a u16 based on the IBTA, no? > Or does that not apply to device-level stuff? You didn't answer the question, it it ABI to some kind of FW interface or something? *HOW* did two fields get overlapped onto a single u16?? The compiler won't do this.. Jason
On 11/2/20 10:27 AM, Jason Gunthorpe wrote: > On Mon, Nov 02, 2020 at 10:21:21AM -0800, Adit Ranadive wrote: >> On 11/2/20 10:02 AM, Jason Gunthorpe wrote: >>> On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: >>>> On 10/29/20 9:16 AM, Adit Ranadive wrote: >>>>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: >>>>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: >>>>>>> The PVRDMA device still reports the active_speed in u8. >>>>>>> Lets use the ib_eth_get_speed to report the speed and >>>>>>> width. Unfortunately, phys_state gets stored as msb of >>>>>>> the new u16 active_speed. >>>>>> >>>>>> This explanation is not clear, I have no idea what this is fixing >>>>> >>>>> It seemed more clear to me in my head, I guess :). >>>>> >>>>> After commit 376ceb31ff87 changed the active_speed attribute to >>>>> u16, both the active_speed and phys_state attributes in the >>>>> pvrdma_port_attr struct are getting stored in this u16. As a >>>>> result, these show up as invalid values in ibv_devinfo. >>>>> >>>>> Our device still gives us back a u8 active_speed so both these >>>>> are getting stored in the u16. This fix I proposed simply gets >>>>> the active_speed from the netdev while the phys_state still >>>>> needs to come from the pvrdma device, i.e. the msb the of the >>>>> u16. I also removed some unused functions as a result. >>>>> >>>>> Alternatively, I could change the u8 active_width and u16 >>>>> active_speed to reserved now that we're getting the active_speed >>>>> and active_width from the ib_get_eth_speed function. >>>> >>>> Jason, did you have any comments on this or did you want me >>>> to just send v1 with an updated description? >>> >>> I still haven't figured out what this is fixing. >>> >>> Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix >>> to revert the type? >> >> I can revert it but I thought that it had to a u16 based on the IBTA, no? >> Or does that not apply to device-level stuff? > > You didn't answer the question, it it ABI to some kind of FW interface > or something? > > *HOW* did two fields get overlapped onto a single u16?? The compiler > won't do this.. > It is an ABI to the device for port attributes. The device gives us back this structure for query port verb. The response from the device is memcopied into this pvrdma_port_attr structure. So both the bytes representing active_speed and phys_state from the device are copied into the single u16 in this structure.
On Mon, Nov 02, 2020 at 10:38:19AM -0800, Adit Ranadive wrote: > On 11/2/20 10:27 AM, Jason Gunthorpe wrote: > > On Mon, Nov 02, 2020 at 10:21:21AM -0800, Adit Ranadive wrote: > >> On 11/2/20 10:02 AM, Jason Gunthorpe wrote: > >>> On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: > >>>> On 10/29/20 9:16 AM, Adit Ranadive wrote: > >>>>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: > >>>>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: > >>>>>>> The PVRDMA device still reports the active_speed in u8. > >>>>>>> Lets use the ib_eth_get_speed to report the speed and > >>>>>>> width. Unfortunately, phys_state gets stored as msb of > >>>>>>> the new u16 active_speed. > >>>>>> > >>>>>> This explanation is not clear, I have no idea what this is fixing > >>>>> > >>>>> It seemed more clear to me in my head, I guess :). > >>>>> > >>>>> After commit 376ceb31ff87 changed the active_speed attribute to > >>>>> u16, both the active_speed and phys_state attributes in the > >>>>> pvrdma_port_attr struct are getting stored in this u16. As a > >>>>> result, these show up as invalid values in ibv_devinfo. > >>>>> > >>>>> Our device still gives us back a u8 active_speed so both these > >>>>> are getting stored in the u16. This fix I proposed simply gets > >>>>> the active_speed from the netdev while the phys_state still > >>>>> needs to come from the pvrdma device, i.e. the msb the of the > >>>>> u16. I also removed some unused functions as a result. > >>>>> > >>>>> Alternatively, I could change the u8 active_width and u16 > >>>>> active_speed to reserved now that we're getting the active_speed > >>>>> and active_width from the ib_get_eth_speed function. > >>>> > >>>> Jason, did you have any comments on this or did you want me > >>>> to just send v1 with an updated description? > >>> > >>> I still haven't figured out what this is fixing. > >>> > >>> Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix > >>> to revert the type? > >> > >> I can revert it but I thought that it had to a u16 based on the IBTA, no? > >> Or does that not apply to device-level stuff? > > > > You didn't answer the question, it it ABI to some kind of FW interface > > or something? > > > > *HOW* did two fields get overlapped onto a single u16?? The compiler > > won't do this.. > > > > It is an ABI to the device for port attributes. The device gives us back > this structure for query port verb. The response from the device is > memcopied into this pvrdma_port_attr structure. So both the bytes > representing active_speed and phys_state from the device are copied > into the single u16 in this structure. So it is ABI and it shouldn't have been changed, point at the stuff that made it ABI and revert the structure layout change.. Jason
On Mon, Nov 02, 2020 at 02:46:40PM -0400, Jason Gunthorpe wrote: > On Mon, Nov 02, 2020 at 10:38:19AM -0800, Adit Ranadive wrote: > > On 11/2/20 10:27 AM, Jason Gunthorpe wrote: > > > On Mon, Nov 02, 2020 at 10:21:21AM -0800, Adit Ranadive wrote: > > >> On 11/2/20 10:02 AM, Jason Gunthorpe wrote: > > >>> On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: > > >>>> On 10/29/20 9:16 AM, Adit Ranadive wrote: > > >>>>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: > > >>>>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: > > >>>>>>> The PVRDMA device still reports the active_speed in u8. > > >>>>>>> Lets use the ib_eth_get_speed to report the speed and > > >>>>>>> width. Unfortunately, phys_state gets stored as msb of > > >>>>>>> the new u16 active_speed. > > >>>>>> > > >>>>>> This explanation is not clear, I have no idea what this is fixing > > >>>>> > > >>>>> It seemed more clear to me in my head, I guess :). > > >>>>> > > >>>>> After commit 376ceb31ff87 changed the active_speed attribute to > > >>>>> u16, both the active_speed and phys_state attributes in the > > >>>>> pvrdma_port_attr struct are getting stored in this u16. As a > > >>>>> result, these show up as invalid values in ibv_devinfo. > > >>>>> > > >>>>> Our device still gives us back a u8 active_speed so both these > > >>>>> are getting stored in the u16. This fix I proposed simply gets > > >>>>> the active_speed from the netdev while the phys_state still > > >>>>> needs to come from the pvrdma device, i.e. the msb the of the > > >>>>> u16. I also removed some unused functions as a result. > > >>>>> > > >>>>> Alternatively, I could change the u8 active_width and u16 > > >>>>> active_speed to reserved now that we're getting the active_speed > > >>>>> and active_width from the ib_get_eth_speed function. > > >>>> > > >>>> Jason, did you have any comments on this or did you want me > > >>>> to just send v1 with an updated description? > > >>> > > >>> I still haven't figured out what this is fixing. > > >>> > > >>> Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix > > >>> to revert the type? > > >> > > >> I can revert it but I thought that it had to a u16 based on the IBTA, no? > > >> Or does that not apply to device-level stuff? > > > > > > You didn't answer the question, it it ABI to some kind of FW interface > > > or something? > > > > > > *HOW* did two fields get overlapped onto a single u16?? The compiler > > > won't do this.. > > > > > > > It is an ABI to the device for port attributes. The device gives us back > > this structure for query port verb. The response from the device is > > memcopied into this pvrdma_port_attr structure. So both the bytes > > representing active_speed and phys_state from the device are copied > > into the single u16 in this structure. > > So it is ABI and it shouldn't have been changed, point at the stuff > that made it ABI and revert the structure layout change.. How will it work for the new IBTA speed? Thanks > > Jason
On 11/2/20 10:56 PM, Leon Romanovsky wrote: > On Mon, Nov 02, 2020 at 02:46:40PM -0400, Jason Gunthorpe wrote: >> On Mon, Nov 02, 2020 at 10:38:19AM -0800, Adit Ranadive wrote: >>> On 11/2/20 10:27 AM, Jason Gunthorpe wrote: >>>> On Mon, Nov 02, 2020 at 10:21:21AM -0800, Adit Ranadive wrote: >>>>> On 11/2/20 10:02 AM, Jason Gunthorpe wrote: >>>>>> On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: >>>>>>> On 10/29/20 9:16 AM, Adit Ranadive wrote: >>>>>>>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: >>>>>>>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: >>>>>>>>>> The PVRDMA device still reports the active_speed in u8. >>>>>>>>>> Lets use the ib_eth_get_speed to report the speed and >>>>>>>>>> width. Unfortunately, phys_state gets stored as msb of >>>>>>>>>> the new u16 active_speed. >>>>>>>>> >>>>>>>>> This explanation is not clear, I have no idea what this is fixing >>>>>>>> >>>>>>>> It seemed more clear to me in my head, I guess :). >>>>>>>> >>>>>>>> After commit 376ceb31ff87 changed the active_speed attribute to >>>>>>>> u16, both the active_speed and phys_state attributes in the >>>>>>>> pvrdma_port_attr struct are getting stored in this u16. As a >>>>>>>> result, these show up as invalid values in ibv_devinfo. >>>>>>>> >>>>>>>> Our device still gives us back a u8 active_speed so both these >>>>>>>> are getting stored in the u16. This fix I proposed simply gets >>>>>>>> the active_speed from the netdev while the phys_state still >>>>>>>> needs to come from the pvrdma device, i.e. the msb the of the >>>>>>>> u16. I also removed some unused functions as a result. >>>>>>>> >>>>>>>> Alternatively, I could change the u8 active_width and u16 >>>>>>>> active_speed to reserved now that we're getting the active_speed >>>>>>>> and active_width from the ib_get_eth_speed function. >>>>>>> >>>>>>> Jason, did you have any comments on this or did you want me >>>>>>> to just send v1 with an updated description? >>>>>> >>>>>> I still haven't figured out what this is fixing. >>>>>> >>>>>> Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix >>>>>> to revert the type? >>>>> >>>>> I can revert it but I thought that it had to a u16 based on the IBTA, no? >>>>> Or does that not apply to device-level stuff? >>>> >>>> You didn't answer the question, it it ABI to some kind of FW interface >>>> or something? >>>> >>>> *HOW* did two fields get overlapped onto a single u16?? The compiler >>>> won't do this.. >>>> >>> >>> It is an ABI to the device for port attributes. The device gives us back >>> this structure for query port verb. The response from the device is >>> memcopied into this pvrdma_port_attr structure. So both the bytes >>> representing active_speed and phys_state from the device are copied >>> into the single u16 in this structure. >> >> So it is ABI and it shouldn't have been changed, point at the stuff >> that made it ABI and revert the structure layout change.. > > How will it work for the new IBTA speed? Hopefully that should be addressed in another patch I'll send out that uses the ib_get_eth_speed api? > > Thanks > >> >> Jason
On Tue, Nov 03, 2020 at 02:41:06PM -0800, Adit Ranadive wrote: > On 11/2/20 10:56 PM, Leon Romanovsky wrote: > > On Mon, Nov 02, 2020 at 02:46:40PM -0400, Jason Gunthorpe wrote: > >> On Mon, Nov 02, 2020 at 10:38:19AM -0800, Adit Ranadive wrote: > >>> On 11/2/20 10:27 AM, Jason Gunthorpe wrote: > >>>> On Mon, Nov 02, 2020 at 10:21:21AM -0800, Adit Ranadive wrote: > >>>>> On 11/2/20 10:02 AM, Jason Gunthorpe wrote: > >>>>>> On Mon, Nov 02, 2020 at 09:55:25AM -0800, Adit Ranadive wrote: > >>>>>>> On 10/29/20 9:16 AM, Adit Ranadive wrote: > >>>>>>>> On 10/29/20 4:57 AM, Jason Gunthorpe wrote: > >>>>>>>>> On Wed, Oct 28, 2020 at 11:19:45PM +0000, Adit Ranadive wrote: > >>>>>>>>>> The PVRDMA device still reports the active_speed in u8. > >>>>>>>>>> Lets use the ib_eth_get_speed to report the speed and > >>>>>>>>>> width. Unfortunately, phys_state gets stored as msb of > >>>>>>>>>> the new u16 active_speed. > >>>>>>>>> > >>>>>>>>> This explanation is not clear, I have no idea what this is fixing > >>>>>>>> > >>>>>>>> It seemed more clear to me in my head, I guess :). > >>>>>>>> > >>>>>>>> After commit 376ceb31ff87 changed the active_speed attribute to > >>>>>>>> u16, both the active_speed and phys_state attributes in the > >>>>>>>> pvrdma_port_attr struct are getting stored in this u16. As a > >>>>>>>> result, these show up as invalid values in ibv_devinfo. > >>>>>>>> > >>>>>>>> Our device still gives us back a u8 active_speed so both these > >>>>>>>> are getting stored in the u16. This fix I proposed simply gets > >>>>>>>> the active_speed from the netdev while the phys_state still > >>>>>>>> needs to come from the pvrdma device, i.e. the msb the of the > >>>>>>>> u16. I also removed some unused functions as a result. > >>>>>>>> > >>>>>>>> Alternatively, I could change the u8 active_width and u16 > >>>>>>>> active_speed to reserved now that we're getting the active_speed > >>>>>>>> and active_width from the ib_get_eth_speed function. > >>>>>>> > >>>>>>> Jason, did you have any comments on this or did you want me > >>>>>>> to just send v1 with an updated description? > >>>>>> > >>>>>> I still haven't figured out what this is fixing. > >>>>>> > >>>>>> Is 'struct pvrdma_port_attr' some kind of ABI? If so why isn't the fix > >>>>>> to revert the type? > >>>>> > >>>>> I can revert it but I thought that it had to a u16 based on the IBTA, no? > >>>>> Or does that not apply to device-level stuff? > >>>> > >>>> You didn't answer the question, it it ABI to some kind of FW interface > >>>> or something? > >>>> > >>>> *HOW* did two fields get overlapped onto a single u16?? The compiler > >>>> won't do this.. > >>>> > >>> > >>> It is an ABI to the device for port attributes. The device gives us back > >>> this structure for query port verb. The response from the device is > >>> memcopied into this pvrdma_port_attr structure. So both the bytes > >>> representing active_speed and phys_state from the device are copied > >>> into the single u16 in this structure. > >> > >> So it is ABI and it shouldn't have been changed, point at the stuff > >> that made it ABI and revert the structure layout change.. > > > > How will it work for the new IBTA speed? > > Hopefully that should be addressed in another patch I'll send out > that uses the ib_get_eth_speed api? It will work for now, but any new speed above NDR will break pvrdma. Thanks > > > > > Thanks > > > >> > >> Jason
diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h index c142f5e7f25f..17f20506575f 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma.h @@ -360,24 +360,6 @@ static inline enum pvrdma_port_width ib_port_width_to_pvrdma( return (enum pvrdma_port_width)width; } -static inline enum ib_port_width pvrdma_port_width_to_ib( - enum pvrdma_port_width width) -{ - return (enum ib_port_width)width; -} - -static inline enum pvrdma_port_speed ib_port_speed_to_pvrdma( - enum ib_port_speed speed) -{ - return (enum pvrdma_port_speed)speed; -} - -static inline enum ib_port_speed pvrdma_port_speed_to_ib( - enum pvrdma_port_speed speed) -{ - return (enum ib_port_speed)speed; -} - static inline int ib_qp_attr_mask_to_pvrdma(int attr_mask) { return attr_mask & PVRDMA_MASK(PVRDMA_QP_ATTR_MASK_MAX); diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c index fc412cbfd042..221705837d78 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.c @@ -167,11 +167,11 @@ int pvrdma_query_port(struct ib_device *ibdev, u8 port, props->sm_sl = resp->attrs.sm_sl; props->subnet_timeout = resp->attrs.subnet_timeout; props->init_type_reply = resp->attrs.init_type_reply; - props->active_width = pvrdma_port_width_to_ib(resp->attrs.active_width); - props->active_speed = pvrdma_port_speed_to_ib(resp->attrs.active_speed); - props->phys_state = resp->attrs.phys_state; + err = ib_get_eth_speed(ibdev, 1, &props->active_speed, + &props->active_width); + props->phys_state = resp->attrs.active_speed >> 8; - return 0; + return err; } /** diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h index f0e5ffba2d51..715416902992 100644 --- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h +++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h @@ -178,7 +178,7 @@ struct pvrdma_port_attr { u8 active_width; u16 active_speed; u8 phys_state; - u8 reserved[2]; + u8 reserved; }; struct pvrdma_global_route {
The PVRDMA device still reports the active_speed in u8. Lets use the ib_eth_get_speed to report the speed and width. Unfortunately, phys_state gets stored as msb of the new u16 active_speed. Fixes: 376ceb31ff87 ("RDMA: Fix link active_speed size") Reviewed-by: Vishnu Dasa <vdasa@vmware.com> Signed-off-by: Adit Ranadive <aditr@vmware.com> --- drivers/infiniband/hw/vmw_pvrdma/pvrdma.h | 18 ------------------ .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 8 ++++---- .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- 3 files changed, 5 insertions(+), 23 deletions(-)