Messages in this thread | | | Date | Tue, 24 May 2022 09:25:45 +0530 | Subject | Re: [PATCH] iommu/arm-smmu-qcom: Add debug support for TLB sync timeouts | From | Sai Prakash Ranjan <> |
| |
On 5/23/2022 10:48 PM, Sai Prakash Ranjan wrote: > TLB sync timeouts can be due to various reasons such as TBU power down > or pending TCU/TBU invalidation/sync and so on. Debugging these often > require dumping of some implementation defined registers to know the > status of TBU/TCU operations and some of these registers are not > accessible in non-secure world such as from kernel and requires SMC > calls to read them in the secure world. So, add this debug support > to dump implementation defined registers for TLB sync timeout issues. > > Signed-off-by: Sai Prakash Ranjan <quic_saipraka@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 50 ++++++++++++++++++++++ > drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 + > drivers/iommu/arm/arm-smmu/arm-smmu.h | 4 ++ > 3 files changed, 56 insertions(+) > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index 7820711c4560..22e9a0085475 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -5,11 +5,19 @@ > > #include <linux/acpi.h> > #include <linux/adreno-smmu-priv.h> > +#include <linux/delay.h> > #include <linux/of_device.h> > #include <linux/qcom_scm.h> > > #include "arm-smmu.h" > > +#define QCOM_DUMMY_VAL -1 > + > +/* Implementation Defined Register Space 0 registers */ > +#define QCOM_SMMU_STATS_SYNC_INV_TBU_ACK 0x5dc > +#define QCOM_SMMU_TBU_PWR_STATUS 0x2204 > +#define QCOM_SMMU_MMU2QSS_AND_SAFE_WAIT_CNTR 0x2670 > + > struct qcom_smmu { > struct arm_smmu_device smmu; > bool bypass_quirk; > @@ -22,6 +30,46 @@ static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) > return container_of(smmu, struct qcom_smmu, smmu); > } > > +static void qcom_smmu_tlb_sync(struct arm_smmu_device *smmu, int page, > + int sync, int status) > +{ > + u32 sync_inv_ack, sync_inv_progress, tbu_pwr_status; > + unsigned int spin_cnt, delay; > + u32 reg; > + int ret; > + > + arm_smmu_writel(smmu, page, sync, QCOM_DUMMY_VAL); > + for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) { > + for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) { > + reg = arm_smmu_readl(smmu, page, status); > + if (!(reg & ARM_SMMU_sTLBGSTATUS_GSACTIVE)) > + return; > + cpu_relax(); > + } > + udelay(delay); > + } > + > + sync_inv_ack = arm_smmu_readl(smmu, ARM_SMMU_IMPL_DEF0, > + QCOM_SMMU_STATS_SYNC_INV_TBU_ACK);
Sorry, this doesn't work always, looks like on earlier chipsets this is a secure register and reading it from non-secure world would probably blow. Also this register can be in other implementation defined space for different chipsets. So I think we can use SCM call here and have a device specific data based on already existing compatible for QCOM SoCs to identify IMP_DEF space used.
> + ret = qcom_scm_io_readl(smmu->ioaddr + QCOM_SMMU_TBU_PWR_STATUS, > + &tbu_pwr_status); > + if (ret) > + dev_err_ratelimited(smmu->dev, > + "Failed to read TBU power status: %d\n", ret); > + > + ret = qcom_scm_io_readl(smmu->ioaddr + QCOM_SMMU_MMU2QSS_AND_SAFE_WAIT_CNTR, > + &sync_inv_progress); > + if (ret) > + dev_err_ratelimited(smmu->dev, > + "Failed to read SAFE WAIT counter: %d\n", ret); > + > + dev_err_ratelimited(smmu->dev, > + "TLB sync timed out -- SMMU may be deadlocked\n" > + "TBU: sync_inv_ack %#x power_status %#x sync_inv_progress %#x\n", > + sync_inv_ack, tbu_pwr_status, sync_inv_progress); > +} > + > static void qcom_adreno_smmu_write_sctlr(struct arm_smmu_device *smmu, int idx, > u32 reg) > { > @@ -374,6 +422,7 @@ static const struct arm_smmu_impl qcom_smmu_impl = { > .def_domain_type = qcom_smmu_def_domain_type, > .reset = qcom_smmu500_reset, > .write_s2cr = qcom_smmu_write_s2cr, > + .tlb_sync = qcom_smmu_tlb_sync, > }; > > static const struct arm_smmu_impl qcom_adreno_smmu_impl = { > @@ -382,6 +431,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_impl = { > .reset = qcom_smmu500_reset, > .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > .write_sctlr = qcom_adreno_smmu_write_sctlr, > + .tlb_sync = qcom_smmu_tlb_sync, > }; > > static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu, > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c > index 2ed3594f384e..4c5b51109835 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c > @@ -2099,6 +2099,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev) > if (IS_ERR(smmu->base)) > return PTR_ERR(smmu->base); > ioaddr = res->start; > + smmu->ioaddr = ioaddr; > + > /* > * The resource size should effectively match the value of SMMU_TOP; > * stash that temporarily until we know PAGESIZE to validate it with. > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h > index 2b9b42fb6f30..8cf6567d970f 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h > @@ -278,6 +278,7 @@ struct arm_smmu_device { > struct device *dev; > > void __iomem *base; > + phys_addr_t ioaddr; > unsigned int numpage; > unsigned int pgshift; > > @@ -502,6 +503,9 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page, > > #define ARM_SMMU_GR0 0 > #define ARM_SMMU_GR1 1 > + > +#define ARM_SMMU_IMPL_DEF0 2 > + > #define ARM_SMMU_CB(s, n) ((s)->numpage + (n)) > > #define arm_smmu_gr0_read(s, o) \
| |