2020-07-01 22:11:42

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 0/2] Fix IRQ 0 cases

From: Bjorn Helgaas <[email protected]>

a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
added a warning if platform_get_irq() returns IRQ 0. These patches fix a
couple places that might cause that.

Bjorn Helgaas (2):
ARM: imx: Remove imx_add_imx_dma() unused irq_err argument
virtio-mmio: Reject invalid IRQ 0 command line argument

arch/arm/mach-imx/devices/devices-common.h | 2 +-
arch/arm/mach-imx/devices/platform-imx-dma.c | 6 +-----
arch/arm/mach-imx/mm-imx21.c | 3 +--
arch/arm/mach-imx/mm-imx27.c | 3 +--
drivers/virtio/virtio_mmio.c | 4 ++--
5 files changed, 6 insertions(+), 12 deletions(-)


base-commit: b3a9e3b9622ae10064826dccb4f7a52bd88c7407
--
2.25.1


2020-07-01 22:14:20

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 2/2] virtio-mmio: Reject invalid IRQ 0 command line argument

From: Bjorn Helgaas <[email protected]>

The "virtio_mmio.device=" command line argument allows a user to specify
the size, address, and IRQ of a virtio device. Previously the only
requirement for the IRQ was that it be an unsigned integer.

Zero is an unsigned integer but an invalid IRQ number, and after
a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid"),
attempts to use IRQ 0 cause warnings.

If the user specifies IRQ 0, return failure instead of registering a device
with IRQ 0.

Fixes: a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
Signed-off-by: Bjorn Helgaas <[email protected]>
Cc: Michael S. Tsirkin <[email protected]>
Cc: Jason Wang <[email protected]>
Cc: [email protected]
---
drivers/virtio/virtio_mmio.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 9d16aaffca9d..627ac0487494 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -641,11 +641,11 @@ static int vm_cmdline_set(const char *device,
&vm_cmdline_id, &consumed);

/*
- * sscanf() must processes at least 2 chunks; also there
+ * sscanf() must process at least 2 chunks; also there
* must be no extra characters after the last chunk, so
* str[consumed] must be '\0'
*/
- if (processed < 2 || str[consumed])
+ if (processed < 2 || str[consumed] || irq == 0)
return -EINVAL;

resources[0].flags = IORESOURCE_MEM;
--
2.25.1

2020-07-01 22:14:30

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 1/2] ARM: imx: Remove imx_add_imx_dma() unused irq_err argument

From: Bjorn Helgaas <[email protected]>

No callers of imx_add_imx_dma() need an error IRQ, so they supply 0 as
"irq_err", which means we register a resource of IRQ 0, which is invalid
and causes a warning if used.

Remove the "irq_err" argument altogether so there's no chance of trying to
use the invalid IRQ 0.

Fixes: a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
Signed-off-by: Bjorn Helgaas <[email protected]>
Cc: Russell King <[email protected]>
Cc: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: [email protected]
Cc: Fabio Estevam <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/arm/mach-imx/devices/devices-common.h | 2 +-
arch/arm/mach-imx/devices/platform-imx-dma.c | 6 +-----
arch/arm/mach-imx/mm-imx21.c | 3 +--
arch/arm/mach-imx/mm-imx27.c | 3 +--
4 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
index 2a685adec1df..ae84c08e11fa 100644
--- a/arch/arm/mach-imx/devices/devices-common.h
+++ b/arch/arm/mach-imx/devices/devices-common.h
@@ -289,6 +289,6 @@ struct platform_device *__init imx_add_spi_imx(
const struct spi_imx_master *pdata);

struct platform_device *imx_add_imx_dma(char *name, resource_size_t iobase,
- int irq, int irq_err);
+ int irq);
struct platform_device *imx_add_imx_sdma(char *name,
resource_size_t iobase, int irq, struct sdma_platform_data *pdata);
diff --git a/arch/arm/mach-imx/devices/platform-imx-dma.c b/arch/arm/mach-imx/devices/platform-imx-dma.c
index 26b47b36257b..12656f24ad0d 100644
--- a/arch/arm/mach-imx/devices/platform-imx-dma.c
+++ b/arch/arm/mach-imx/devices/platform-imx-dma.c
@@ -6,7 +6,7 @@
#include "devices-common.h"

struct platform_device __init __maybe_unused *imx_add_imx_dma(char *name,
- resource_size_t iobase, int irq, int irq_err)
+ resource_size_t iobase, int irq)
{
struct resource res[] = {
{
@@ -17,10 +17,6 @@ struct platform_device __init __maybe_unused *imx_add_imx_dma(char *name,
.start = irq,
.end = irq,
.flags = IORESOURCE_IRQ,
- }, {
- .start = irq_err,
- .end = irq_err,
- .flags = IORESOURCE_IRQ,
},
};

diff --git a/arch/arm/mach-imx/mm-imx21.c b/arch/arm/mach-imx/mm-imx21.c
index 50a2edac8513..b834026e4615 100644
--- a/arch/arm/mach-imx/mm-imx21.c
+++ b/arch/arm/mach-imx/mm-imx21.c
@@ -78,8 +78,7 @@ void __init imx21_soc_init(void)
mxc_register_gpio("imx21-gpio", 5, MX21_GPIO6_BASE_ADDR, SZ_256, MX21_INT_GPIO, 0);

pinctrl_provide_dummies();
- imx_add_imx_dma("imx21-dma", MX21_DMA_BASE_ADDR,
- MX21_INT_DMACH0, 0); /* No ERR irq */
+ imx_add_imx_dma("imx21-dma", MX21_DMA_BASE_ADDR, MX21_INT_DMACH0);
platform_device_register_simple("imx21-audmux", 0, imx21_audmux_res,
ARRAY_SIZE(imx21_audmux_res));
}
diff --git a/arch/arm/mach-imx/mm-imx27.c b/arch/arm/mach-imx/mm-imx27.c
index 4e4125140025..2717614f101d 100644
--- a/arch/arm/mach-imx/mm-imx27.c
+++ b/arch/arm/mach-imx/mm-imx27.c
@@ -79,8 +79,7 @@ void __init imx27_soc_init(void)
mxc_register_gpio("imx21-gpio", 5, MX27_GPIO6_BASE_ADDR, SZ_256, MX27_INT_GPIO, 0);

pinctrl_provide_dummies();
- imx_add_imx_dma("imx27-dma", MX27_DMA_BASE_ADDR,
- MX27_INT_DMACH0, 0); /* No ERR irq */
+ imx_add_imx_dma("imx27-dma", MX27_DMA_BASE_ADDR, MX27_INT_DMACH0);
/* imx27 has the imx21 type audmux */
platform_device_register_simple("imx21-audmux", 0, imx27_audmux_res,
ARRAY_SIZE(imx27_audmux_res));
--
2.25.1

2020-07-02 03:08:59

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH 2/2] virtio-mmio: Reject invalid IRQ 0 command line argument


On 2020/7/2 上午6:10, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> The "virtio_mmio.device=" command line argument allows a user to specify
> the size, address, and IRQ of a virtio device. Previously the only
> requirement for the IRQ was that it be an unsigned integer.
>
> Zero is an unsigned integer but an invalid IRQ number, and after
> a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid"),
> attempts to use IRQ 0 cause warnings.
>
> If the user specifies IRQ 0, return failure instead of registering a device
> with IRQ 0.
>
> Fixes: a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
> Signed-off-by: Bjorn Helgaas <[email protected]>
> Cc: Michael S. Tsirkin <[email protected]>
> Cc: Jason Wang <[email protected]>
> Cc: [email protected]
> ---
> drivers/virtio/virtio_mmio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 9d16aaffca9d..627ac0487494 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -641,11 +641,11 @@ static int vm_cmdline_set(const char *device,
> &vm_cmdline_id, &consumed);
>
> /*
> - * sscanf() must processes at least 2 chunks; also there
> + * sscanf() must process at least 2 chunks; also there
> * must be no extra characters after the last chunk, so
> * str[consumed] must be '\0'
> */
> - if (processed < 2 || str[consumed])
> + if (processed < 2 || str[consumed] || irq == 0)
> return -EINVAL;
>
> resources[0].flags = IORESOURCE_MEM;


Acked-by: Jason Wang <[email protected]>


2020-07-13 03:27:59

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: imx: Remove imx_add_imx_dma() unused irq_err argument

On Wed, Jul 01, 2020 at 05:10:39PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> No callers of imx_add_imx_dma() need an error IRQ, so they supply 0 as
> "irq_err", which means we register a resource of IRQ 0, which is invalid
> and causes a warning if used.
>
> Remove the "irq_err" argument altogether so there's no chance of trying to
> use the invalid IRQ 0.
>
> Fixes: a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
> Signed-off-by: Bjorn Helgaas <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Sascha Hauer <[email protected]>
> Cc: [email protected]
> Cc: Fabio Estevam <[email protected]>
> Cc: [email protected]
> Cc: [email protected]

Applied, thanks.

2020-07-21 22:13:34

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/2] virtio-mmio: Reject invalid IRQ 0 command line argument

On Thu, Jul 02, 2020 at 11:06:11AM +0800, Jason Wang wrote:
> On 2020/7/2 上午6:10, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <[email protected]>
> >
> > The "virtio_mmio.device=" command line argument allows a user to specify
> > the size, address, and IRQ of a virtio device. Previously the only
> > requirement for the IRQ was that it be an unsigned integer.
> >
> > Zero is an unsigned integer but an invalid IRQ number, and after
> > a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid"),
> > attempts to use IRQ 0 cause warnings.
> >
> > If the user specifies IRQ 0, return failure instead of registering a device
> > with IRQ 0.
> >
> > Fixes: a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
> > Signed-off-by: Bjorn Helgaas <[email protected]>
> > Cc: Michael S. Tsirkin <[email protected]>
> > Cc: Jason Wang <[email protected]>
> > Cc: [email protected]
> > ---
> > drivers/virtio/virtio_mmio.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> > index 9d16aaffca9d..627ac0487494 100644
> > --- a/drivers/virtio/virtio_mmio.c
> > +++ b/drivers/virtio/virtio_mmio.c
> > @@ -641,11 +641,11 @@ static int vm_cmdline_set(const char *device,
> > &vm_cmdline_id, &consumed);
> > /*
> > - * sscanf() must processes at least 2 chunks; also there
> > + * sscanf() must process at least 2 chunks; also there
> > * must be no extra characters after the last chunk, so
> > * str[consumed] must be '\0'
> > */
> > - if (processed < 2 || str[consumed])
> > + if (processed < 2 || str[consumed] || irq == 0)
> > return -EINVAL;
> > resources[0].flags = IORESOURCE_MEM;
>
> Acked-by: Jason Wang <[email protected]>

Thanks, I applied this to my for-linus branch for v5.8.

2020-07-22 11:47:42

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 2/2] virtio-mmio: Reject invalid IRQ 0 command line argument

On Wed, Jul 01, 2020 at 05:10:40PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> The "virtio_mmio.device=" command line argument allows a user to specify
> the size, address, and IRQ of a virtio device. Previously the only
> requirement for the IRQ was that it be an unsigned integer.
>
> Zero is an unsigned integer but an invalid IRQ number, and after
> a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid"),
> attempts to use IRQ 0 cause warnings.
>
> If the user specifies IRQ 0, return failure instead of registering a device
> with IRQ 0.
>
> Fixes: a85a6c86c25be ("driver core: platform: Clarify that IRQ 0 is invalid")
> Signed-off-by: Bjorn Helgaas <[email protected]>
> Cc: Michael S. Tsirkin <[email protected]>
> Cc: Jason Wang <[email protected]>
> Cc: [email protected]

Acked-by: Michael S. Tsirkin <[email protected]>

> ---
> drivers/virtio/virtio_mmio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 9d16aaffca9d..627ac0487494 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -641,11 +641,11 @@ static int vm_cmdline_set(const char *device,
> &vm_cmdline_id, &consumed);
>
> /*
> - * sscanf() must processes at least 2 chunks; also there
> + * sscanf() must process at least 2 chunks; also there
> * must be no extra characters after the last chunk, so
> * str[consumed] must be '\0'
> */
> - if (processed < 2 || str[consumed])
> + if (processed < 2 || str[consumed] || irq == 0)
> return -EINVAL;
>
> resources[0].flags = IORESOURCE_MEM;
> --
> 2.25.1