All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
Cc: linux-pci@vger.kernel.org, linux@yadro.com
Subject: Re: [PATCH v2] PCI: pciehp: Fix re-enabling the slot marked for safe removal
Date: Tue, 12 Mar 2019 13:20:14 +0100	[thread overview]
Message-ID: <20190312122014.jrwgnqpuiywb7xsd@wunner.de> (raw)
In-Reply-To: <20190312120548.31875-1-s.miroshnichenko@yadro.com>

On Tue, Mar 12, 2019 at 03:05:48PM +0300, Sergey Miroshnichenko wrote:
> During the safe removal procedure, a Data Link Layer State Changed event
> may occur after pciehp_power_off_slot(), and it is handled when the slot is
> already set to OFF_STATE. This results in re-enabling the device and makes
> it impossible to actually safely remove it.
> 
> Clear out the Presence Detect Changed and Data Link Layer State Changed
> events when the disabled slot has settled down.
> 
> It is still possible to re-enable the device if it remains in the slot
> after pressing the Attention Button by pressing it again.
> 
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>

Reviewed-by: Lukas Wunner <lukas@wunner.de>

Did this work correctly before v4.19 or is it a regression caused by
d331710ea78f ("PCI: pciehp: Become resilient to missed events")?
Either way, it seems reasonable to backport it to stable and
v4.19 is the earliest that it should apply cleanly to, so:

Cc: stable@vger.kernel.org # v4.19+

Thanks a lot for catching this, I wasn't able to test the code paths
which are only exercised if a power controller is present (because
Thunderbolt doesn't have one).

Lukas

> ---
>  drivers/pci/hotplug/pciehp_ctrl.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
> index 3f3df4c29f6e..905282a8ddaa 100644
> --- a/drivers/pci/hotplug/pciehp_ctrl.c
> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
> @@ -115,6 +115,10 @@ static void remove_board(struct controller *ctrl, bool safe_removal)
>  		 * removed from the slot/adapter.
>  		 */
>  		msleep(1000);
> +
> +		/* Ignore link or presence changes caused by power off */
> +		atomic_and(~(PCI_EXP_SLTSTA_DLLSC | PCI_EXP_SLTSTA_PDC),
> +			   &ctrl->pending_events);
>  	}
>  
>  	/* turn off Green LED */
> -- 
> 2.20.1

  reply	other threads:[~2019-03-12 12:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-12 12:05 [PATCH v2] PCI: pciehp: Fix re-enabling the slot marked for safe removal Sergey Miroshnichenko
2019-03-12 12:20 ` Lukas Wunner [this message]
2019-03-13 13:42   ` Sergey Miroshnichenko
2019-04-04 19:44 ` Bjorn Helgaas
2019-04-08 19:55 ` Bjorn Helgaas
2019-04-09 23:41   ` Micah Parrish
2019-04-10  8:33     ` Lukas Wunner
2019-04-10 21:22     ` Bjorn Helgaas
2019-04-10  8:26   ` Lukas Wunner

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=20190312122014.jrwgnqpuiywb7xsd@wunner.de \
    --to=lukas@wunner.de \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@yadro.com \
    --cc=s.miroshnichenko@yadro.com \
    /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: link
Be 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.