Re: [PATCH v4] Kernel oops from pci_disable_msi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday, November 28, 2013 12:39 AM, Bjørn Erik Nilsen wrote:
> 
> Commit 904d0e7889933fb48d921c998fd1cabb3a9d6635 added

Please replace it with the following.

Commit 904d0e7889933fb48d921c998fd1cabb3a9d6635
"PCI: designware: Add irq_create_mapping()" added

Also, the subject of the commit is not good.
Usually, 'verb + objective + etc.' looks good.

Ex. PCI: designware: Remove irq_alloc_descs()

I tested this patch on Exynos platform; it works properly.
I will wait for v5 patch which addresses Marek Vasut's comments.
Thank you.

Best regards,
Jingoo Han

> irq_create_mapping which pre-allocates irq descs.
> Problem was that in assign_irq these descs were explicitly
> allocated and hence also freed, resulting in a crash.
> 
> Signed-off-by: Bjørn Erik Nilsen <ben@xxxxxxxxxxxxxx>
> ---
>  drivers/pci/host/pcie-designware.c | 50 +++++++++++++++++++++++++-------------
>  1 file changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index e33b68b..6c61abd 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -242,11 +242,18 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
>  	if (!irq)
>  		goto no_valid_irq;
> 
> +	/*
> +	 * irq_create_mapping (called from dw_pcie_host_init) pre-allocates
> +	 * descs so there is no need to allocate descs here. We can therefore
> +	 * assume that if irq_find_mapping above returns non-zero, then the
> +	 * descs are also successfully allocated. Hence no need to check the
> +	 * return value of irq_set_msi_desc_off below.
> +	 */
> +
>  	i = 0;
>  	while (i < no_irqs) {
>  		set_bit(pos0 + i, pp->msi_irq_in_use);
> -		irq_alloc_descs((irq + i), (irq + i), 1, 0);
> -		irq_set_msi_desc(irq + i, desc);
> +		irq_set_msi_desc_off(irq, i, desc);
>  		/*Enable corresponding interrupt in MSI interrupt controller */
>  		res = ((pos0 + i) / 32) * 12;
>  		bit = (pos0 + i) % 32;
> @@ -266,7 +273,7 @@ no_valid_irq:
> 
>  static void clear_irq(unsigned int irq)
>  {
> -	int res, bit, val, pos;
> +	int res, bit, val, pos, i, nvec;
>  	struct irq_desc *desc;
>  	struct msi_desc *msi;
>  	struct pcie_port *pp;
> @@ -281,18 +288,28 @@ static void clear_irq(unsigned int irq)
>  		return;
>  	}
> 
> +	/* undo what was done in assign_irq */
>  	pos = data->hwirq;
> +	nvec = 1 << msi->msi_attrib.multiple;
> 
> -	irq_free_desc(irq);
> -
> -	clear_bit(pos, pp->msi_irq_in_use);
> +	i = 0;
> +	while (i < nvec) {
> +		clear_bit(pos + i, pp->msi_irq_in_use);
> +		irq_set_msi_desc_off(irq, i, NULL);
> +		/* Disable corresponding interrupt on MSI interrupt controller */
> +		res = ((pos + i) / 32) * 12;
> +		bit = (pos + i) % 32;
> +		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
> +		val &= ~(1 << bit);
> +		dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
> +		++i;
> +	}
> 
> -	/* Disable corresponding interrupt on MSI interrupt controller */
> -	res = (pos / 32) * 12;
> -	bit = pos % 32;
> -	dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val);
> -	val &= ~(1 << bit);
> -	dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val);
> +	msi->msg.address_lo = 0;
> +	msi->msg.address_hi = 0;
> +	msi->msg.data = 0;
> +	msi->irq = 0;
> +	msi->msi_attrib.multiple = 0;
>  }
> 
>  static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
> @@ -320,12 +337,11 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
>  	if (irq < 0)
>  		return irq;
> 
> -	msg_ctr &= ~PCI_MSI_FLAGS_QSIZE;
> -	msg_ctr |= msgvec << 4;
> -	pci_write_config_word(pdev, desc->msi_attrib.pos + PCI_MSI_FLAGS,
> -				msg_ctr);
> +	/*
> +	 * write_msi_msg() will update PCI_MSI_FLAGS so there
> +	 * is no need to explicitly call pci_write_config here
> +	 */
>  	desc->msi_attrib.multiple = msgvec;
> -
>  	msg.address_lo = virt_to_phys((void *)pp->msi_data);
>  	msg.address_hi = 0x0;
>  	msg.data = pos;
> --
> 1.8.4.4
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux