* [RFC] Proposal to address hfi1 UI and EPROM devices @ 2016-05-02 19:55 Dennis Dalessandro [not found] ` <20160502195502.GA31800-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Dennis Dalessandro @ 2016-05-02 19:55 UTC (permalink / raw) To: linux-rdma-u79uwXL29TY76Z2rM5mHXA Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w The hfi1 driver has a "UI" device associated with it. This is currently a char dev. It is used for debugging/diagnostics. Basically it provides direct user level access to HW registers. We have been exploring options to remove this char dev, yet retain the same functionality. One approach we have considered is using the regmap interface. This has a number of problems. It is primarily a read only interface. The write capability is not even available as a compile time option. A code change is needed to make use of it. There are other issues as well such as being 32bit when we need 64bit. So this seems like the wrong option. Another approach is using the driver's resource0 file. We can mmap() the BAR and access the registers. The main drawback we have with that is we have some hardware requirements that the driver needs to arbitrate access to certain registers. This means some form of kernel/user space shared locking. We are currently not aware of anything that the kernel provides to achieve this. However, we are still looking and would appreciate any pointers. The third approach would be to take our existing UI implementation and plop it down in debugfs as a binary file. We can control our locking just as we do today, in the driver. This gets rid of the character device, and it's not even available unless the admin decides to mount debugfs. We also should be able to use any of these schemes to handle our eprom reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, but we technically don't need to do it right now when we could make do with the "UI" functionality and hide the details in user space. In the future there may well be value in having an eprom capability in the rdma sub-system, but for now we believe we can avoid extending the kernel in this regard. -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160502195502.GA31800-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160502195502.GA31800-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2016-05-03 16:24 ` Leon Romanovsky [not found] ` <20160503162457.GB29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-03 17:31 ` Jason Gunthorpe 2016-05-05 18:57 ` Doug Ledford 2 siblings, 1 reply; 22+ messages in thread From: Leon Romanovsky @ 2016-05-03 16:24 UTC (permalink / raw) To: Dennis Dalessandro Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 953 bytes --] On Mon, May 02, 2016 at 03:55:02PM -0400, Dennis Dalessandro wrote: > We also should be able to use any of these schemes to handle our eprom > reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, but > we technically don't need to do it right now when we could make do with the > "UI" functionality and hide the details in user space. In the future there > may well be value in having an eprom capability in the rdma sub-system, but > for now we believe we can avoid extending the kernel in this regard. Just to understand better your RFC. Are you asking from the community excuse do not implement core functionality just because you don't need it? Am I right? Thanks > > -Denny > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160503162457.GB29160-2ukJVAZIZ/Y@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160503162457.GB29160-2ukJVAZIZ/Y@public.gmane.org> @ 2016-05-03 16:54 ` Dennis Dalessandro [not found] ` <20160503165403.GA11903-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Dennis Dalessandro @ 2016-05-03 16:54 UTC (permalink / raw) To: Leon Romanovsky Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Tue, May 03, 2016 at 07:24:57PM +0300, Leon Romanovsky wrote: >On Mon, May 02, 2016 at 03:55:02PM -0400, Dennis Dalessandro wrote: >> We also should be able to use any of these schemes to handle our eprom >> reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, but >> we technically don't need to do it right now when we could make do with the >> "UI" functionality and hide the details in user space. In the future there >> may well be value in having an eprom capability in the rdma sub-system, but >> for now we believe we can avoid extending the kernel in this regard. > >Just to understand better your RFC. Are you asking from the community >excuse do not implement core functionality just because you don't need >it? Am I right? The purpose of the RFC is to get the community's feedback on our plans to solve our UI/EPROM issue. The paragraphs [1] not snipped in your response focus on that. It comes down to: Use resource0 which does not support locking vs use a file in /sys/kernel/debug. The point of the particular paragraph you are questioning is that I don't think we should add more code and complexity to the kernel for something that is not needed. [1] http://marc.info/?l=linux-rdma&m=146221891012428&w=2 -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160503165403.GA11903-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160503165403.GA11903-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2016-05-03 18:42 ` Leon Romanovsky [not found] ` <20160503184218.GC29160-2ukJVAZIZ/Y@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Leon Romanovsky @ 2016-05-03 18:42 UTC (permalink / raw) To: Dennis Dalessandro Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 1788 bytes --] On Tue, May 03, 2016 at 12:54:05PM -0400, Dennis Dalessandro wrote: > On Tue, May 03, 2016 at 07:24:57PM +0300, Leon Romanovsky wrote: > >On Mon, May 02, 2016 at 03:55:02PM -0400, Dennis Dalessandro wrote: > >>We also should be able to use any of these schemes to handle our eprom > >>reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, but > >>we technically don't need to do it right now when we could make do with the > >>"UI" functionality and hide the details in user space. In the future there > >>may well be value in having an eprom capability in the rdma sub-system, but > >>for now we believe we can avoid extending the kernel in this regard. > > > >Just to understand better your RFC. Are you asking from the community > >excuse do not implement core functionality just because you don't need > >it? Am I right? > > The purpose of the RFC is to get the community's feedback on our plans to > solve our UI/EPROM issue. The paragraphs [1] not snipped in your response > focus on that. It comes down to: > > Use resource0 which does not support locking vs use a file in > /sys/kernel/debug. I didn't quote the whole email because I didn't see any question in these 3 options: * First option - doesn't meet the requirements and hard to extend. * Second option - doesn't meet the requirements and HW limitations * Third option - presented as one possible option and looks like the correct one. And the real question is "where" do you need to implement third option. > > The point of the particular paragraph you are questioning is that I don't > think we should add more code and complexity to the kernel for something > that is not needed. > > [1] http://marc.info/?l=linux-rdma&m=146221891012428&w=2 > > -Denny [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160503184218.GC29160-2ukJVAZIZ/Y@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160503184218.GC29160-2ukJVAZIZ/Y@public.gmane.org> @ 2016-05-04 4:41 ` Leon Romanovsky [not found] ` <20160504044107.GE29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-04 12:20 ` Dennis Dalessandro 1 sibling, 1 reply; 22+ messages in thread From: Leon Romanovsky @ 2016-05-04 4:41 UTC (permalink / raw) To: Dennis Dalessandro Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 2239 bytes --] On Tue, May 03, 2016 at 09:42:18PM +0300, Leon Romanovsky wrote: > On Tue, May 03, 2016 at 12:54:05PM -0400, Dennis Dalessandro wrote: > > On Tue, May 03, 2016 at 07:24:57PM +0300, Leon Romanovsky wrote: > > >On Mon, May 02, 2016 at 03:55:02PM -0400, Dennis Dalessandro wrote: > > >>We also should be able to use any of these schemes to handle our eprom > > >>reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, but > > >>we technically don't need to do it right now when we could make do with the > > >>"UI" functionality and hide the details in user space. In the future there > > >>may well be value in having an eprom capability in the rdma sub-system, but > > >>for now we believe we can avoid extending the kernel in this regard. > > > > > >Just to understand better your RFC. Are you asking from the community > > >excuse do not implement core functionality just because you don't need > > >it? Am I right? > > > > The purpose of the RFC is to get the community's feedback on our plans to > > solve our UI/EPROM issue. The paragraphs [1] not snipped in your response > > focus on that. It comes down to: > > > > Use resource0 which does not support locking vs use a file in > > /sys/kernel/debug. > > I didn't quote the whole email because I didn't see any question in > these 3 options: > * First option - doesn't meet the requirements and hard to extend. > * Second option - doesn't meet the requirements and HW limitations > * Third option - presented as one possible option and looks like the > correct one. > > And the real question is "where" do you need to implement third option. And to remove the tension, all what you was supposed to ask can be summarized in one question: "Do we have other customers for EEPROM in our RDMA stack?". If the answer is yes, you will need to implement it in core, and if the answer is no, you will implement it in your driver. Hope it helps. > > > > > The point of the particular paragraph you are questioning is that I don't > > think we should add more code and complexity to the kernel for something > > that is not needed. > > > > [1] http://marc.info/?l=linux-rdma&m=146221891012428&w=2 > > > > -Denny [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160504044107.GE29160-2ukJVAZIZ/Y@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160504044107.GE29160-2ukJVAZIZ/Y@public.gmane.org> @ 2016-05-04 12:36 ` Dennis Dalessandro [not found] ` <20160504123621.GC10916-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Dennis Dalessandro @ 2016-05-04 12:36 UTC (permalink / raw) To: Leon Romanovsky Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Wed, May 04, 2016 at 07:41:07AM +0300, Leon Romanovsky wrote: >> I didn't quote the whole email because I didn't see any question in >> these 3 options: >> * First option - doesn't meet the requirements and hard to extend. >> * Second option - doesn't meet the requirements and HW limitations >> * Third option - presented as one possible option and looks like the >> correct one. >> >> And the real question is "where" do you need to implement third option. > >And to remove the tension, all what you was supposed to ask can be >summarized in one question: "Do we have other customers for EEPROM in >our RDMA stack?". If the answer is yes, you will need to implement it in >core, and if the answer is no, you will implement it in your driver. > >Hope it helps. I think it's slightly more complicated than that. There are three options really: core, driver, or get it out of the kernel. Whether we have other things that need EEPROM in the RDMA stack is only part of the question. We also should consider: do we have customers for EEPROM in the RDMA stack that could program/access said EEPROM using user space and are instead doing it in a driver? If that is the case then I'd say they should follow the path we are planning for hfi1 and remove the code from the kernel. If there is already a way to do something from user space that should be leveraged as much as possible. -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160504123621.GC10916-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160504123621.GC10916-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2016-05-04 18:15 ` Jason Gunthorpe [not found] ` <20160504181509.GA20488-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2016-05-04 18:15 UTC (permalink / raw) To: Dennis Dalessandro Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Wed, May 04, 2016 at 08:36:21AM -0400, Dennis Dalessandro wrote: > I think it's slightly more complicated than that. There are three options > really: core, driver, or get it out of the kernel. >From a core maintenance perspective, I think it is very simple, if someone wishes to add code to their driver to manipulate the EEPROM, then at this point the work to make a common uAPI falls on to their shoulders. I'd also make a very clear message to driver submitters: Do not include uAPIs in your initial driver patch set. Those should follow on as dedicated well identified patches so that they attracted the proper review. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160504181509.GA20488-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160504181509.GA20488-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-05-05 6:38 ` Leon Romanovsky [not found] ` <20160505063834.GH29160-2ukJVAZIZ/Y@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Leon Romanovsky @ 2016-05-05 6:38 UTC (permalink / raw) To: Jason Gunthorpe Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 1180 bytes --] On Wed, May 04, 2016 at 12:15:09PM -0600, Jason Gunthorpe wrote: > On Wed, May 04, 2016 at 08:36:21AM -0400, Dennis Dalessandro wrote: > > I think it's slightly more complicated than that. There are three options > > really: core, driver, or get it out of the kernel. > > From a core maintenance perspective, I think it is very simple, if > someone wishes to add code to their driver to manipulate the EEPROM, > then at this point the work to make a common uAPI falls on to their > shoulders. Agree, And before rushing to code this functionality, please double check that there is no already implemented such similar common functionality in other subsystems. I bet that you already did it. > > I'd also make a very clear message to driver submitters: Do not > include uAPIs in your initial driver patch set. Those should follow on > as dedicated well identified patches so that they attracted the proper > review. It is hard to agree with this point. It contradicts to development model of submitting whole feature at once and not unconnected piece of code, which someone will be needed to maintain without any real user behind it. > > Jason [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160505063834.GH29160-2ukJVAZIZ/Y@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505063834.GH29160-2ukJVAZIZ/Y@public.gmane.org> @ 2016-05-05 12:00 ` Dennis Dalessandro [not found] ` <20160505120033.GA23895-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Dennis Dalessandro @ 2016-05-05 12:00 UTC (permalink / raw) To: Leon Romanovsky Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Thu, May 05, 2016 at 09:38:34AM +0300, Leon Romanovsky wrote: >> I'd also make a very clear message to driver submitters: Do not >> include uAPIs in your initial driver patch set. Those should follow on >> as dedicated well identified patches so that they attracted the proper >> review. > >It is hard to agree with this point. >It contradicts to development model of submitting whole feature at once >and not unconnected piece of code, which someone will be needed to >maintain without any real user behind it. I think the message really should be that if your driver contains uAPI changes those should be in separate patches that are clearly identified. So if you have a driver that is developed off-list initially, instead of just breaking it up into chunks for submission add another step. Something like this: 1) Submit patch series which break-ups internally developed code 2) Submit patch series with separated out uAPI code 3) Submit patch that makes the build go-live These can all be submitted together, but with the patches broken up like this reviewers can target uAPI code more easily. -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160505120033.GA23895-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505120033.GA23895-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> @ 2016-05-05 12:39 ` Leon Romanovsky [not found] ` <20160505123932.GK29160-2ukJVAZIZ/Y@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Leon Romanovsky @ 2016-05-05 12:39 UTC (permalink / raw) To: Dennis Dalessandro Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 1579 bytes --] On Thu, May 05, 2016 at 08:00:34AM -0400, Dennis Dalessandro wrote: > On Thu, May 05, 2016 at 09:38:34AM +0300, Leon Romanovsky wrote: > >>I'd also make a very clear message to driver submitters: Do not > >>include uAPIs in your initial driver patch set. Those should follow on > >>as dedicated well identified patches so that they attracted the proper > >>review. > > > >It is hard to agree with this point. > >It contradicts to development model of submitting whole feature at once > >and not unconnected piece of code, which someone will be needed to > >maintain without any real user behind it. > > I think the message really should be that if your driver contains uAPI > changes those should be in separate patches that are clearly identified. So > if you have a driver that is developed off-list initially, instead of just > breaking it up into chunks for submission add another step. > > Something like this: > 1) Submit patch series which break-ups internally developed code > 2) Submit patch series with separated out uAPI code > 3) Submit patch that makes the build go-live > > These can all be submitted together, but with the patches broken up like > this reviewers can target uAPI code more easily. At the end, there is no point of accepting (1) without finished review of (2 and 3). Right now all patch series already have such internal separation in a slightly different order. I'm as a reviewer prefer to see whole picture and follow logic from the general entry point to the specific implementation in driver. > > -Denny [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160505123932.GK29160-2ukJVAZIZ/Y@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505123932.GK29160-2ukJVAZIZ/Y@public.gmane.org> @ 2016-05-05 18:08 ` Jason Gunthorpe [not found] ` <20160505180843.GA5957-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2016-05-05 18:08 UTC (permalink / raw) To: Leon Romanovsky Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Thu, May 05, 2016 at 03:39:32PM +0300, Leon Romanovsky wrote: > > I think the message really should be that if your driver contains uAPI > > changes those should be in separate patches that are clearly identified. So > > if you have a driver that is developed off-list initially, instead of just > > breaking it up into chunks for submission add another step. > > > > Something like this: > > 1) Submit patch series which break-ups internally developed code > > 2) Submit patch series with separated out uAPI code > > 3) Submit patch that makes the build go-live Yes. Perhaps even submit #2 after getting #1 mainlined. Make it easy to find the important/controversial things and the review process will work much better for everyone. Buring stuff in a monster patch is just going to stretch it out. > > These can all be submitted together, but with the patches broken up like > > this reviewers can target uAPI code more easily. > At the end, there is no point of accepting (1) without finished review > of (2 and 3). Right now all patch series already have such internal separation > in a slightly different order. Eh? Drivers should be able to stand alone without dedicated uapis (excluding the udata stuff). For instance, the HFI1 driver is as functional as any other RDMA driver without it's cdev, eeprom, debug and sysfs uAPIs. Those are all value add features that do not impact the driver's ability to operate as an RDMA device. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160505180843.GA5957-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505180843.GA5957-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-05-05 18:36 ` Doug Ledford [not found] ` <b7f1735c-5362-514e-268a-49a09b316a88-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-05-06 4:36 ` Leon Romanovsky 1 sibling, 1 reply; 22+ messages in thread From: Doug Ledford @ 2016-05-05 18:36 UTC (permalink / raw) To: Jason Gunthorpe, Leon Romanovsky Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 2178 bytes --] On 05/05/2016 02:08 PM, Jason Gunthorpe wrote: > On Thu, May 05, 2016 at 03:39:32PM +0300, Leon Romanovsky wrote: > >>> I think the message really should be that if your driver contains uAPI >>> changes those should be in separate patches that are clearly identified. So >>> if you have a driver that is developed off-list initially, instead of just >>> breaking it up into chunks for submission add another step. >>> >>> Something like this: >>> 1) Submit patch series which break-ups internally developed code >>> 2) Submit patch series with separated out uAPI code >>> 3) Submit patch that makes the build go-live > > Yes. Perhaps even submit #2 after getting #1 mainlined. > > Make it easy to find the important/controversial things and the review > process will work much better for everyone. Buring stuff in a monster > patch is just going to stretch it out. > >>> These can all be submitted together, but with the patches broken up like >>> this reviewers can target uAPI code more easily. > >> At the end, there is no point of accepting (1) without finished review >> of (2 and 3). Right now all patch series already have such internal separation >> in a slightly different order. > > Eh? > > Drivers should be able to stand alone without dedicated uapis > (excluding the udata stuff). That's probably true most of the time... > For instance, the HFI1 driver is as functional as any other RDMA > driver without it's cdev, eeprom, debug and sysfs uAPIs. Those are all > value add features that do not impact the driver's ability to operate > as an RDMA device. In this case though, not so much. The qib and hfi1 devices have always been PSM devices first, verbs devices merely as a compat layer. It's easier to provides a verbs interface for kernel ULPs than it is to write PSM in the kernel. But their hardware is designed around the way PSM uses it, and how verbs makes use of it is decidedly sub-optimal. They could do without the eeprom, debug, and sysfs stuff, but the cdev and PSM not so much. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <b7f1735c-5362-514e-268a-49a09b316a88-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <b7f1735c-5362-514e-268a-49a09b316a88-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-05-05 18:46 ` Jason Gunthorpe 0 siblings, 0 replies; 22+ messages in thread From: Jason Gunthorpe @ 2016-05-05 18:46 UTC (permalink / raw) To: Doug Ledford Cc: Leon Romanovsky, Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Thu, May 05, 2016 at 02:36:16PM -0400, Doug Ledford wrote: > > For instance, the HFI1 driver is as functional as any other RDMA > > driver without it's cdev, eeprom, debug and sysfs uAPIs. Those are all > > value add features that do not impact the driver's ability to operate > > as an RDMA device. > > In this case though, not so much. The qib and hfi1 devices have always > been PSM devices first, verbs devices merely as a compat layer. > It's The driver can do both interfaces, and the hardware was clearly designed to accelerate the typical verbs operations as well as support the psm style of operation. > easier to provides a verbs interface for kernel ULPs than it is to write > PSM in the kernel. But their hardware is designed around the way PSM > uses it, and how verbs makes use of it is decidedly sub-optimal. They > could do without the eeprom, debug, and sysfs stuff, but the cdev and > PSM not so much. I didn't say the cdev shouldn't exist, only that the core driver should have been submitted first and then patched to add in the other stuff, maybe in the same series, but certainly split up. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505180843.GA5957-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-05 18:36 ` Doug Ledford @ 2016-05-06 4:36 ` Leon Romanovsky 1 sibling, 0 replies; 22+ messages in thread From: Leon Romanovsky @ 2016-05-06 4:36 UTC (permalink / raw) To: Jason Gunthorpe Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 1871 bytes --] On Thu, May 05, 2016 at 12:08:43PM -0600, Jason Gunthorpe wrote: > On Thu, May 05, 2016 at 03:39:32PM +0300, Leon Romanovsky wrote: > > > > I think the message really should be that if your driver contains uAPI > > > changes those should be in separate patches that are clearly identified. So > > > if you have a driver that is developed off-list initially, instead of just > > > breaking it up into chunks for submission add another step. > > > > > > Something like this: > > > 1) Submit patch series which break-ups internally developed code > > > 2) Submit patch series with separated out uAPI code > > > 3) Submit patch that makes the build go-live > > Yes. Perhaps even submit #2 after getting #1 mainlined. > > Make it easy to find the important/controversial things and the review > process will work much better for everyone. Buring stuff in a monster > patch is just going to stretch it out. > > > > These can all be submitted together, but with the patches broken up like > > > this reviewers can target uAPI code more easily. > > > At the end, there is no point of accepting (1) without finished review > > of (2 and 3). Right now all patch series already have such internal separation > > in a slightly different order. > > Eh? > > Drivers should be able to stand alone without dedicated uapis > (excluding the udata stuff). And they are, however the point is a little bit different. There is no meaning in internal feature implementation without access through verbs interface. > > For instance, the HFI1 driver is as functional as any other RDMA > driver without it's cdev, eeprom, debug and sysfs uAPIs. Those are all > value add features that do not impact the driver's ability to operate > as an RDMA device. Let's close for them PSM and verbs interfaces and see how it is operable. > > Jason [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160503184218.GC29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-04 4:41 ` Leon Romanovsky @ 2016-05-04 12:20 ` Dennis Dalessandro 1 sibling, 0 replies; 22+ messages in thread From: Dennis Dalessandro @ 2016-05-04 12:20 UTC (permalink / raw) To: Leon Romanovsky Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Tue, May 03, 2016 at 09:42:18PM +0300, Leon Romanovsky wrote: >On Tue, May 03, 2016 at 12:54:05PM -0400, Dennis Dalessandro wrote: >> On Tue, May 03, 2016 at 07:24:57PM +0300, Leon Romanovsky wrote: >> >On Mon, May 02, 2016 at 03:55:02PM -0400, Dennis Dalessandro wrote: >> >>We also should be able to use any of these schemes to handle our eprom >> >>reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, but >> >>we technically don't need to do it right now when we could make do with the >> >>"UI" functionality and hide the details in user space. In the future there >> >>may well be value in having an eprom capability in the rdma sub-system, but >> >>for now we believe we can avoid extending the kernel in this regard. >> > >> >Just to understand better your RFC. Are you asking from the community >> >excuse do not implement core functionality just because you don't need >> >it? Am I right? >> >> The purpose of the RFC is to get the community's feedback on our plans to >> solve our UI/EPROM issue. The paragraphs [1] not snipped in your response >> focus on that. It comes down to: >> >> Use resource0 which does not support locking vs use a file in >> /sys/kernel/debug. > >I didn't quote the whole email because I didn't see any question in >these 3 options: >* First option - doesn't meet the requirements and hard to extend. >* Second option - doesn't meet the requirements and HW limitations >* Third option - presented as one possible option and looks like the > correct one. > >And the real question is "where" do you need to implement third option. As in where would it be visible? That would be in /sys/kernel/debug/hfi1. We have some counters and stats there already. Based on Jason's response I'm thinking we can use that just for the locking aspect. -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160502195502.GA31800-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2016-05-03 16:24 ` Leon Romanovsky @ 2016-05-03 17:31 ` Jason Gunthorpe [not found] ` <20160503173130.GA1921-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-05 18:57 ` Doug Ledford 2 siblings, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2016-05-03 17:31 UTC (permalink / raw) To: Dennis Dalessandro Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Mon, May 02, 2016 at 03:55:02PM -0400, Dennis Dalessandro wrote: > One approach we have considered is using the regmap interface. This has a > number of problems. It is primarily a read only interface. The write > capability is not even available as a compile time option. A code change is > needed to make use of it. There are other issues as well such as being 32bit > when we need 64bit. So this seems like the wrong option. Yah, the places I've seen regmap used don't seem to match this use case. > Another approach is using the driver's resource0 file. We can mmap() the BAR > and access the registers. The main drawback we have with that is we have > some hardware requirements that the driver needs to arbitrate access to > certain registers. This means some form of kernel/user space shared locking. > We are currently not aware of anything that the kernel provides to achieve > this. However, we are still looking and would appreciate any pointers. You should probably use resource0 as much as possible - that minimizes the kernel code required. > The third approach would be to take our existing UI implementation and plop > it down in debugfs as a binary file. We can control our locking just as we > do today, in the driver. This gets rid of the character device, and it's not > even available unless the admin decides to mount debugfs. .. and use debugfs just for that little bit of shared locking you need to make resource0 work out. Minimizing the kernel impact for debugging code is generally desirable. > We also should be able to use any of these schemes to handle our eprom > reading/writing. Adding eprom to IPoIB as Doug suggested is a fine > plan, but I would be fine with this if you moved the eeprom handling to user space and used resource0. Less fine if the code remains in the kernel but just moves to debugfs. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160503173130.GA1921-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160503173130.GA1921-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-05-04 12:17 ` Dennis Dalessandro 0 siblings, 0 replies; 22+ messages in thread From: Dennis Dalessandro @ 2016-05-04 12:17 UTC (permalink / raw) To: Jason Gunthorpe Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, dledford-H+wXaHxf7aLQT0dZR+AlfA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Tue, May 03, 2016 at 11:31:30AM -0600, Jason Gunthorpe wrote: >> Another approach is using the driver's resource0 file. We can mmap() the BAR >> and access the registers. The main drawback we have with that is we have >> some hardware requirements that the driver needs to arbitrate access to >> certain registers. This means some form of kernel/user space shared locking. >> We are currently not aware of anything that the kernel provides to achieve >> this. However, we are still looking and would appreciate any pointers. > >You should probably use resource0 as much as possible - that minimizes >the kernel code required. > >> The third approach would be to take our existing UI implementation and plop >> it down in debugfs as a binary file. We can control our locking just as we >> do today, in the driver. This gets rid of the character device, and it's not >> even available unless the admin decides to mount debugfs. > >.. and use debugfs just for that little bit of shared locking you need >to make resource0 work out. Yeah. I was hoping we could find some other kernel facility that can provide a locking type mechanism between the driver and user space rather than go roll our own. We'll come up with something more concrete in the form of patch and post soon though. >Minimizing the kernel impact for debugging code is generally desirable. Agree. Especially in this case where we should be able to make do with the already provided facilities. -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160502195502.GA31800-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2016-05-03 16:24 ` Leon Romanovsky 2016-05-03 17:31 ` Jason Gunthorpe @ 2016-05-05 18:57 ` Doug Ledford [not found] ` <72645a3b-5945-419a-d7af-1c065080e415-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2 siblings, 1 reply; 22+ messages in thread From: Doug Ledford @ 2016-05-05 18:57 UTC (permalink / raw) To: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA Cc: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 2702 bytes --] On 05/02/2016 03:55 PM, Dennis Dalessandro wrote: [ snip stuff about the UI file ] The UI file discussion appears to have reached a reasonable conclusion, so I'm not going to add to it. > We also should be able to use any of these schemes to handle our eprom > reading/writing. Adding eprom to IPoIB as Doug suggested is a fine plan, > but > we technically don't need to do it right now when we could make do with the > "UI" functionality and hide the details in user space. In the future > there may well be value in having an eprom capability in the rdma > sub-system, but for now we believe we can avoid extending the kernel in > this regard. I'm not sure I agree here. First, on the "technically don't need to do it" and "can make do". At least one of the points of these reviews is to identify and squash cases of "technically don't need to do it" and "can make do" when it revolves around doing something half-assed that should be done properly instead. The cxgb? drivers use the kernel firmware load mechanism. I'm fine with that. The Mellanox drivers use a custom, user space tool. I *was* fine with that, when it was a one off thing. Now you are talking about doing the same thing, and it's going to become a two off thing. That makes me somewhat less OK with either one. Upfitting IPoIB to pass through the netstack eeprom stuff should be almost trivial to do. 1) Add eeprom_get and eeprom_set routines to struct ib_device 2) In driver, set eeprom_get and eeprom_set in ib_device 3) In IPoIB, copy eeprom_get and eeprom_set from ib_device to ethtool ops Everything else is done in user space. You can use ethtool, or you could write a custom tool to do the trick. Mellanox could do the same thing and modify mstflint to just pass the work to the ethool entry points while still retaining all of their FS/IMG/GUID/MAC/PSID magic in mstflint. For your case, if all you need to do is to write a new file to the eeprom, you should be able to do this: cat <binary-image> | ethtool --change-eeprom DEVNAME magic MAGIC offset 0 length `wc -c <binary-image>` and the eeprom is written with the new data. If you need to do special things, like Mellanox, in terms of recovering burned data like GIDs or MACs or whatever, then you can write a tool that does that, but leaves the EEPROM banging to the driver and just passes the buffers around using the ethtool interface. Really, it doesn't look hard at all, and it sounds much better than continuing with everybody writing their own tool and reinventing the wheel. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <72645a3b-5945-419a-d7af-1c065080e415-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <72645a3b-5945-419a-d7af-1c065080e415-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-05-05 19:20 ` Jason Gunthorpe [not found] ` <20160505192024.GA17249-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2016-05-05 19:20 UTC (permalink / raw) To: Doug Ledford Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Thu, May 05, 2016 at 02:57:01PM -0400, Doug Ledford wrote: > and the eeprom is written with the new data. If you need to do special > things, like Mellanox, in terms of recovering burned data like GIDs > or The 'eeprom' and device firmware are very different things. hfi1 has both, and uses request_firmware too. I've never heard of a driver using ethtool eeprom to deal with nv firmware like mlx has. AFAIK there is no kernel convention for that stuff. It is more common for storage than network drivers. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160505192024.GA17249-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505192024.GA17249-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-05-05 19:32 ` Doug Ledford [not found] ` <5334ab9c-428a-547f-b80a-e0bee3f85449-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Doug Ledford @ 2016-05-05 19:32 UTC (permalink / raw) To: Jason Gunthorpe Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 2334 bytes --] On 05/05/2016 03:20 PM, Jason Gunthorpe wrote: > On Thu, May 05, 2016 at 02:57:01PM -0400, Doug Ledford wrote: > >> and the eeprom is written with the new data. If you need to do special >> things, like Mellanox, in terms of recovering burned data like GIDs >> or > > The 'eeprom' and device firmware are very different things. hfi1 has > both, and uses request_firmware too. > > I've never heard of a driver using ethtool eeprom to deal with nv > firmware like mlx has. There's no reason it couldn't. Since you can pass offset and length parameters and write things in multiple chunks, you can actually set up access to eeprom, nv ram, and firmware all through the one interface simply by defining the start/stop points of each to be at specific, well known locations for your device. Then your routine could write to the correct device based upon the location. And in truth, whether you are talking about firmware, some sort of eeprom, or nv ram, it's all the same basic principle: read from or write to this non-volatile memory of arbitrary size (ok, depending on the device, firmware might be different...it might not be non-volatile, or there might be a non-volatile version for boot and a running copy that can be downloaded by the kernel where the running copy and the boot time non-volatile copy are not necessarily the same). Only the the nitty gritty of the access varies based upon the device in question. Aside from making sure to get the right device, user space couldn't care less about the distinction of the hardware type used to implement it. > AFAIK there is no kernel convention for that > stuff. It is more common for storage than network drivers. You're right, it is more common for storage devices (raid hardware in particular). And a lot of that hardware doesn't have a good means for doing this. Different vendors have rolled their own solutions (Dell for instance uses their own kernel driver to allow updated the firmware on all of the devices they ship, including raid storage controllers, onboard Ethernet and PXE booters, etc). But since we have a netdevice on everything, I see no reason not to use the eeprom routines already present. -- Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG KeyID: 0E572FDD [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 884 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <5334ab9c-428a-547f-b80a-e0bee3f85449-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <5334ab9c-428a-547f-b80a-e0bee3f85449-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-05-05 20:38 ` Jason Gunthorpe [not found] ` <20160505203858.GA18611-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 22+ messages in thread From: Jason Gunthorpe @ 2016-05-05 20:38 UTC (permalink / raw) To: Doug Ledford Cc: Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, ira.weiny-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Thu, May 05, 2016 at 03:32:32PM -0400, Doug Ledford wrote: > On 05/05/2016 03:20 PM, Jason Gunthorpe wrote: > > On Thu, May 05, 2016 at 02:57:01PM -0400, Doug Ledford wrote: > > > >> and the eeprom is written with the new data. If you need to do special > >> things, like Mellanox, in terms of recovering burned data like GIDs > >> or > > > > The 'eeprom' and device firmware are very different things. hfi1 has > > both, and uses request_firmware too. > > > > I've never heard of a driver using ethtool eeprom to deal with nv > > firmware like mlx has. > > There's no reason it couldn't. Since you can pass offset and length > parameters and write things in multiple chunks, you can actually set up > access to eeprom, nv ram, and firmware all through the one interface > simply by defining the start/stop points of each to be at specific, well > known locations for your device. Well, sort of. firmware write tends to be super-critical, doing it wrong can often mean the card is bricked. eg some devices require good firmware to start the PCI-E at all. This means the firmware write process needs to be bomb-proof and all competent vendors provide a user space program that does all necesary checks. Using the latest version of that program is always a good idea :) I would be strongly against moving that sort of complexity into the kernel. In turn this means users will never have a uniform user space experience, like 'cat | ethtool' - because that will not include the checks. Further, the very worst thing we could do is create a situation where a new kernel driver is required to do a firmware update (eg because we decided to move the checks into the kernel), and worse, potentially the new driver won't load on old firmware or old kernels. IIRC mlx had some problems like this once. >From that view, I think, if it can be don entirely via resource0, then that is what vendors should do, there is no value in a common API for firmware nv writing. ethtool eeprom exists as simple debugging/helper tool that should really never be used by end users. It is reasonble to duplicate it for eeprom like things, and AFAIK those uses cannot truely brick the hardware. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20160505203858.GA18611-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [RFC] Proposal to address hfi1 UI and EPROM devices [not found] ` <20160505203858.GA18611-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-05-05 23:58 ` Ira Weiny 0 siblings, 0 replies; 22+ messages in thread From: Ira Weiny @ 2016-05-05 23:58 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, Dennis Dalessandro, linux-rdma-u79uwXL29TY76Z2rM5mHXA, mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w, dean.luick-ral2JQCrhuEAvxtiuMwx3w, mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w, jubin.john-ral2JQCrhuEAvxtiuMwx3w On Thu, May 05, 2016 at 02:38:58PM -0600, Jason Gunthorpe wrote: > On Thu, May 05, 2016 at 03:32:32PM -0400, Doug Ledford wrote: > > On 05/05/2016 03:20 PM, Jason Gunthorpe wrote: > > > On Thu, May 05, 2016 at 02:57:01PM -0400, Doug Ledford wrote: > > > > > >> and the eeprom is written with the new data. If you need to do special > > >> things, like Mellanox, in terms of recovering burned data like GIDs > > >> or > > > > > > The 'eeprom' and device firmware are very different things. hfi1 has > > > both, and uses request_firmware too. > > > > > > I've never heard of a driver using ethtool eeprom to deal with nv > > > firmware like mlx has. > > > > There's no reason it couldn't. Since you can pass offset and length > > parameters and write things in multiple chunks, you can actually set up > > access to eeprom, nv ram, and firmware all through the one interface > > simply by defining the start/stop points of each to be at specific, well > > known locations for your device. > > Well, sort of. > > firmware write tends to be super-critical, doing it wrong can often > mean the card is bricked. eg some devices require good firmware to > start the PCI-E at all. The firmware for hfi1 is already done with the kernel standard firmware functions. I think we will need Mellanox to weigh in on their firmware update but I suspect that it is a critical operation which needs to be handled very carefully. > > This means the firmware write process needs to be bomb-proof and all > competent vendors provide a user space program that does all necesary > checks. Using the latest version of that program is always a good idea > :) > > I would be strongly against moving that sort of complexity into the > kernel. > > In turn this means users will never have a uniform user space > experience, like 'cat | ethtool' - because that will not include the > checks. > > Further, the very worst thing we could do is create a situation where > a new kernel driver is required to do a firmware update (eg because we > decided to move the checks into the kernel), and worse, potentially > the new driver won't load on old firmware or old kernels. IIRC mlx had > some problems like this once. > > From that view, I think, if it can be don entirely via resource0, then > that is what vendors should do, there is no value in a common API for > firmware nv writing. > > ethtool eeprom exists as simple debugging/helper tool that should > really never be used by end users. It is reasonble to duplicate it for > eeprom like things, and AFAIK those uses cannot truely brick the > hardware. The eeprom update for hfi1 should be a rare operation. resource0 gives us enough access to do this in the field but with very carefully crafted instructions and/or tools. This keeps the kernel simple yet gives us access without requiring users to change their kernels. The only exception would be a lock to tell the driver and hardware we are accessing registers. Perhaps this is as simple as calling open on a debugfs file then we automatically know when the process has gone away? All of this can be done with _very_ simple kernel code which really never has to change while maintaining a very high degree of flexibility. Ira -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-05-06 4:36 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-02 19:55 [RFC] Proposal to address hfi1 UI and EPROM devices Dennis Dalessandro [not found] ` <20160502195502.GA31800-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2016-05-03 16:24 ` Leon Romanovsky [not found] ` <20160503162457.GB29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-03 16:54 ` Dennis Dalessandro [not found] ` <20160503165403.GA11903-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2016-05-03 18:42 ` Leon Romanovsky [not found] ` <20160503184218.GC29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-04 4:41 ` Leon Romanovsky [not found] ` <20160504044107.GE29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-04 12:36 ` Dennis Dalessandro [not found] ` <20160504123621.GC10916-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2016-05-04 18:15 ` Jason Gunthorpe [not found] ` <20160504181509.GA20488-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-05 6:38 ` Leon Romanovsky [not found] ` <20160505063834.GH29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-05 12:00 ` Dennis Dalessandro [not found] ` <20160505120033.GA23895-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org> 2016-05-05 12:39 ` Leon Romanovsky [not found] ` <20160505123932.GK29160-2ukJVAZIZ/Y@public.gmane.org> 2016-05-05 18:08 ` Jason Gunthorpe [not found] ` <20160505180843.GA5957-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-05 18:36 ` Doug Ledford [not found] ` <b7f1735c-5362-514e-268a-49a09b316a88-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-05-05 18:46 ` Jason Gunthorpe 2016-05-06 4:36 ` Leon Romanovsky 2016-05-04 12:20 ` Dennis Dalessandro 2016-05-03 17:31 ` Jason Gunthorpe [not found] ` <20160503173130.GA1921-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-04 12:17 ` Dennis Dalessandro 2016-05-05 18:57 ` Doug Ledford [not found] ` <72645a3b-5945-419a-d7af-1c065080e415-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-05-05 19:20 ` Jason Gunthorpe [not found] ` <20160505192024.GA17249-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-05 19:32 ` Doug Ledford [not found] ` <5334ab9c-428a-547f-b80a-e0bee3f85449-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-05-05 20:38 ` Jason Gunthorpe [not found] ` <20160505203858.GA18611-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-05-05 23:58 ` Ira Weiny
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.