[PATCH 0/2] Check for MSI allocation

[PATCH 0/2] Check for MSI allocation

[Julian Vetter]

Our HW has a mailbox periph that supports up to 8192 MSIs, so the currently
supported 2048 is not enough. I believe there is no real reason to
contstrain the max platform MSIs to 2048, because the value is stored in
an irq_hw_number_t which is big enough to hold more MSIs. The value is
OR'ed with the device ID of which there is also a limited number.
The second patch contains a WARN() to warn the user if a device tries to
obtain more that MAX_DEV_MSIS, because this could lead to potentially
conflicting virtual interrupt numbers (as was the case for us) without
the Linux kernel telling us.

Julian Vetter (2):
  msi: The MSI framework only supports 2048 platform MSIs
  msi: Add sanity check if more than MAX_DEV_MSIS MSIs are requested

 drivers/base/platform-msi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH 1/2] msi: The MSI framework only supports 2048 platform MSIs

[Julian Vetter]

Some devices need more MSIs. To support this the number must be
increased.

Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com>
---
 drivers/base/platform-msi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 296ea673d661..4b0b2fe3a7ff 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -13,7 +13,7 @@
 #include <linux/msi.h>
 #include <linux/slab.h>
 
-#define DEV_ID_SHIFT	21
+#define DEV_ID_SHIFT	19
 #define MAX_DEV_MSIS	(1 << (32 - DEV_ID_SHIFT))
 
 /*
-- 
2.17.1

[PATCH 2/2] msi: Add sanity check if more than MAX_DEV_MSIS MSIs are requested

[Julian Vetter]

If a device requests more than MAX_DEV_MSIS the MSI index will collide with
the devid and might cause Linux to compute twice the same virtual interrupt
number for two different devices.

Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com>
---
 drivers/base/platform-msi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 4b0b2fe3a7ff..627d8f6b83ea 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -40,6 +40,8 @@ static irq_hw_number_t platform_msi_calc_hwirq(struct msi_desc *desc)
 {
 	u32 devid = desc->dev->msi.data->platform_data->devid;
 
+	WARN_ON(desc->msi_index >= MAX_DEV_MSIS);
+
 	return (devid << (32 - DEV_ID_SHIFT)) | desc->msi_index;
 }
 
-- 
2.17.1

Re: [PATCH 2/2] msi: Add sanity check if more than MAX_DEV_MSIS MSIs are requested

[Greg KH]

On Thu, Jun 09, 2022 at 05:38:43PM +0200, Julian Vetter wrote:
> If a device requests more than MAX_DEV_MSIS the MSI index will collide with
> the devid and might cause Linux to compute twice the same virtual interrupt
> number for two different devices.
> 
> Signed-off-by: Julian Vetter <jvetter@kalrayinc.com>
> Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com>
> ---
>  drivers/base/platform-msi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
> index 4b0b2fe3a7ff..627d8f6b83ea 100644
> --- a/drivers/base/platform-msi.c
> +++ b/drivers/base/platform-msi.c
> @@ -40,6 +40,8 @@ static irq_hw_number_t platform_msi_calc_hwirq(struct msi_desc *desc)
>  {
>  	u32 devid = desc->dev->msi.data->platform_data->devid;
>  
> +	WARN_ON(desc->msi_index >= MAX_DEV_MSIS);

So you just caused a system to reboot if you have panic-on-warn?  What
is someone supposed to do if this is ever hit?  Why can't you handle it
as an error and properly recover instead of just ignoring it?

thanks,

greg k-h