This patch series improves the power up behavior by allowing MHI host driver to
set BHI and/or BHIe offsets early on in the preparation phase and fail pre-power
up if offsets are not found or not within a limited MMIO region. This also
allows MHI host to clean up the offsets in the unprepare after power down phase.
Going forward, controllers will be required to specify a reg_len field which
will be used to check whether the BHI/BHIe offsets are in range or not.
This series has been tested on X86_64 architecture with the PCI generic driver
as controller and an SDX55 device.
Bhaumik Bhatt (6):
bus: mhi: core: Set BHI/BHIe offsets on power up preparation
bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up
bus: mhi: Add MMIO region length to controller structure
ath11k: set register access length for MHI driver
bus: mhi: pci_generic: Set register access length for MHI driver
bus: mhi: core: Add range checks for BHI and BHIe
drivers/bus/mhi/core/init.c | 58 +++++++++++++++++++++++------------
drivers/bus/mhi/core/pm.c | 28 +++--------------
drivers/bus/mhi/pci_generic.c | 1 +
drivers/net/wireless/ath/ath11k/mhi.c | 1 +
include/linux/mhi.h | 2 ++
5 files changed, 47 insertions(+), 43 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Set the BHI and BHIe pointers to NULL as part of clean-up. This
makes sure that stale pointers are not accessed after powering
MHI down.
Suggested-by: Hemant Kumar <[email protected]>
Signed-off-by: Bhaumik Bhatt <[email protected]>
---
drivers/bus/mhi/core/init.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 11c7a3d..1cc2f22 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1132,6 +1132,9 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
mhi_cntrl->rddm_image = NULL;
}
+ mhi_cntrl->bhi = NULL;
+ mhi_cntrl->bhie = NULL;
+
mhi_deinit_dev_ctxt(mhi_cntrl);
}
EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
MHI driver requires register space length to add range checks and
prevent memory region accesses outside of that for MMIO space.
Set it from the PCI generic controller driver before registering
the MHI controller.
Signed-off-by: Bhaumik Bhatt <[email protected]>
---
drivers/bus/mhi/pci_generic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
index 7c810f0..fb7889f 100644
--- a/drivers/bus/mhi/pci_generic.c
+++ b/drivers/bus/mhi/pci_generic.c
@@ -463,6 +463,7 @@ static int mhi_pci_claim(struct mhi_controller *mhi_cntrl,
return err;
}
mhi_cntrl->regs = pcim_iomap_table(pdev)[bar_num];
+ mhi_cntrl->reg_len = pci_resource_len(pdev, bar_num);
err = pci_set_dma_mask(pdev, dma_mask);
if (err) {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
When obtaining the BHI or BHIe offsets during the power up
preparation phase, range checks are missing. These can help
controller drivers avoid accessing any address outside of the
MMIO region. Ensure that mhi_cntrl->reg_len is set before MHI
registration as it is a required field and range checks will
fail without it.
Signed-off-by: Bhaumik Bhatt <[email protected]>
---
drivers/bus/mhi/core/init.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 1cc2f22..86ad06e 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -885,7 +885,8 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
if (!mhi_cntrl || !mhi_cntrl->cntrl_dev || !mhi_cntrl->regs ||
!mhi_cntrl->runtime_get || !mhi_cntrl->runtime_put ||
!mhi_cntrl->status_cb || !mhi_cntrl->read_reg ||
- !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs || !mhi_cntrl->irq)
+ !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs ||
+ !mhi_cntrl->irq || !mhi_cntrl->reg_len)
return -EINVAL;
ret = parse_config(mhi_cntrl, config);
@@ -1077,6 +1078,12 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
dev_err(dev, "Error getting BHI offset\n");
goto error_reg_offset;
}
+
+ if (bhi_off >= mhi_cntrl->reg_len) {
+ dev_err(dev, "BHI offset is out of range\n");
+ ret = -EINVAL;
+ goto error_reg_offset;
+ }
mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;
if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {
@@ -1086,6 +1093,12 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
dev_err(dev, "Error getting BHIE offset\n");
goto error_reg_offset;
}
+
+ if (bhie_off >= mhi_cntrl->reg_len) {
+ dev_err(dev, "BHIe offset is out of range\n");
+ ret = -EINVAL;
+ goto error_reg_offset;
+ }
mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On 4/16/2021 12:47 PM, Bhaumik Bhatt wrote:
> Set the BHI and BHIe pointers to NULL as part of clean-up. This
> makes sure that stale pointers are not accessed after powering
> MHI down.
>
> Suggested-by: Hemant Kumar <[email protected]>
> Signed-off-by: Bhaumik Bhatt <[email protected]>
> ---
Reviewed-by: Jeffrey Hugo <[email protected]>