drivers/ufs/host/ufs-qcom.c | 39 ++++++++++++++----------------------- 1 file changed, 15 insertions(+), 24 deletions(-)
ESI/MSI is a performance optimization feature that provides dedicated
interrupts per MCQ hardware queue . This is optional feature and
UFS MCQ should work with and without ESI feature.
Commit e46a28cea29a ("scsi: ufs: qcom: Remove the MSI descriptor abuse")
brings a regression in ESI (Enhanced System Interrupt) configuration
that causes a null pointer dereference when Platform MSI allocation
fails.
The issue occurs in when platform_device_msi_init_and_alloc_irqs()
in ufs_qcom_config_esi() fails (returns -EINVAL) but the current
code uses __free() macro for automatic cleanup free MSI resources
that were never successfully allocated.
Unable to handle kernel NULL pointer dereference at virtual
address 0000000000000008
Call trace:
mutex_lock+0xc/0x54 (P)
platform_device_msi_free_irqs_all+0x1c/0x40
ufs_qcom_config_esi+0x1d0/0x220 [ufs_qcom]
ufshcd_config_mcq+0x28/0x104
ufshcd_init+0xa3c/0xf40
ufshcd_pltfrm_init+0x504/0x7d4
ufs_qcom_probe+0x20/0x58 [ufs_qcom]
Fix by restructuring the ESI configuration to try MSI allocation
first, before any other resource allocation and instead use
explicit cleanup instead of __free() macro to avoid cleanup
of unallocated resources.
Tested on SM8750 platform with MCQ enabled, both with and without
Platform ESI support.
Fixes: e46a28cea29a ("scsi: ufs: qcom: Remove the MSI descriptor abuse")
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
Changes from v1:
1. Added correct sha1 of change id which caused regression.
2. Address Markus comment to add fixes: and Cc: tags.
---
drivers/ufs/host/ufs-qcom.c | 39 ++++++++++++++-----------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 4bbe4de1679b..bef8dc12de20 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -2078,17 +2078,6 @@ static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *data)
return IRQ_HANDLED;
}
-static void ufs_qcom_irq_free(struct ufs_qcom_irq *uqi)
-{
- for (struct ufs_qcom_irq *q = uqi; q->irq; q++)
- devm_free_irq(q->hba->dev, q->irq, q->hba);
-
- platform_device_msi_free_irqs_all(uqi->hba->dev);
- devm_kfree(uqi->hba->dev, uqi);
-}
-
-DEFINE_FREE(ufs_qcom_irq, struct ufs_qcom_irq *, if (_T) ufs_qcom_irq_free(_T))
-
static int ufs_qcom_config_esi(struct ufs_hba *hba)
{
struct ufs_qcom_host *host = ufshcd_get_variant(hba);
@@ -2103,18 +2092,18 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
*/
nr_irqs = hba->nr_hw_queues - hba->nr_queues[HCTX_TYPE_POLL];
- struct ufs_qcom_irq *qi __free(ufs_qcom_irq) =
- devm_kcalloc(hba->dev, nr_irqs, sizeof(*qi), GFP_KERNEL);
- if (!qi)
- return -ENOMEM;
- /* Preset so __free() has a pointer to hba in all error paths */
- qi[0].hba = hba;
-
ret = platform_device_msi_init_and_alloc_irqs(hba->dev, nr_irqs,
ufs_qcom_write_msi_msg);
if (ret) {
- dev_err(hba->dev, "Failed to request Platform MSI %d\n", ret);
- return ret;
+ dev_warn(hba->dev, "Platform MSI not supported or failed, continuing without ESI\n");
+ return ret; /* Continue without ESI */
+ }
+
+ struct ufs_qcom_irq *qi = devm_kcalloc(hba->dev, nr_irqs, sizeof(*qi), GFP_KERNEL);
+
+ if (!qi) {
+ platform_device_msi_free_irqs_all(hba->dev);
+ return -ENOMEM;
}
for (int idx = 0; idx < nr_irqs; idx++) {
@@ -2125,15 +2114,17 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
ret = devm_request_irq(hba->dev, qi[idx].irq, ufs_qcom_mcq_esi_handler,
IRQF_SHARED, "qcom-mcq-esi", qi + idx);
if (ret) {
- dev_err(hba->dev, "%s: Fail to request IRQ for %d, err = %d\n",
+ dev_err(hba->dev, "%s: Failed to request IRQ for %d, err = %d\n",
__func__, qi[idx].irq, ret);
- qi[idx].irq = 0;
+ /* Free previously allocated IRQs */
+ for (int j = 0; j < idx; j++)
+ devm_free_irq(hba->dev, qi[j].irq, qi + j);
+ platform_device_msi_free_irqs_all(hba->dev);
+ devm_kfree(hba->dev, qi);
return ret;
}
}
- retain_and_null_ptr(qi);
-
if (host->hw_ver.major >= 6) {
ufshcd_rmwl(hba, ESI_VEC_MASK, FIELD_PREP(ESI_VEC_MASK, MAX_ESI_VEC - 1),
REG_UFS_CFG3);
--
2.48.1
On 8/11/2025 1:03 PM, Nitin Rawat wrote:
> ESI/MSI is a performance optimization feature that provides dedicated
> interrupts per MCQ hardware queue . This is optional feature and
> UFS MCQ should work with and without ESI feature.
>
> Commit e46a28cea29a ("scsi: ufs: qcom: Remove the MSI descriptor abuse")
> brings a regression in ESI (Enhanced System Interrupt) configuration
> that causes a null pointer dereference when Platform MSI allocation
> fails.
>
> The issue occurs in when platform_device_msi_init_and_alloc_irqs()
> in ufs_qcom_config_esi() fails (returns -EINVAL) but the current
> code uses __free() macro for automatic cleanup free MSI resources
> that were never successfully allocated.
>
> Unable to handle kernel NULL pointer dereference at virtual
> address 0000000000000008
>
> Call trace:
> mutex_lock+0xc/0x54 (P)
> platform_device_msi_free_irqs_all+0x1c/0x40
> ufs_qcom_config_esi+0x1d0/0x220 [ufs_qcom]
> ufshcd_config_mcq+0x28/0x104
> ufshcd_init+0xa3c/0xf40
> ufshcd_pltfrm_init+0x504/0x7d4
> ufs_qcom_probe+0x20/0x58 [ufs_qcom]
>
> Fix by restructuring the ESI configuration to try MSI allocation
> first, before any other resource allocation and instead use
> explicit cleanup instead of __free() macro to avoid cleanup
> of unallocated resources.
>
> Tested on SM8750 platform with MCQ enabled, both with and without
> Platform ESI support.
>
> Fixes: e46a28cea29a ("scsi: ufs: qcom: Remove the MSI descriptor abuse")
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> ---
> Changes from v1:
> 1. Added correct sha1 of change id which caused regression.
> 2. Address Markus comment to add fixes: and Cc: tags.
> ---
> drivers/ufs/host/ufs-qcom.c | 39 ++++++++++++++-----------------------
> 1 file changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 4bbe4de1679b..bef8dc12de20 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -2078,17 +2078,6 @@ static irqreturn_t ufs_qcom_mcq_esi_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static void ufs_qcom_irq_free(struct ufs_qcom_irq *uqi)
> -{
> - for (struct ufs_qcom_irq *q = uqi; q->irq; q++)
> - devm_free_irq(q->hba->dev, q->irq, q->hba);
> -
> - platform_device_msi_free_irqs_all(uqi->hba->dev);
> - devm_kfree(uqi->hba->dev, uqi);
> -}
> -
> -DEFINE_FREE(ufs_qcom_irq, struct ufs_qcom_irq *, if (_T) ufs_qcom_irq_free(_T))
> -
> static int ufs_qcom_config_esi(struct ufs_hba *hba)
> {
> struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> @@ -2103,18 +2092,18 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
> */
> nr_irqs = hba->nr_hw_queues - hba->nr_queues[HCTX_TYPE_POLL];
>
> - struct ufs_qcom_irq *qi __free(ufs_qcom_irq) =
> - devm_kcalloc(hba->dev, nr_irqs, sizeof(*qi), GFP_KERNEL);
> - if (!qi)
> - return -ENOMEM;
> - /* Preset so __free() has a pointer to hba in all error paths */
> - qi[0].hba = hba;
> -
> ret = platform_device_msi_init_and_alloc_irqs(hba->dev, nr_irqs,
> ufs_qcom_write_msi_msg);
> if (ret) {
> - dev_err(hba->dev, "Failed to request Platform MSI %d\n", ret);
> - return ret;
> + dev_warn(hba->dev, "Platform MSI not supported or failed, continuing without ESI\n");
> + return ret; /* Continue without ESI */
> + }
> +
> + struct ufs_qcom_irq *qi = devm_kcalloc(hba->dev, nr_irqs, sizeof(*qi), GFP_KERNEL);
> +
> + if (!qi) {
> + platform_device_msi_free_irqs_all(hba->dev);
> + return -ENOMEM;
> }
>
> for (int idx = 0; idx < nr_irqs; idx++) {
> @@ -2125,15 +2114,17 @@ static int ufs_qcom_config_esi(struct ufs_hba *hba)
> ret = devm_request_irq(hba->dev, qi[idx].irq, ufs_qcom_mcq_esi_handler,
> IRQF_SHARED, "qcom-mcq-esi", qi + idx);
> if (ret) {
> - dev_err(hba->dev, "%s: Fail to request IRQ for %d, err = %d\n",
> + dev_err(hba->dev, "%s: Failed to request IRQ for %d, err = %d\n",
> __func__, qi[idx].irq, ret);
> - qi[idx].irq = 0;
> + /* Free previously allocated IRQs */
> + for (int j = 0; j < idx; j++)
> + devm_free_irq(hba->dev, qi[j].irq, qi + j);
> + platform_device_msi_free_irqs_all(hba->dev);
> + devm_kfree(hba->dev, qi);
> return ret;
> }
> }
>
> - retain_and_null_ptr(qi);
> -
> if (host->hw_ver.major >= 6) {
> ufshcd_rmwl(hba, ESI_VEC_MASK, FIELD_PREP(ESI_VEC_MASK, MAX_ESI_VEC - 1),
> REG_UFS_CFG3);
> --
> 2.48.1
>
Gentle Reminder!!
Hi Nitin! >> ESI/MSI is a performance optimization feature that provides dedicated >> interrupts per MCQ hardware queue . This is optional feature and UFS >> MCQ should work with and without ESI feature. > > Gentle Reminder!! > Already upstream: https://git.kernel.org/torvalds/c/6300d5c54387 -- Martin K. Petersen
© 2016 - 2025 Red Hat, Inc.