On Mon, Oct 24, 2016 at 10:51:13PM -0500, Bjorn Helgaas wrote: >On Tue, Oct 25, 2016 at 12:47:28PM +1100, Gavin Shan wrote: >> On Mon, Oct 24, 2016 at 09:03:16AM -0500, Bjorn Helgaas wrote: >> >On Mon, Oct 24, 2016 at 10:28:02AM +1100, Gavin Shan wrote: >> >> On Fri, Oct 21, 2016 at 11:50:34AM -0500, Bjorn Helgaas wrote: >> >> >On Fri, Sep 30, 2016 at 09:47:50AM +1000, Gavin Shan wrote:
.../... > >That specific case (pci_enable_device() followed by >pci_update_resource()) should *not* work. pci_enable_device() is >normally called by a driver's .probe() method, and after we call a >.probe() method, the PCI core shouldn't touch the device at all >because there's no means of mutual exclusion between the driver and >the PCI core. > >I think pci_update_resource() should only be called in situations >where the caller already knows that nobody is using the device. For >regular PCI BARs, that doesn't necessarily mean PCI_COMMAND_MEMORY is >turned off, because firmware leaves PCI_COMMAND_MEMORY enabled for >many devices, even though nobody is using them. > >Anyway, I think that's a project for another day. That's too much to >tackle for the limited problem you're trying to solve. > Bjorn, it's all about discussion. Please take your time and reply when you have bandwidth. Well, some drivers break the order and expects the relaxed order to work. One example is drivers/char/agp/efficeon-agp.c::agp_efficeon_probe(). I didn't check all usage cases. I think it's hard for user, who calls pci_update_resource(), to know that nobody is using the devcie (limited to memory BARs as we concern). The memory write is usually non-posted transaction and it can be on the way to target device when pci_update_resource() is called. So which one tranaction will be completed first (disabling memory decoding and memory write). I guess it can happen even with the mutual exclusion, especially on SMP system. Yes, the situation is worse without the synchronization. .../... >> >> Yeah, it would be the solution to have. If you agree, I will post >> updatd version according to this: Clearing PCI_SRIOV_CTRL_MSE when >> updating IOV BARs. The bit won't be touched if pdev->mmio_always_on >> is true. > >I think you should ignore pdev->mmio_always_on for IOV BARs. >mmio_always_on is basically a workaround for devices that either don't >follow the spec or where we didn't completely understand the problem. >I don't think there's any reason to set mmio_always_on for SR-IOV >devices. > Agree, thanks for the comments again. I will post updated version shortly. Thanks, Gavin
