diff mbox series

[04/10] PCI: pciehp: Do not handle events if interrupts are masked

Message ID 20180906155020.51700-5-mika.westerberg@linux.intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series PCI: Allow D3cold for PCIe hierarchies | expand

Commit Message

Mika Westerberg Sept. 6, 2018, 3:50 p.m. UTC
PCIe native hotplug shares MSI vector with native PME so the interrupt
handler might get called even the hotplug interrupt is masked. In that
case we should not handle any events because the interrupt was not meant
for us. Modify the PCIe hotplug interrupt handler to check this
accordingly and bail out if it finds out that the interrupt was not
about hotplug.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/hotplug/pciehp_hpc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Lukas Wunner Sept. 6, 2018, 4:04 p.m. UTC | #1
On Thu, Sep 06, 2018 at 06:50:14PM +0300, Mika Westerberg wrote:
> PCIe native hotplug shares MSI vector with native PME so the interrupt
> handler might get called even the hotplug interrupt is masked. In that
> case we should not handle any events because the interrupt was not meant
> for us. Modify the PCIe hotplug interrupt handler to check this
> accordingly and bail out if it finds out that the interrupt was not
> about hotplug.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Lukas Wunner <lukas@wunner.de>
Keith Busch Sept. 7, 2018, 10:45 p.m. UTC | #2
On Thu, Sep 06, 2018 at 06:50:14PM +0300, Mika Westerberg wrote:
> PCIe native hotplug shares MSI vector with native PME so the interrupt
> handler might get called even the hotplug interrupt is masked. In that
> case we should not handle any events because the interrupt was not meant
> for us. Modify the PCIe hotplug interrupt handler to check this
> accordingly and bail out if it finds out that the interrupt was not
> about hotplug.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/hotplug/pciehp_hpc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 2249c4d06efd..19ed13d44b8f 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -533,9 +533,11 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
>  	u16 status, events;
>  
>  	/*
> -	 * Interrupts only occur in D3hot or shallower (PCIe r4.0, sec 6.7.3.4).
> +	 * Interrupts only occur in D3hot or shallower and only if enabled
> +	 * in the Slot Control register (PCIe r4.0, sec 6.7.3.4).
>  	 */
> -	if (pdev->current_state == PCI_D3cold)
> +	if (pdev->current_state == PCI_D3cold ||
> +	    !(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE))
>  		return IRQ_NONE;
>  
>  	/*

Isn't this going to break pciehp_poll_mode?
Lukas Wunner Sept. 8, 2018, 6:16 a.m. UTC | #3
On Fri, Sep 07, 2018 at 04:45:46PM -0600, Keith Busch wrote:
> On Thu, Sep 06, 2018 at 06:50:14PM +0300, Mika Westerberg wrote:
> > PCIe native hotplug shares MSI vector with native PME so the interrupt
> > handler might get called even the hotplug interrupt is masked. In that
> > case we should not handle any events because the interrupt was not meant
> > for us. Modify the PCIe hotplug interrupt handler to check this
> > accordingly and bail out if it finds out that the interrupt was not
> > about hotplug.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/pci/hotplug/pciehp_hpc.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> > index 2249c4d06efd..19ed13d44b8f 100644
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -533,9 +533,11 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
> >  	u16 status, events;
> >  
> >  	/*
> > -	 * Interrupts only occur in D3hot or shallower (PCIe r4.0, sec 6.7.3.4).
> > +	 * Interrupts only occur in D3hot or shallower and only if enabled
> > +	 * in the Slot Control register (PCIe r4.0, sec 6.7.3.4).
> >  	 */
> > -	if (pdev->current_state == PCI_D3cold)
> > +	if (pdev->current_state == PCI_D3cold ||
> > +	    !(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE))
> >  		return IRQ_NONE;
> >  
> >  	/*
> 
> Isn't this going to break pciehp_poll_mode?

Excellent catch, you're right, it needs to be constrained to
!pciehp_poll_mode.
Mika Westerberg Sept. 10, 2018, 7:17 a.m. UTC | #4
On Sat, Sep 08, 2018 at 08:16:17AM +0200, Lukas Wunner wrote:
> On Fri, Sep 07, 2018 at 04:45:46PM -0600, Keith Busch wrote:
> > On Thu, Sep 06, 2018 at 06:50:14PM +0300, Mika Westerberg wrote:
> > > PCIe native hotplug shares MSI vector with native PME so the interrupt
> > > handler might get called even the hotplug interrupt is masked. In that
> > > case we should not handle any events because the interrupt was not meant
> > > for us. Modify the PCIe hotplug interrupt handler to check this
> > > accordingly and bail out if it finds out that the interrupt was not
> > > about hotplug.
> > > 
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > ---
> > >  drivers/pci/hotplug/pciehp_hpc.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> > > index 2249c4d06efd..19ed13d44b8f 100644
> > > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > > @@ -533,9 +533,11 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
> > >  	u16 status, events;
> > >  
> > >  	/*
> > > -	 * Interrupts only occur in D3hot or shallower (PCIe r4.0, sec 6.7.3.4).
> > > +	 * Interrupts only occur in D3hot or shallower and only if enabled
> > > +	 * in the Slot Control register (PCIe r4.0, sec 6.7.3.4).
> > >  	 */
> > > -	if (pdev->current_state == PCI_D3cold)
> > > +	if (pdev->current_state == PCI_D3cold ||
> > > +	    !(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE))
> > >  		return IRQ_NONE;
> > >  
> > >  	/*
> > 
> > Isn't this going to break pciehp_poll_mode?
> 
> Excellent catch, you're right, it needs to be constrained to
> !pciehp_poll_mode.

OK, I'll add the check to v2.
diff mbox series

Patch

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 2249c4d06efd..19ed13d44b8f 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -533,9 +533,11 @@  static irqreturn_t pciehp_isr(int irq, void *dev_id)
 	u16 status, events;
 
 	/*
-	 * Interrupts only occur in D3hot or shallower (PCIe r4.0, sec 6.7.3.4).
+	 * Interrupts only occur in D3hot or shallower and only if enabled
+	 * in the Slot Control register (PCIe r4.0, sec 6.7.3.4).
 	 */
-	if (pdev->current_state == PCI_D3cold)
+	if (pdev->current_state == PCI_D3cold ||
+	    !(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE))
 		return IRQ_NONE;
 
 	/*