Messages in this thread Patch in this message | | | Subject | Re: [RFC PATCH] PCI/portdrv: No need to call pci_disable_device() during shutdown | From | Tiezhu Yang <> | Date | Fri, 11 Sep 2020 11:19:53 +0800 |
| |
On 09/11/2020 10:35 AM, Oliver O'Halloran wrote: > On Fri, Sep 11, 2020 at 11:55 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: >> On 09/11/2020 04:21 AM, Bjorn Helgaas wrote: >>> [+cc Huacai] >>> >>> On Thu, Sep 10, 2020 at 02:41:39PM -0500, Bjorn Helgaas wrote: >>>> On Sat, Sep 05, 2020 at 04:33:26PM +0800, Tiezhu Yang wrote: >>>>> After commit 745be2e700cd ("PCIe: portdrv: call pci_disable_device >>>>> during remove") and commit cc27b735ad3a ("PCI/portdrv: Turn off PCIe >>>>> services during shutdown"), it also calls pci_disable_device() during >>>>> shutdown, this seems unnecessary, so just remove it. >>>> I would like to get rid of the portdrv completely by folding its >>>> functionality into the PCI core itself, so there would be no portdrv >>>> probe or remove. >>>> >>>> Does this solve a problem? >> Yes, sometimes it can not shutdown or reboot normally with >> pci_disable_device(). > Do you have any more details about what goes wrong here?
This issue is related with the operation "pci_command &= ~PCI_COMMAND_MASTER;" in the following function:
drivers/pci/pci.c static void do_pci_disable_device(struct pci_dev *dev) { u16 pci_command;
pci_read_config_word(dev, PCI_COMMAND, &pci_command); if (pci_command & PCI_COMMAND_MASTER) { pci_command &= ~PCI_COMMAND_MASTER; pci_write_config_word(dev, PCI_COMMAND, pci_command); }
pcibios_disable_device(dev); }
When remove "pci_command &= ~PCI_COMMAND_MASTER;", it can work well.
> Leaving > devices enabled when actually shutting down probably doesn't matter.
Yes, I think so too.
> However, .shutdown() is also used when kexec()ing into a new kernel > and we probably should be disabling devices before handing over to the > new kernel. > > Is the real issue that we're closing the bridge windows before the > endpoint drivers have had a chance to clean up?
I notice that check kexec_in_progress first before call pci_disable_devie() in pci_device_shutdown(), can we do the similar thing in pcie?
drivers/pci/pci-driver.c static void pci_device_shutdown(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); struct pci_driver *drv = pci_dev->driver;
pm_runtime_resume(dev);
if (drv && drv->shutdown) drv->shutdown(pci_dev);
/* * If this is a kexec reboot, turn off Bus Master bit on the * device to tell it to not continue to do DMA. Don't touch * devices in D3cold or unknown states. * If it is not a kexec reboot, firmware will hit the PCI * devices with big hammer and stop their DMA any way. */ if (kexec_in_progress && (pci_dev->current_state <= PCI_D3hot)) pci_clear_master(pci_dev); }
Something like this:
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 50a9522..3de1dab 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -491,7 +491,8 @@ void pcie_port_device_remove(struct pci_dev *dev) { device_for_each_child(&dev->dev, NULL, remove_iter); pci_free_irq_vectors(dev); - pci_disable_device(dev); + if (kexec_in_progress && (pci_dev->current_state <= PCI_D3hot)) + pci_disable_device(dev); }
/** > > Oliver
| |