From: Alan Mikhak <[email protected]>
Modify dw_edma_irq_request() to check if a struct msi_desc entry exists
before copying the contents of its struct msi_msg pointer.
Without this sanity check, __get_cached_msi_msg() crashes when invoked by
dw_edma_irq_request() running on a Linux-based PCIe endpoint device. MSI
interrupt are not received by PCIe endpoint devices. If irq_get_msi_desc()
returns null, then there is no cached struct msi_msg to be copied.
This patch depends on the following patch:
[PATCH v2] dmaengine: dw-edma: Decouple dw-edma-core.c from struct pci_dev
https://patchwork.kernel.org/patch/11491757/
Rebased on linux-next which has above patch applied.
Fixes: Build error with config x86_64-randconfig-f003-20200422
Fixes: Build error with config s390-allmodconfig
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: Alan Mikhak <[email protected]>
---
drivers/dma/dw-edma/dw-edma-core.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index db401eb11322..306ab50462be 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -13,6 +13,7 @@
#include <linux/dmaengine.h>
#include <linux/err.h>
#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/dma/edma.h>
#include <linux/dma-mapping.h>
@@ -773,6 +774,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
u32 rd_mask = 1;
int i, err = 0;
u32 ch_cnt;
+ int irq;
ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
@@ -781,16 +783,16 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
if (dw->nr_irqs == 1) {
/* Common IRQ shared among all channels */
- err = request_irq(dw->ops->irq_vector(dev, 0),
- dw_edma_interrupt_common,
+ irq = dw->ops->irq_vector(dev, 0);
+ err = request_irq(irq, dw_edma_interrupt_common,
IRQF_SHARED, dw->name, &dw->irq[0]);
if (err) {
dw->nr_irqs = 0;
return err;
}
- get_cached_msi_msg(dw->ops->irq_vector(dev, 0),
- &dw->irq[0].msi);
+ if (irq_get_msi_desc(irq))
+ get_cached_msi_msg(irq, &dw->irq[0].msi);
} else {
/* Distribute IRQs equally among all channels */
int tmp = dw->nr_irqs;
@@ -804,7 +806,8 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
- err = request_irq(dw->ops->irq_vector(dev, i),
+ irq = dw->ops->irq_vector(dev, i);
+ err = request_irq(irq,
i < *wr_alloc ?
dw_edma_interrupt_write :
dw_edma_interrupt_read,
@@ -815,8 +818,8 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
return err;
}
- get_cached_msi_msg(dw->ops->irq_vector(dev, i),
- &dw->irq[i].msi);
+ if (irq_get_msi_desc(irq))
+ get_cached_msi_msg(irq, &dw->irq[i].msi);
}
dw->nr_irqs = i;
--
2.7.4
On Thu, Apr 23, 2020 at 2:58:21, Alan Mikhak <[email protected]>
wrote:
> From: Alan Mikhak <[email protected]>
>
> Modify dw_edma_irq_request() to check if a struct msi_desc entry exists
> before copying the contents of its struct msi_msg pointer.
>
> Without this sanity check, __get_cached_msi_msg() crashes when invoked by
> dw_edma_irq_request() running on a Linux-based PCIe endpoint device. MSI
> interrupt are not received by PCIe endpoint devices. If irq_get_msi_desc()
> returns null, then there is no cached struct msi_msg to be copied.
>
> This patch depends on the following patch:
> [PATCH v2] dmaengine: dw-edma: Decouple dw-edma-core.c from struct pci_dev
> https://urldefense.com/v3/__https://patchwork.kernel.org/patch/11491757/__;!!A4F2R9G_pg!L_vf_Tml7Ca4sWVvZp5crRCp7YsMj6B93G9cMAO8Dj3w9I0MArjwuwNKtDz9rr0RlpXiqPg$
>
> Rebased on linux-next which has above patch applied.
>
> Fixes: Build error with config x86_64-randconfig-f003-20200422
> Fixes: Build error with config s390-allmodconfig
> Reported-by: kbuild test robot <[email protected]>
> Signed-off-by: Alan Mikhak <[email protected]>
> ---
> drivers/dma/dw-edma/dw-edma-core.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index db401eb11322..306ab50462be 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -13,6 +13,7 @@
> #include <linux/dmaengine.h>
> #include <linux/err.h>
> #include <linux/interrupt.h>
> +#include <linux/irq.h>
> #include <linux/dma/edma.h>
> #include <linux/dma-mapping.h>
>
> @@ -773,6 +774,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> u32 rd_mask = 1;
> int i, err = 0;
> u32 ch_cnt;
> + int irq;
>
> ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
>
> @@ -781,16 +783,16 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
>
> if (dw->nr_irqs == 1) {
> /* Common IRQ shared among all channels */
> - err = request_irq(dw->ops->irq_vector(dev, 0),
> - dw_edma_interrupt_common,
> + irq = dw->ops->irq_vector(dev, 0);
> + err = request_irq(irq, dw_edma_interrupt_common,
> IRQF_SHARED, dw->name, &dw->irq[0]);
> if (err) {
> dw->nr_irqs = 0;
> return err;
> }
>
> - get_cached_msi_msg(dw->ops->irq_vector(dev, 0),
> - &dw->irq[0].msi);
> + if (irq_get_msi_desc(irq))
> + get_cached_msi_msg(irq, &dw->irq[0].msi);
> } else {
> /* Distribute IRQs equally among all channels */
> int tmp = dw->nr_irqs;
> @@ -804,7 +806,8 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
>
> for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
> - err = request_irq(dw->ops->irq_vector(dev, i),
> + irq = dw->ops->irq_vector(dev, i);
> + err = request_irq(irq,
> i < *wr_alloc ?
> dw_edma_interrupt_write :
> dw_edma_interrupt_read,
> @@ -815,8 +818,8 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> return err;
> }
>
> - get_cached_msi_msg(dw->ops->irq_vector(dev, i),
> - &dw->irq[i].msi);
> + if (irq_get_msi_desc(irq))
> + get_cached_msi_msg(irq, &dw->irq[i].msi);
> }
>
> dw->nr_irqs = i;
> --
> 2.7.4
Acked-by: Gustavo Pimentel <[email protected]>
On Thu, Apr 23, 2020 at 2:28 AM Gustavo Pimentel
<[email protected]> wrote:
>
> On Thu, Apr 23, 2020 at 2:58:21, Alan Mikhak <[email protected]>
> wrote:
>
> > From: Alan Mikhak <[email protected]>
> >
> > Modify dw_edma_irq_request() to check if a struct msi_desc entry exists
> > before copying the contents of its struct msi_msg pointer.
> >
> > Without this sanity check, __get_cached_msi_msg() crashes when invoked by
> > dw_edma_irq_request() running on a Linux-based PCIe endpoint device. MSI
> > interrupt are not received by PCIe endpoint devices. If irq_get_msi_desc()
> > returns null, then there is no cached struct msi_msg to be copied.
> >
> > This patch depends on the following patch:
> > [PATCH v2] dmaengine: dw-edma: Decouple dw-edma-core.c from struct pci_dev
> > https://urldefense.com/v3/__https://patchwork.kernel.org/patch/11491757/__;!!A4F2R9G_pg!L_vf_Tml7Ca4sWVvZp5crRCp7YsMj6B93G9cMAO8Dj3w9I0MArjwuwNKtDz9rr0RlpXiqPg$
> >
> > Rebased on linux-next which has above patch applied.
> >
> > Fixes: Build error with config x86_64-randconfig-f003-20200422
> > Fixes: Build error with config s390-allmodconfig
> > Reported-by: kbuild test robot <[email protected]>
> > Signed-off-by: Alan Mikhak <[email protected]>
> > ---
> > drivers/dma/dw-edma/dw-edma-core.c | 17 ++++++++++-------
> > 1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index db401eb11322..306ab50462be 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -13,6 +13,7 @@
> > #include <linux/dmaengine.h>
> > #include <linux/err.h>
> > #include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > #include <linux/dma/edma.h>
> > #include <linux/dma-mapping.h>
> >
> > @@ -773,6 +774,7 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> > u32 rd_mask = 1;
> > int i, err = 0;
> > u32 ch_cnt;
> > + int irq;
> >
> > ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
> >
> > @@ -781,16 +783,16 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> >
> > if (dw->nr_irqs == 1) {
> > /* Common IRQ shared among all channels */
> > - err = request_irq(dw->ops->irq_vector(dev, 0),
> > - dw_edma_interrupt_common,
> > + irq = dw->ops->irq_vector(dev, 0);
> > + err = request_irq(irq, dw_edma_interrupt_common,
> > IRQF_SHARED, dw->name, &dw->irq[0]);
> > if (err) {
> > dw->nr_irqs = 0;
> > return err;
> > }
> >
> > - get_cached_msi_msg(dw->ops->irq_vector(dev, 0),
> > - &dw->irq[0].msi);
> > + if (irq_get_msi_desc(irq))
> > + get_cached_msi_msg(irq, &dw->irq[0].msi);
> > } else {
> > /* Distribute IRQs equally among all channels */
> > int tmp = dw->nr_irqs;
> > @@ -804,7 +806,8 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> > dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
> >
> > for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
> > - err = request_irq(dw->ops->irq_vector(dev, i),
> > + irq = dw->ops->irq_vector(dev, i);
> > + err = request_irq(irq,
> > i < *wr_alloc ?
> > dw_edma_interrupt_write :
> > dw_edma_interrupt_read,
> > @@ -815,8 +818,8 @@ static int dw_edma_irq_request(struct dw_edma_chip *chip,
> > return err;
> > }
> >
> > - get_cached_msi_msg(dw->ops->irq_vector(dev, i),
> > - &dw->irq[i].msi);
> > + if (irq_get_msi_desc(irq))
> > + get_cached_msi_msg(irq, &dw->irq[i].msi);
> > }
> >
> > dw->nr_irqs = i;
> > --
> > 2.7.4
>
>
> Acked-by: Gustavo Pimentel <[email protected]>
Thanks Gustavo for the Ack.
FYI, I first considered adding an ops function to decouple dw-edma-core.c
from struct msi_msg. However, in a separate use case that I have in mind,
dw-edma would run on a host system having Synopsys DesignWare PCI eDMA
hardware on the host-side. In this use case, the host system eDMA engines
may be used in conjunction with an endpoint device also having the same
eDMA hardware. In this use case, dw-edma running on the host would need
to call get_cached_msi_msg() just in case the host has an msi_msg cached
from the endpoint device. As a result, I opted to not add a new ops
function.
Regards,
Alan
>
>
On 22-04-20, 18:58, Alan Mikhak wrote:
> From: Alan Mikhak <[email protected]>
>
> Modify dw_edma_irq_request() to check if a struct msi_desc entry exists
> before copying the contents of its struct msi_msg pointer.
>
> Without this sanity check, __get_cached_msi_msg() crashes when invoked by
> dw_edma_irq_request() running on a Linux-based PCIe endpoint device. MSI
> interrupt are not received by PCIe endpoint devices. If irq_get_msi_desc()
> returns null, then there is no cached struct msi_msg to be copied.
>
> This patch depends on the following patch:
> [PATCH v2] dmaengine: dw-edma: Decouple dw-edma-core.c from struct pci_dev
> https://patchwork.kernel.org/patch/11491757/
>
> Rebased on linux-next which has above patch applied.
These two para above should be added after s-o-b line as git-am skips
the lines after the marker. The above line are useful for applying but not
when applied. I have stripped these and applied the patch, thanks
--
~Vinod