On 2019/5/15 19:49, Jason Gunthorpe wrote: > On Wed, May 15, 2019 at 05:38:02PM +0800, Liuyixian (Eason) wrote: >> >> >> On 2019/5/9 18:50, Liuyixian (Eason) wrote: >>> >>> >>> On 2019/5/2 21:03, Jason Gunthorpe wrote: >>>> On Tue, Apr 30, 2019 at 04:27:41PM +0800, Liuyixian (Eason) wrote: >>>>> >>>>> >>>>> On 2019/4/27 5:05, Leon Romanovsky wrote: >>>>>> On Fri, Apr 26, 2019 at 11:36:56AM -0300, Jason Gunthorpe wrote: >>>>>>> On Fri, Apr 26, 2019 at 06:12:11PM +0800, Liuyixian (Eason) wrote: >>>>>>> >>>>>>>> However, I have talked with our chip team about function clear >>>>>>>> functionality. We think it is necessary to inform the chip to >>>>>>>> perform the outstanding task and some cleanup work and restore >>>>>>>> hardware resources in time when rmmod ko. Otherwise, it is >>>>>>>> dangerous to reuse the hardware as it can not guarantee those >>>>>>>> work can be done well without the notification from our driver. >>>>>>> >>>>>>> If it is dangerous to reuse the hardware then you have to do this >>>>>>> cleanup on device startup, not on device removal. >>>>>> >>>>>> Right, I can think about gazillion ways to brick such HW. >>>>>> The simplest way will be to call SysRq during RDMA traffic >>>>>> and no cleanup function will be called in such case. >>>>>> >>>>>> Thanks >>>>> >>>>> Hi Jason and Leon, >>>>> >>>>> As hip08 is a fake pcie device, we could not disassociate and stop the hardware access >>>>> through the chain break mechanism as a real pcie device. Alternatively, function clear >>>>> is used as a notification to the hardware to stop accessing and ensure to not read or >>>>> write DDR later. That is, the role of function clear to hip08 is similar as the chain >>>>> break to pcie device. >>>> >>>> What? This hardware is broken and doesn't respond to the bus master >>>> enable bit in the PCI config space?? >>>> >>> Hi Jason, >>> >>> Sorry to reply to you late. >>> >>> Yes, the bus master enable bit should be set by a pcie device when startup and removal. >>> The hns (nic) module use it as well. However, we couldn't use/operate this bit in hip08 >>> as it shares the PF(physical function) with nic. Therefore, we need function clear to >>> notify the hardware to do the cleanup thing and cache write back. >>> >>> Thanks. >>> >> >> Hi Jason and Leon, >> >> Do you have further more suggestions? > > The approach seems completely wrong to me - no other driver is doing > something so sketchy. > > You need to explain why hns is so special > > Jason Thanks, Jason. I will revisit the solution and feedback soon.
