On Thursday 25 September 2008 00:38:55 Zhao, Yu wrote: > On Thursday, September 25, 2008 12:15 AM, Matthew Wilcox wrote: > >On Wed, Sep 24, 2008 at 10:39:12PM +0800, Zhao, Yu wrote: > >> >Let's talk about what else we might need to do here. The spec says: > >> > > >> > Any outstanding INTx interrupt asserted by the Function must be > >> > de-asserted by sending the corresponding Deassert_INTx Message prior > >> > to starting the FLR. > >> > >> I believe this is requirement of the hardware rather than the > >> software. PCIe endpoint using legacy INTx should deassert the interrupt > >> when either the Interrupt Disable bit in the Command register is set > >> or FLR bit is triggered. Software needs not to worry about acking the > >> asserted INTx when doing these operations. > > > >Oh, that makes sense. Sorry, for some reason I got confused and thought > >the Deassert_INTx was sent by the root port back to the device. It > >isn't, of course, it's sent by the device. You're right, we don't need > >to do anything to support this case. > > > >I believe my comment about shared interrupts still holds true, though, > >so we still need to do _something_ for interrupts -- either > >disable_irq() or make sure no interrupt handler currently registered > >will try to touch this device. > > Agree. > > >> >So we need to make sure there are no pending interrupts for this > >> >function before issuing the FLR. Is that the responsibility of the > >> >caller? We should say so if it is. If we're going to do it in this > >> >function, we should use disable_irq() to be sure that the interrupt > >> >isn't currently executing on some other CPU (having first checked that > >> >the device isn't using MSI-X, because if it is, we don't use the > >> >interrupt in pdev->irq). If we're using a shared interrupt, other > >> > users are going to suffer. > >> > > >> >I still don't really understand how this flr fits in with every other > >> >part of the system. Is the device supposed to be quiesced before this > >> >is called? If so, a lot of what it does is pointless. Or is it > >> >supposed to do the quiescing? If so, it's missing some bits. > >> > >> Though the original purpose of this function is for KVM, having an API > >> to reset device using standardized methods obviously benefits the driver > >> developers a lot -- they don't have to write their own ones or use > >> device specific mechanisms to reset the hardware at initialization or > >> error recovery stage if devices support one of these methods. The > >> standardized methods include: FLR, PM D3-D0 and secondary bus reset, and > >> all of them should be implemented in this function. Only > >> setting/clearing FLR bit is almost useless since it's neither supported > >> by PCI device nor most of PCIe devices. The code above also should call > >> pci_set_pcie_reset_state too, to check if PCIe conventional reset is > >> supported. > > > >I'm not quite sure what you mean there, but if we've asked for a function > >to be reset, we clearly don't want to reset the entire device. Better to > >fail, and return an error to the caller, that way they can decide whether > >to try to reset the entire device or not. > > > >I'd quite like to see this tied into the Linux PCI error handling > >framework as part of the steps involved in recovery. It would be nice > >(if the problem is only with one function of a device) to only reset > >that one function, not all of them. > > > >I'm not quite sure how PCI error recovery interacts with v12n. It seems > >more than slightly rude to reset a device that includes a function in > >another machine's address space. Anyway, I've fairly aggressively not > >been paying attention to v12n and don't intend to start now. > > Sorry I didn't make my point clear. My suggestion is that we could have an > API to do PCI device reset (or per-function reset if the device is > multi-function device), so the driver developers could use this API rather > than write same code in their drivers, when they want to reset PCI devices > that support standardized reset mechanism. This API should support all > other standardized methods besides FLR, so it could be much useful > comparing to the pcie_do_flr. > > Any thoughts or comments on this? Well, I think this target is specific to the virtualization, what we need is do a device reset to ensure that the state didn't retain after moved to another domain. But I think Matthew is thinking about another usage of flr, the error recovery. From this sight of view, we needn't/can't do such aggressive assumption. Currently, we'd better do it step by step? If we need such a API, and I think it would a enhanced one including FLR, not replace FLR's function? -- regards Yang, Sheng > > >-- > >Matthew Wilcox Intel Open Source Technology > > Centre "Bill, look, we understand that you're interested in selling us > > this operating system, but compare it to ours. We can't possibly take > > such a retrograde step." -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
