| 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
| ||
|
Message-Id: <200711181323.52679.rjw@sisk.pl>
Date: Sun, 18 Nov 2007 13:23:51 +0100
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Mark Lord <lkml@....ca>
Cc: kristen.c.accardi@...el.com,
Linux Kernel <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Theodore Ts'o <tytso@....edu>, greg@...ah.com,
pcihpd-discuss@...ts.sourceforge.net
Subject: Re: [PATCH] Fix PCIe double initialization bug
On Sunday, 18 of November 2007, Mark Lord wrote:
> pciehp_fix_double_init_bug.patch:
>
> Earlier patches to split out the hardware init for PCIe hotplug
> resulted in some one-time initializations being redone on every
> resume cycle. Eg. irq/polling initialization.
>
> This patch splits the hardware init into two parts,
> and separates the one-time initializations from those
> so that they only ever get done once, as intended.
>
> Signed-off-by: Mark Lord <mlord@...ox.com>
Which kernel is it against?
> ---
>
> This patch is for -mm and for Kristen's queue. Not for 2.6.24.
>
> drivers/pci/hotplug/pciehp.h | 2
> drivers/pci/hotplug/pciehp_core.c | 2
> drivers/pci/hotplug/pciehp_hpc.c | 119 +++++++++++++++-------------
> 3 files changed, 69 insertions(+), 54 deletions(-)
>
> --- linux/drivers/pci/hotplug/pciehp.h.orig 2007-11-13 23:57:09.000000000 -0500
> +++ linux/drivers/pci/hotplug/pciehp.h 2007-11-17 19:10:01.000000000 -0500
> @@ -163,7 +163,7 @@
> int pcie_init(struct controller *ctrl, struct pcie_device *dev);
> int pciehp_enable_slot(struct slot *p_slot);
> int pciehp_disable_slot(struct slot *p_slot);
> -int pcie_init_hardware(struct controller *ctrl, struct pcie_device *dev);
> +int pcie_init_hardware_part2(struct controller *ctrl, struct pcie_device *dev);
>
> static inline struct slot *pciehp_find_slot(struct controller *ctrl, u8 device)
> {
> --- linux/drivers/pci/hotplug/pciehp_core.c.orig 2007-11-13 23:57:09.000000000 -0500
> +++ linux/drivers/pci/hotplug/pciehp_core.c 2007-11-17 19:09:43.000000000 -0500
> @@ -521,7 +521,7 @@
> u8 status;
>
> /* reinitialize the chipset's event detection logic */
> - pcie_init_hardware(ctrl, dev);
> + pcie_init_hardware_part2(ctrl, dev);
>
> t_slot = pciehp_find_slot(ctrl, ctrl->slot_device_offset);
>
> --- linux/drivers/pci/hotplug/pciehp_hpc.c.orig 2007-11-13 23:57:09.000000000 -0500
> +++ linux/drivers/pci/hotplug/pciehp_hpc.c 2007-11-17 19:13:49.000000000 -0500
> @@ -1067,28 +1067,25 @@
> }
> #endif
>
> -int pcie_init_hardware(struct controller *ctrl, struct pcie_device *dev)
> +static int pcie_init_hardware_part1(struct controller *ctrl,
> + struct pcie_device *dev)
> {
> int rc;
> u16 temp_word;
> - u16 intr_enable = 0;
> u32 slot_cap;
> u16 slot_status;
> - struct pci_dev *pdev;
> -
> - pdev = dev->port;
>
> rc = pciehp_readl(ctrl, SLOTCAP, &slot_cap);
> if (rc) {
> err("%s: Cannot read SLOTCAP register\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + return -1;
> }
>
> /* Mask Hot-plug Interrupt Enable */
> rc = pciehp_readw(ctrl, SLOTCTRL, &temp_word);
> if (rc) {
> err("%s: Cannot read SLOTCTRL register\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + return -1;
> }
>
> dbg("%s: SLOTCTRL %x value read %x\n",
> @@ -1099,62 +1096,46 @@
> rc = pciehp_writew(ctrl, SLOTCTRL, temp_word);
> if (rc) {
> err("%s: Cannot write to SLOTCTRL register\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + return -1;
> }
>
> rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status);
> if (rc) {
> err("%s: Cannot read SLOTSTATUS register\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + return -1;
> }
>
> temp_word = 0x1F; /* Clear all events */
> rc = pciehp_writew(ctrl, SLOTSTATUS, temp_word);
> if (rc) {
> err("%s: Cannot write to SLOTSTATUS register\n", __FUNCTION__);
> - goto abort_free_ctlr;
> + return -1;
> }
> + return 0;
> +}
>
> - if (pciehp_poll_mode) {
> - /* Install interrupt polling timer. Start with 10 sec delay */
> - init_timer(&ctrl->poll_timer);
> - start_int_poll_timer(ctrl, 10);
> - } else {
> - /* Installs the interrupt handler */
> - rc = request_irq(ctrl->pci_dev->irq, pcie_isr, IRQF_SHARED,
> - MY_NAME, (void *)ctrl);
> - dbg("%s: request_irq %d for hpc%d (returns %d)\n",
> - __FUNCTION__, ctrl->pci_dev->irq,
> - atomic_read(&pciehp_num_controllers), rc);
> - if (rc) {
> - err("Can't get irq %d for the hotplug controller\n",
> - ctrl->pci_dev->irq);
> - goto abort_free_ctlr;
> - }
> - }
> - dbg("pciehp ctrl b:d:f:irq=0x%x:%x:%x:%x\n", pdev->bus->number,
> - PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), dev->irq);
> -
> - /*
> - * If this is the first controller to be initialized,
> - * initialize the pciehp work queue
> - */
> - if (atomic_add_return(1, &pciehp_num_controllers) == 1) {
> - pciehp_wq = create_singlethread_workqueue("pciehpd");
> - if (!pciehp_wq) {
> - rc = -ENOMEM;
> - goto abort_free_irq;
> - }
> - }
> +int pcie_init_hardware_part2(struct controller *ctrl, struct pcie_device *dev)
> +{
> + int rc;
> + u16 temp_word;
> + u16 intr_enable = 0;
> + u32 slot_cap;
> + u16 slot_status;
>
> rc = pciehp_readw(ctrl, SLOTCTRL, &temp_word);
> if (rc) {
> err("%s: Cannot read SLOTCTRL register\n", __FUNCTION__);
> - goto abort_free_irq;
> + goto abort;
> }
>
> intr_enable = intr_enable | PRSN_DETECT_ENABLE;
>
> + rc = pciehp_readl(ctrl, SLOTCAP, &slot_cap);
> + if (rc) {
> + err("%s: Cannot read SLOTCAP register\n", __FUNCTION__);
> + goto abort;
> + }
> +
> if (ATTN_BUTTN(slot_cap))
> intr_enable = intr_enable | ATTN_BUTTN_ENABLE;
>
> @@ -1179,7 +1160,7 @@
> rc = pciehp_writew(ctrl, SLOTCTRL, temp_word);
> if (rc) {
> err("%s: Cannot write to SLOTCTRL register\n", __FUNCTION__);
> - goto abort_free_irq;
> + goto abort;
> }
> rc = pciehp_readw(ctrl, SLOTSTATUS, &slot_status);
> if (rc) {
> @@ -1214,14 +1195,7 @@
> }
> if (rc)
> err("%s : disabling interrupts failed\n", __FUNCTION__);
> -
> -abort_free_irq:
> - if (pciehp_poll_mode)
> - del_timer_sync(&ctrl->poll_timer);
> - else
> - free_irq(ctrl->pci_dev->irq, ctrl);
> -
> -abort_free_ctlr:
> +abort:
> return -1;
> }
>
> @@ -1318,11 +1292,52 @@
> ctrl->first_slot = slot_cap >> 19;
> ctrl->ctrlcap = slot_cap & 0x0000007f;
>
> - rc = pcie_init_hardware(ctrl, dev);
> + rc = pcie_init_hardware_part1(ctrl, dev);
> + if (rc)
> + goto abort;
> +
> + if (pciehp_poll_mode) {
> + /* Install interrupt polling timer. Start with 10 sec delay */
> + init_timer(&ctrl->poll_timer);
> + start_int_poll_timer(ctrl, 10);
> + } else {
> + /* Installs the interrupt handler */
> + rc = request_irq(ctrl->pci_dev->irq, pcie_isr, IRQF_SHARED,
> + MY_NAME, (void *)ctrl);
> + dbg("%s: request_irq %d for hpc%d (returns %d)\n",
> + __FUNCTION__, ctrl->pci_dev->irq,
> + atomic_read(&pciehp_num_controllers), rc);
> + if (rc) {
> + err("Can't get irq %d for the hotplug controller\n",
> + ctrl->pci_dev->irq);
> + goto abort;
> + }
> + }
> + dbg("pciehp ctrl b:d:f:irq=0x%x:%x:%x:%x\n", pdev->bus->number,
> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), dev->irq);
> +
> + /*
> + * If this is the first controller to be initialized,
> + * initialize the pciehp work queue
> + */
> + if (atomic_add_return(1, &pciehp_num_controllers) == 1) {
> + pciehp_wq = create_singlethread_workqueue("pciehpd");
> + if (!pciehp_wq) {
> + rc = -ENOMEM;
> + goto abort_free_irq;
> + }
> + }
> +
> + rc = pcie_init_hardware_part2(ctrl, dev);
> if (rc == 0) {
> ctrl->hpc_ops = &pciehp_hpc_ops;
> return 0;
> }
> +abort_free_irq:
> + if (pciehp_poll_mode)
> + del_timer_sync(&ctrl->poll_timer);
> + else
> + free_irq(ctrl->pci_dev->irq, ctrl);
> abort:
> return -1;
> }
> -
> 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/
>
>
--
"Premature optimization is the root of all evil." - Donald Knuth
-
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/
Powered by blists - more mailing lists