linux-kernel - Re: [PATCH] PCI: Remove duplicate pci_disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5179FC4A.4080000@huawei.com>
Date:	Fri, 26 Apr 2013 12:02:18 +0800
From:	Yijing Wang <wangyijing@...wei.com>
To:	Yinghai Lu <yinghai@...nel.org>
CC:	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Huang Ying <ying.huang@...el.com>,
	David Bulkow <David.Bulkow@...atus.com>,
	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>,
	<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	Jiang Liu <jiang.liu@...wei.com>
Subject: Re: [PATCH] PCI: Remove duplicate pci_disable_device for pcie port

Hi Yinghai,
   We should not remove this additional pci_disable_device().
Because we enable pcie port device twice before. The first is pci_enable_brides(),
in x86, it was called in pci_assign_unassigned_resources(). The second in pcie_port_device_register().
So we should call pci_disable_device() twice for pci_dev->enable_cnt balance.

But there is still a problem here. If we unbind a pcie port device pcie port driver, we can not
use its child devices again, because this pcie port device was disabled absolutely.

So I think we should move the second pci_disable_device() to remove.c.

I sent this patch to Bjorn and following is Bjorn reply
"And it's not clear to me whether unbinding the
pcie port driver should disable the bridge at all.  I think one could
argue that the bridge should remain functional even if the driver is
unloaded, because the PCI core *enables* the bridge even if the driver
is never loaded."

Yinghai, how do you think about this issue?



On 2013/4/26 9:47, Yinghai Lu wrote:
> During chasing one PCI xHCI hotplug problem, David Bulkow found
> 
> 	static void pcie_portdrv_remove(struct pci_dev *dev)
> 	{
>         	pcie_port_device_remove(dev);
> 	        pci_disable_device(dev);
> 	}
> and
> 	void pcie_port_device_remove(struct pci_dev *dev)
> 	{
>         	device_for_each_child(&dev->dev, NULL, remove_iter);
> 	        cleanup_service_irqs(dev);
> 	        pci_disable_device(dev);
> 	}
> 
> that extra pci_disable_device in pcie_port_device_remove() was added by
> | commit dc5351784eb36f1fec4efa88e01581be72c0b711
> | Author: Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>
> | Date:   Wed Nov 25 21:04:00 2009 +0900
> |
> |    PCI: portdrv: cleanup service irqs initialization
> 
> so pci_dsiable_device is called two times.
> 
> We should remove extra one in pcie_portdrv_remove.
> 
> Reported-by: David Bulkow <David.Bulkow@...atus.com>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> 
> ---
>  drivers/pci/pcie/portdrv_pci.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> Index: linux-2.6/drivers/pci/pcie/portdrv_pci.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pcie/portdrv_pci.c
> +++ linux-2.6/drivers/pci/pcie/portdrv_pci.c
> @@ -223,7 +223,6 @@ static int pcie_portdrv_probe(struct pci
>  static void pcie_portdrv_remove(struct pci_dev *dev)
>  {
>  	pcie_port_device_remove(dev);
> -	pci_disable_device(dev);
>  }
>  
>  static int error_detected_iter(struct device *device, void *data)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> .
> 


-- 
Thanks!
Yijing

View attachment "0001-PCI-move-second-pci_disable_device-into-pci_stop_bus.patch" of type "text/x-patch" (1662 bytes)