From: Sinan Kaya <okaya@codeaurora.org> To: Bjorn Helgaas <helgaas@kernel.org> Cc: linux-pci@vger.kernel.org, timur@codeaurora.org, ryan@finnie.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>, "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Thomas Gleixner <tglx@linutronix.de>, Kate Stewart <kstewart@linuxfoundation.org>, Frederick Lawler <fred@fredlawl.com>, Dongdong Liu <liudongdong3@huawei.com>, Mika Westerberg <mika.westerberg@linux.intel.com>, open list <linux-kernel@vger.kernel.org>, Don Brace <don.brace@microsemi.com>, esc.storagedev@microsemi.com, linux-scsi@vger.kernel.org Subject: Re: [PATCH V2] PCI/portdrv: do not disable device on reboot/shutdown Date: Thu, 24 May 2018 07:43:05 -0400 [thread overview] Message-ID: <82656e20-e821-1944-3399-1667ceb27719@codeaurora.org> (raw) In-Reply-To: <61f70fd6-52fd-da07-ce73-303f95132131@codeaurora.org> On 5/23/2018 6:57 PM, Sinan Kaya wrote: >> The crash seems to indicate that the hpsa device attempted a DMA after >> we cleared the Root Port's PCI_COMMAND_MASTER, which means >> hpsa_shutdown() didn't stop DMA from the device (it looks like *most* >> shutdown methods don't disable device DMA, so it's in good company). > All drivers are expected to shutdown DMA and interrupts in their shutdown() > routines. They can skip removing threads, data structures etc. but DMA and > interrupt disabling are required. This is the difference between shutdown() > and remove() callbacks. I found this note yesterday to see why we are not disabling the devices in the PCI core itself. pci_device_remove() /* * We would love to complain here if pci_dev->is_enabled is set, that * the driver should have called pci_disable_device(), but the * unfortunate fact is there are too many odd BIOS and bridge setups * that don't like drivers doing that all of the time. * Oh well, we can dream of sane hardware when we sleep, no matter how * horrible the crap we have to deal with is when we are awake... */ Ryan, can you discard the previous patch and test this one instead? remove() path in hpsa driver seems to call pci_disable_device() via hpsa_remove_one() hpsa_free_pci_init() but nothing on the shutdown path. diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 4ed3d26..3823f04 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -8651,6 +8651,7 @@ static void hpsa_shutdown(struct pci_dev *pdev) h->access.set_intr_mask(h, HPSA_INTR_OFF); hpsa_free_irqs(h); /* init_one 4 */ hpsa_disable_interrupt_mode(h); /* pci_init 2 */ + pci_disable_device(h->pdev); } -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
WARNING: multiple messages have this Message-ID (diff)
From: okaya@codeaurora.org (Sinan Kaya) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH V2] PCI/portdrv: do not disable device on reboot/shutdown Date: Thu, 24 May 2018 07:43:05 -0400 [thread overview] Message-ID: <82656e20-e821-1944-3399-1667ceb27719@codeaurora.org> (raw) In-Reply-To: <61f70fd6-52fd-da07-ce73-303f95132131@codeaurora.org> On 5/23/2018 6:57 PM, Sinan Kaya wrote: >> The crash seems to indicate that the hpsa device attempted a DMA after >> we cleared the Root Port's PCI_COMMAND_MASTER, which means >> hpsa_shutdown() didn't stop DMA from the device (it looks like *most* >> shutdown methods don't disable device DMA, so it's in good company). > All drivers are expected to shutdown DMA and interrupts in their shutdown() > routines. They can skip removing threads, data structures etc. but DMA and > interrupt disabling are required. This is the difference between shutdown() > and remove() callbacks. I found this note yesterday to see why we are not disabling the devices in the PCI core itself. pci_device_remove() /* * We would love to complain here if pci_dev->is_enabled is set, that * the driver should have called pci_disable_device(), but the * unfortunate fact is there are too many odd BIOS and bridge setups * that don't like drivers doing that all of the time. * Oh well, we can dream of sane hardware when we sleep, no matter how * horrible the crap we have to deal with is when we are awake... */ Ryan, can you discard the previous patch and test this one instead? remove() path in hpsa driver seems to call pci_disable_device() via hpsa_remove_one() hpsa_free_pci_init() but nothing on the shutdown path. diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 4ed3d26..3823f04 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -8651,6 +8651,7 @@ static void hpsa_shutdown(struct pci_dev *pdev) h->access.set_intr_mask(h, HPSA_INTR_OFF); hpsa_free_irqs(h); /* init_one 4 */ hpsa_disable_interrupt_mode(h); /* pci_init 2 */ + pci_disable_device(h->pdev); } -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
next prev parent reply other threads:[~2018-05-24 11:43 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-23 2:44 [PATCH V2] PCI/portdrv: do not disable device on reboot/shutdown Sinan Kaya 2018-05-23 2:44 ` Sinan Kaya 2018-05-23 2:44 ` Sinan Kaya 2018-05-23 21:32 ` Bjorn Helgaas 2018-05-23 21:32 ` Bjorn Helgaas 2018-05-23 21:32 ` Bjorn Helgaas 2018-05-23 22:57 ` Sinan Kaya 2018-05-23 22:57 ` Sinan Kaya 2018-05-24 11:43 ` Sinan Kaya [this message] 2018-05-24 11:43 ` Sinan Kaya 2018-05-24 13:07 ` Bjorn Helgaas 2018-05-24 13:07 ` Bjorn Helgaas 2018-05-24 13:07 ` Bjorn Helgaas 2018-05-24 13:35 ` okaya 2018-05-24 13:35 ` okaya at codeaurora.org 2018-05-24 13:37 ` Don Brace 2018-05-24 13:37 ` Don Brace 2018-05-24 13:37 ` Don Brace 2018-05-28 21:25 ` Sinan Kaya 2018-05-28 21:25 ` Sinan Kaya 2018-05-28 21:25 ` Sinan Kaya 2018-05-24 18:35 ` Bjorn Helgaas 2018-05-24 18:35 ` Bjorn Helgaas 2018-05-24 18:35 ` Bjorn Helgaas 2018-05-25 13:30 ` Sinan Kaya 2018-05-25 13:30 ` Sinan Kaya 2018-05-25 22:10 ` Bjorn Helgaas 2018-05-25 22:10 ` Bjorn Helgaas 2018-05-25 22:10 ` Bjorn Helgaas 2018-05-25 22:34 ` okaya 2018-05-25 22:34 ` okaya at codeaurora.org 2018-05-30 2:41 ` Sinan Kaya 2018-05-30 2:41 ` Sinan Kaya
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=82656e20-e821-1944-3399-1667ceb27719@codeaurora.org \ --to=okaya@codeaurora.org \ --cc=bhelgaas@google.com \ --cc=don.brace@microsemi.com \ --cc=esc.storagedev@microsemi.com \ --cc=fred@fredlawl.com \ --cc=gregkh@linuxfoundation.org \ --cc=helgaas@kernel.org \ --cc=kstewart@linuxfoundation.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=liudongdong3@huawei.com \ --cc=mika.westerberg@linux.intel.com \ --cc=rafael.j.wysocki@intel.com \ --cc=ryan@finnie.org \ --cc=stable@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=timur@codeaurora.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.