iommu/arm-smmu: Track context bank state
Robin Murphy
reviewerPATCH 1/2 iommu/arm-smmu: Track context bank state
Echoing what we do for Stream Map Entries, maintain a software shadow state for context bank configuration. With this in place, we are mere moments away from blissfully easy suspend/resume support.
Signed-off-by Robin Murphy <[email protected]>
---
Since the runtime PM discussion has come back again, I figured it was probably about time to finish off my plan for system PM. Lightly tested on Juno (MMU-401) with hibernation.
Robin.
drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++++------------------ 1 file changed, 97 insertions(+), 62 deletions(-)
drivers/iommu/arm-smmu.c +97 -62
@@ -338,6 +338,13 @@ struct arm_smmu_smr {bool valid; }; +struct arm_smmu_cb { +u64 ttbr[2 ]; +u32 tcr[2 ]; +u32 mair[2 ]; +struct arm_smmu_cfg *cfg; +}; +struct arm_smmu_master_cfg {struct arm_smmu_device *smmu;s16 smendx[];
@@ -380,6 +387,7 @@ struct arm_smmu_device {u32 num_context_banks;u32 num_s2_context_banks;DECLARE_BITMAP (context_map,ARM_SMMU_MAX_CBS ); +struct arm_smmu_cb *cbs;atomic_t irptndx;u32 num_mapping_groups;
@@ -768,17 +776,69 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)static void arm_smmu_init_context_bank (struct arm_smmu_domain *smmu_domain,struct io_pgtable_cfg *pgtbl_cfg) { -u32 reg, reg2; -u64 reg64; -bool stage1;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -struct arm_smmu_device *smmu = smmu_domain->smmu; +struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx]; +bool stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; + + cb->cfg = cfg; + +/* TTBCR */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->tcr[0 ] = pgtbl_cfg->arm_v7s_cfg.tcr; + }else { + cb->tcr[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.tcr; + cb->tcr[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; + cb->tcr[1 ] |=TTBCR2_SEP_UPSTREAM ; +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) + cb->tcr[1 ] |=TTBCR2_AS ; + } + }else { + cb->tcr[0 ] = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; + } + +/* TTBRs */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->ttbr[0 ] = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; + cb->ttbr[1 ] = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; + }else { + cb->ttbr[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; + cb->ttbr[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; + } + }else { + cb->ttbr[0 ] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; + } + +/* MAIRs (stage-1 only) */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->mair[0 ] = pgtbl_cfg->arm_v7s_cfg.prrr; + cb->mair[1 ] = pgtbl_cfg->arm_v7s_cfg.nmrr; + }else { + cb->mair[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; + cb->mair[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; + } + } +} + +static void arm_smmu_write_context_bank (struct arm_smmu_device *smmu,int idx) +{ +u32 reg; +bool stage1; +struct arm_smmu_cb *cb = &smmu->cbs[idx]; +struct arm_smmu_cfg *cfg = cb->cfg; +struct arm_smmu_cfg default_cfg = {0 };void __iomem *cb_base, *gr1_base; +if (!cfg) + cfg = &default_cfg; + gr1_base =ARM_SMMU_GR1 (smmu); stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); + cb_base =ARM_SMMU_CB (smmu, idx); +/* CBA2R */ if (smmu->version >ARM_SMMU_V1 ) {if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) reg =CBA2R_RW64_64BIT ;
@@ -788,7 +848,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,if (smmu->features &ARM_SMMU_FEAT_VMID16 ) reg |= cfg->vmid <<CBA2R_VMID_SHIFT ; -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (idx)); }/* CBAR */
@@ -807,72 +867,43 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,/* 8-bit VMIDs live in CBAR */ reg |= cfg->vmid <<CBAR_VMID_SHIFT ; } -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (idx));/* * TTBCR * We must write this before the TTBRs, since it determines the * access behaviour of some fields (in particular, ASID[15:8]). */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.tcr; - reg2 =0 ; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.tcr; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; - reg2 |=TTBCR2_SEP_UPSTREAM ; -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) - reg2 |=TTBCR2_AS ; - } -if (smmu->version >ARM_SMMU_V1 ) -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_TTBCR2 ); - }else { - reg = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBCR ); +if (stage1 && smmu->version >ARM_SMMU_V1 ) +writel_relaxed (cb->tcr[1 ], cb_base +ARM_SMMU_CB_TTBCR2 ); +writel_relaxed (cb->tcr[0 ], cb_base +ARM_SMMU_CB_TTBCR );/* TTBRs */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR0 ); - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR1 ); -writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); - }else { - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR1 ); - } +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { +writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); +writel_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writel_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }else { - reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }/* MAIRs (stage-1 only) */ if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.prrr; - reg2 = pgtbl_cfg->arm_v7s_cfg.nmrr; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_S1_MAIR0 ); -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_S1_MAIR1 ); +writel_relaxed (cb->mair[0 ], cb_base +ARM_SMMU_CB_S1_MAIR0 ); +writel_relaxed (cb->mair[1 ], cb_base +ARM_SMMU_CB_S1_MAIR1 ); }/* SCTLR */ - reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; -if (stage1) - reg |=SCTLR_S1_ASIDPNE ; -#ifdef __BIG_ENDIAN - reg |=SCTLR_E ; -#endif +if (!cb->cfg) { + reg =0 ; + }else { + reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; +if (stage1) + reg |=SCTLR_S1_ASIDPNE ; +if (IS_ENABLED (CONFIG_CPU_BIG_ENDIAN )) + reg |=SCTLR_E ; + }writel_relaxed (reg, cb_base +ARM_SMMU_CB_SCTLR ); }
@@ -1035,6 +1066,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,/* Initialise the context bank with our page table cfg */ arm_smmu_init_context_bank (smmu_domain, &pgtbl_cfg); +arm_smmu_write_context_bank (smmu, cfg->cbndx);/* * Request context fault interrupt. Do this last to avoid the
@@ -1067,7 +1099,6 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain);struct arm_smmu_device *smmu = smmu_domain->smmu;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -void __iomem *cb_base;int irq;if (!smmu || domain->type ==IOMMU_DOMAIN_IDENTITY )
@@ -1077,8 +1108,8 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)* Disable the context bank and free the page tables before freeing * it. */ - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); + smmu->cbs[cfg->cbndx] = (struct arm_smmu_cb ){0 }; +arm_smmu_write_context_bank (smmu, cfg->cbndx);if (cfg->irptndx !=INVALID_IRPTNDX ) { irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];
@@ -1722,7 +1753,6 @@ static struct iommu_ops arm_smmu_ops = {static void arm_smmu_device_reset (struct arm_smmu_device *smmu) {void __iomem *gr0_base =ARM_SMMU_GR0 (smmu); -void __iomem *cb_base;int i;u32 reg, major;
@@ -1758,8 +1788,9 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)/* Make sure all context banks are disabled and clear CB_FSR */ for (i =0 ; i < smmu->num_context_banks; ++i) { - cb_base =ARM_SMMU_CB (smmu, i); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); +void __iomem *cb_base =ARM_SMMU_CB (smmu, i); + +arm_smmu_write_context_bank (smmu, i);writel_relaxed (FSR_FAULT , cb_base +ARM_SMMU_CB_FSR );/* * Disable MMU-500's not-particularly-beneficial next-page
@@ -1964,6 +1995,10 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) smmu->cavium_id_base -= smmu->num_context_banks;dev_notice (smmu->dev,"\tenabling workaround for Cavium erratum 27704\n" ); } + smmu->cbs =devm_kcalloc (smmu->dev, smmu->num_context_banks, +sizeof (*smmu->cbs),GFP_KERNEL ); +if (!smmu->cbs) +return -ENOMEM ;/* ID2 */ id =readl_relaxed (gr0_base +ARM_SMMU_GR0_ID2 );
Signature · 1 line
2.12.2.dirty
Robin Murphy
reviewerPATCH 2/2 iommu/arm-smmu: Add system PM support
With all our hardware state tracked in such a way that we can naturally restore it as part of the necessary reset, resuming is trivial, and there's nothing to do on suspend at all.
Signed-off-by Robin Murphy <[email protected]>
--- drivers/iommu/arm-smmu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/iommu/arm-smmu.c +12 -0
@@ -2356,10 +2356,22 @@ static int arm_smmu_device_remove(struct platform_device *pdev)return 0 ; } +static int __maybe_unused arm_smmu_pm_resume (struct device *dev) +{ +struct arm_smmu_device *smmu =dev_get_drvdata (dev); + +arm_smmu_device_reset (smmu); +return 0 ; +} + + +static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume); +static struct platform_driver arm_smmu_driver = { .driver = { .name ="arm-smmu" , .of_match_table =of_match_ptr (arm_smmu_of_match), + .pm = &arm_smmu_pm_ops, }, .probe = arm_smmu_device_probe, .remove = arm_smmu_device_remove,
Signature · 1 line
2.12.2.dirty
PATCH 2/2 iommu/arm-smmu: Add system PM support
Quoted 14 lines · Robin Murphy
With all our hardware state tracked in such a way that we can naturally restore it as part of the necessary reset, resuming is trivial, and there's nothing to do on suspend at all.
Signed-off-by Robin Murphy <[email protected]>--- drivers/iommu/arm-smmu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/iommu/arm-smmu.c +0
-0
drivers/iommu/arm-smmu.c static int arm_smmu_device_remove(struct platform_device *pdev) · line 2360
Quoted 5 lines
drivers/iommu/arm-smmu.c +2
-0@@ -2356,10 +2356,22 @@ static int arm_smmu_device_remove(struct platform_device *pdev)return 0 ; } +static int __maybe_unused arm_smmu_pm_resume (struct device *dev) +{
Did you actually get a warning here without the __maybe_unused annotation? It looks like some other drivers just guard the thing with CONFIG_PM_SLEEP.
Quoted 4 lines
drivers/iommu/arm-smmu.c +4
-0+struct arm_smmu_device *smmu =dev_get_drvdata (dev); + +arm_smmu_device_reset (smmu); +return 0 ;
drivers/iommu/arm-smmu.c static int arm_smmu_device_remove(struct platform_device *pdev) · line 2374
Quoted 10 lines
drivers/iommu/arm-smmu.c +6
-0+} + + +static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume); +static struct platform_driver arm_smmu_driver = { .driver = { .name ="arm-smmu" , .of_match_table =of_match_ptr (arm_smmu_of_match), + .pm = &arm_smmu_pm_ops,
Cosmetic: can you tab-align this assignment please?
Will
Robin Murphy
reviewerPATCH 2/2 iommu/arm-smmu: Add system PM support
Quoted 18 lines · Will Deacon
Quoted 17 lines · Robin Murphy
With all our hardware state tracked in such a way that we can naturally restore it as part of the necessary reset, resuming is trivial, and there's nothing to do on suspend at all.
Signed-off-by Robin Murphy <[email protected]>--- drivers/iommu/arm-smmu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/iommu/arm-smmu.c +0
-0@@ -2356,10 +2356,22 @@ static int arm_smmu_device_remove(struct platform_device *pdev)return 0 ; }
Quoted 5 lines
drivers/iommu/arm-smmu.c static int arm_smmu_device_remove(struct platform_device *pdev) · line 2360
Quoted 2 lines
drivers/iommu/arm-smmu.c +2
-0+static int __maybe_unused arm_smmu_pm_resume (struct device *dev) +{Did you actually get a warning here without the
__maybe_unusedannotation? It looks like some other drivers just guard the thing withCONFIG_PM_SLEEP.
I'm under the impression that the annotation is preferred over #ifdefs for new code (for the sake of coverage, I guess).
Quoted 8 lines
Quoted 8 lines
drivers/iommu/arm-smmu.c +8
-0+struct arm_smmu_device *smmu =dev_get_drvdata (dev); + +arm_smmu_device_reset (smmu); +return 0 ; +} + + +static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
Quoted 8 lines
drivers/iommu/arm-smmu.c static int arm_smmu_device_remove(struct platform_device *pdev) · line 2374
Quoted 6 lines
drivers/iommu/arm-smmu.c +2
-0+static struct platform_driver arm_smmu_driver = { .driver = { .name ="arm-smmu" , .of_match_table =of_match_ptr (arm_smmu_of_match), + .pm = &arm_smmu_pm_ops,Cosmetic: can you tab-align this assignment please?
Oops, I missed that - will do.
Robin.
PATCH 2/2 iommu/arm-smmu: Add system PM support
Quoted 19 lines · On Tue, 8 Aug 2017 13:14:18 +0100 Robin Murphy <[email protected]> wrote:
Quoted 18 lines · Will Deacon
Quoted 17 lines · Robin Murphy
With all our hardware state tracked in such a way that we can naturally restore it as part of the necessary reset, resuming is trivial, and there's nothing to do on suspend at all.
Signed-off-by Robin Murphy <[email protected]>--- drivers/iommu/arm-smmu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/iommu/arm-smmu.c +0
-0@@ -2356,10 +2356,22 @@ static int arm_smmu_device_remove(struct platform_device *pdev)return 0 ; }
Quoted 8 lines
Quoted 5 lines
drivers/iommu/arm-smmu.c static int arm_smmu_device_remove(struct platform_device *pdev) · line 2360
Quoted 2 lines
drivers/iommu/arm-smmu.c +2
-0+static int __maybe_unused arm_smmu_pm_resume (struct device *dev) +{Did you actually get a warning here without the
__maybe_unusedannotation? It looks like some other drivers just guard the thing withCONFIG_PM_SLEEP.I'm under the impression that the annotation is preferred over #ifdefs for new code (for the sake of coverage, I guess).
https://patchwork.kernel.org/patch/9734367/
Is a good thread discussing this. Both coverage and to avoid common pitfalls of the ifdef fun.
Jonathan
Quoted 26 lines
Quoted 17 lines
Quoted 5 lines
drivers/iommu/arm-smmu.c +4
-0+struct arm_smmu_device *smmu =dev_get_drvdata (dev); + +arm_smmu_device_reset (smmu); +return 0 ;drivers/iommu/arm-smmu.c static int arm_smmu_device_remove(struct platform_device *pdev) · line 2374
Quoted 10 lines
drivers/iommu/arm-smmu.c +6
-0+} + + +static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume); +static struct platform_driver arm_smmu_driver = { .driver = { .name ="arm-smmu" , .of_match_table =of_match_ptr (arm_smmu_of_match), + .pm = &arm_smmu_pm_ops,Cosmetic: can you tab-align this assignment please?
Oops, I missed that - will do.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/2 iommu/arm-smmu: Track context bank state
In reply to Robin MurphyHi Robin,
Quoted 21 lines · Robin Murphy
Echoing what we do for Stream Map Entries, maintain a software shadow state for context bank configuration. With this in place, we are mere moments away from blissfully easy suspend/resume support.
Signed-off-by Robin Murphy <[email protected]>---
Since the runtime PM discussion has come back again, I figured it was probably about time to finish off my plan for system PM. Lightly tested on Juno (MMU-401) with hibernation.
Robin.
drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++++------------------ 1 file changed, 97 insertions(+), 62 deletions(-)drivers/iommu/arm-smmu.c +0
-0
drivers/iommu/arm-smmu.c struct arm_smmu_smr { · line 347
Quoted 10 lines
drivers/iommu/arm-smmu.c +7
-0@@ -338,6 +338,13 @@ struct arm_smmu_smr {bool valid; }; +struct arm_smmu_cb { +u64 ttbr[2 ]; +u32 tcr[2 ]; +u32 mair[2 ]; +struct arm_smmu_cfg *cfg; +}; +
When i was trying this sometime back [1], was saving and using the pgtbl->cfg and domain->cfg for restoring the context. But this looks correct to make a actual shadow once and use it later all the time. Will also try some testing today.
Reviewed-by sricharan at codeaurora.org
[1] https://patchwork.kernel.org/patch/9389717/
Regards,
Sricharan
Quoted 248 lines
drivers/iommu/arm-smmu.c +90
-62struct arm_smmu_master_cfg {struct arm_smmu_device *smmu;s16 smendx[];@@ -380,6 +387,7 @@ struct arm_smmu_device {u32 num_context_banks;u32 num_s2_context_banks;DECLARE_BITMAP (context_map,ARM_SMMU_MAX_CBS ); +struct arm_smmu_cb *cbs;atomic_t irptndx;u32 num_mapping_groups;@@ -768,17 +776,69 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)static void arm_smmu_init_context_bank (struct arm_smmu_domain *smmu_domain,struct io_pgtable_cfg *pgtbl_cfg) { -u32 reg, reg2; -u64 reg64; -bool stage1;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -struct arm_smmu_device *smmu = smmu_domain->smmu; +struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx]; +bool stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; + + cb->cfg = cfg; + +/* TTBCR */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->tcr[0 ] = pgtbl_cfg->arm_v7s_cfg.tcr; + }else { + cb->tcr[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.tcr; + cb->tcr[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; + cb->tcr[1 ] |=TTBCR2_SEP_UPSTREAM ; +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) + cb->tcr[1 ] |=TTBCR2_AS ; + } + }else { + cb->tcr[0 ] = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; + } + +/* TTBRs */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->ttbr[0 ] = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; + cb->ttbr[1 ] = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; + }else { + cb->ttbr[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; + cb->ttbr[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; + } + }else { + cb->ttbr[0 ] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; + } + +/* MAIRs (stage-1 only) */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->mair[0 ] = pgtbl_cfg->arm_v7s_cfg.prrr; + cb->mair[1 ] = pgtbl_cfg->arm_v7s_cfg.nmrr; + }else { + cb->mair[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; + cb->mair[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; + } + } +} + +static void arm_smmu_write_context_bank (struct arm_smmu_device *smmu,int idx) +{ +u32 reg; +bool stage1; +struct arm_smmu_cb *cb = &smmu->cbs[idx]; +struct arm_smmu_cfg *cfg = cb->cfg; +struct arm_smmu_cfg default_cfg = {0 };void __iomem *cb_base, *gr1_base; +if (!cfg) + cfg = &default_cfg; + gr1_base =ARM_SMMU_GR1 (smmu); stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); + cb_base =ARM_SMMU_CB (smmu, idx); +/* CBA2R */ if (smmu->version >ARM_SMMU_V1 ) {if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) reg =CBA2R_RW64_64BIT ;@@ -788,7 +848,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,if (smmu->features &ARM_SMMU_FEAT_VMID16 ) reg |= cfg->vmid <<CBA2R_VMID_SHIFT ; -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (idx)); }/* CBAR */ @@ -807,72 +867,43 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,/* 8-bit VMIDs live in CBAR */ reg |= cfg->vmid <<CBAR_VMID_SHIFT ; } -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (idx));/* * TTBCR * We must write this before the TTBRs, since it determines the * access behaviour of some fields (in particular, ASID[15:8]). */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.tcr; - reg2 =0 ; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.tcr; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; - reg2 |=TTBCR2_SEP_UPSTREAM ; -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) - reg2 |=TTBCR2_AS ; - } -if (smmu->version >ARM_SMMU_V1 ) -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_TTBCR2 ); - }else { - reg = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBCR ); +if (stage1 && smmu->version >ARM_SMMU_V1 ) +writel_relaxed (cb->tcr[1 ], cb_base +ARM_SMMU_CB_TTBCR2 ); +writel_relaxed (cb->tcr[0 ], cb_base +ARM_SMMU_CB_TTBCR );/* TTBRs */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR0 ); - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR1 ); -writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); - }else { - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR1 ); - } +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { +writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); +writel_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writel_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }else { - reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }/* MAIRs (stage-1 only) */ if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.prrr; - reg2 = pgtbl_cfg->arm_v7s_cfg.nmrr; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_S1_MAIR0 ); -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_S1_MAIR1 ); +writel_relaxed (cb->mair[0 ], cb_base +ARM_SMMU_CB_S1_MAIR0 ); +writel_relaxed (cb->mair[1 ], cb_base +ARM_SMMU_CB_S1_MAIR1 ); }/* SCTLR */ - reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; -if (stage1) - reg |=SCTLR_S1_ASIDPNE ; -#ifdef __BIG_ENDIAN - reg |=SCTLR_E ; -#endif +if (!cb->cfg) { + reg =0 ; + }else { + reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; +if (stage1) + reg |=SCTLR_S1_ASIDPNE ; +if (IS_ENABLED (CONFIG_CPU_BIG_ENDIAN )) + reg |=SCTLR_E ; + }writel_relaxed (reg, cb_base +ARM_SMMU_CB_SCTLR ); }@@ -1035,6 +1066,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,/* Initialise the context bank with our page table cfg */ arm_smmu_init_context_bank (smmu_domain, &pgtbl_cfg); +arm_smmu_write_context_bank (smmu, cfg->cbndx);/* * Request context fault interrupt. Do this last to avoid the @@ -1067,7 +1099,6 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain);struct arm_smmu_device *smmu = smmu_domain->smmu;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -void __iomem *cb_base;int irq;if (!smmu || domain->type ==IOMMU_DOMAIN_IDENTITY )@@ -1077,8 +1108,8 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)* Disable the context bank and free the page tables before freeing * it. */ - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); + smmu->cbs[cfg->cbndx] = (struct arm_smmu_cb ){0 }; +arm_smmu_write_context_bank (smmu, cfg->cbndx);if (cfg->irptndx !=INVALID_IRPTNDX ) { irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];@@ -1722,7 +1753,6 @@ static struct iommu_ops arm_smmu_ops = {static void arm_smmu_device_reset (struct arm_smmu_device *smmu) {void __iomem *gr0_base =ARM_SMMU_GR0 (smmu); -void __iomem *cb_base;int i;u32 reg, major;@@ -1758,8 +1788,9 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)/* Make sure all context banks are disabled and clear CB_FSR */ for (i =0 ; i < smmu->num_context_banks; ++i) { - cb_base =ARM_SMMU_CB (smmu, i); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); +void __iomem *cb_base =ARM_SMMU_CB (smmu, i); + +arm_smmu_write_context_bank (smmu, i);writel_relaxed (FSR_FAULT , cb_base +ARM_SMMU_CB_FSR );/* * Disable MMU-500's not-particularly-beneficial next-page @@ -1964,6 +1995,10 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) smmu->cavium_id_base -= smmu->num_context_banks;dev_notice (smmu->dev,"\tenabling workaround for Cavium erratum 27704\n" ); } + smmu->cbs =devm_kcalloc (smmu->dev, smmu->num_context_banks, +sizeof (*smmu->cbs),GFP_KERNEL ); +if (!smmu->cbs) +return -ENOMEM ;/* ID2 */ id =readl_relaxed (gr0_base +ARM_SMMU_GR0_ID2 );
Signature · 5 lines
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Robin Murphy
reviewerPATCH 1/2 iommu/arm-smmu: Track context bank state
Quoted 27 lines · Sricharan R
Hi Robin,
Quoted 24 lines · Robin Murphy
Echoing what we do for Stream Map Entries, maintain a software shadow state for context bank configuration. With this in place, we are mere moments away from blissfully easy suspend/resume support.
Signed-off-by Robin Murphy <[email protected]>---
Since the runtime PM discussion has come back again, I figured it was probably about time to finish off my plan for system PM. Lightly tested on Juno (MMU-401) with hibernation.
Robin.
drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++++------------------ 1 file changed, 97 insertions(+), 62 deletions(-)drivers/iommu/arm-smmu.c +0
-0@@ -338,6 +338,13 @@ struct arm_smmu_smr {bool valid; };
Quoted 12 lines
drivers/iommu/arm-smmu.c struct arm_smmu_smr { · line 347
Quoted 7 lines
drivers/iommu/arm-smmu.c +7
-0+struct arm_smmu_cb { +u64 ttbr[2 ]; +u32 tcr[2 ]; +u32 mair[2 ]; +struct arm_smmu_cfg *cfg; +}; +When i was trying this sometime back [1], was saving and using the
pgtbl->cfganddomain->cfgfor restoring the context. But this looks correct to make a actual shadow once and use it later all the time.
Yeah, I did have a go at using the io_pgtable_cfg, but given the variable format of the registers involved it turned out simpler to shadow the raw register state directly.
Quoted 3 lines
Will also try some testing today.
Reviewed-by sricharan at codeaurora.org
Thanks!
Robin.
Quoted 255 lines
[1] https://patchwork.kernel.org/patch/9389717/
Regards,
SricharanQuoted 248 lines
drivers/iommu/arm-smmu.c +90
-62struct arm_smmu_master_cfg {struct arm_smmu_device *smmu;s16 smendx[];@@ -380,6 +387,7 @@ struct arm_smmu_device {u32 num_context_banks;u32 num_s2_context_banks;DECLARE_BITMAP (context_map,ARM_SMMU_MAX_CBS ); +struct arm_smmu_cb *cbs;atomic_t irptndx;u32 num_mapping_groups;@@ -768,17 +776,69 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)static void arm_smmu_init_context_bank (struct arm_smmu_domain *smmu_domain,struct io_pgtable_cfg *pgtbl_cfg) { -u32 reg, reg2; -u64 reg64; -bool stage1;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -struct arm_smmu_device *smmu = smmu_domain->smmu; +struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx]; +bool stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; + + cb->cfg = cfg; + +/* TTBCR */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->tcr[0 ] = pgtbl_cfg->arm_v7s_cfg.tcr; + }else { + cb->tcr[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.tcr; + cb->tcr[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; + cb->tcr[1 ] |=TTBCR2_SEP_UPSTREAM ; +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) + cb->tcr[1 ] |=TTBCR2_AS ; + } + }else { + cb->tcr[0 ] = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; + } + +/* TTBRs */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->ttbr[0 ] = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; + cb->ttbr[1 ] = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; + }else { + cb->ttbr[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; + cb->ttbr[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; + } + }else { + cb->ttbr[0 ] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; + } + +/* MAIRs (stage-1 only) */ +if (stage1) { +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { + cb->mair[0 ] = pgtbl_cfg->arm_v7s_cfg.prrr; + cb->mair[1 ] = pgtbl_cfg->arm_v7s_cfg.nmrr; + }else { + cb->mair[0 ] = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; + cb->mair[1 ] = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; + } + } +} + +static void arm_smmu_write_context_bank (struct arm_smmu_device *smmu,int idx) +{ +u32 reg; +bool stage1; +struct arm_smmu_cb *cb = &smmu->cbs[idx]; +struct arm_smmu_cfg *cfg = cb->cfg; +struct arm_smmu_cfg default_cfg = {0 };void __iomem *cb_base, *gr1_base; +if (!cfg) + cfg = &default_cfg; + gr1_base =ARM_SMMU_GR1 (smmu); stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); + cb_base =ARM_SMMU_CB (smmu, idx); +/* CBA2R */ if (smmu->version >ARM_SMMU_V1 ) {if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) reg =CBA2R_RW64_64BIT ;@@ -788,7 +848,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,if (smmu->features &ARM_SMMU_FEAT_VMID16 ) reg |= cfg->vmid <<CBA2R_VMID_SHIFT ; -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (idx)); }/* CBAR */ @@ -807,72 +867,43 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,/* 8-bit VMIDs live in CBAR */ reg |= cfg->vmid <<CBAR_VMID_SHIFT ; } -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (idx));/* * TTBCR * We must write this before the TTBRs, since it determines the * access behaviour of some fields (in particular, ASID[15:8]). */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.tcr; - reg2 =0 ; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.tcr; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; - reg2 |=TTBCR2_SEP_UPSTREAM ; -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) - reg2 |=TTBCR2_AS ; - } -if (smmu->version >ARM_SMMU_V1 ) -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_TTBCR2 ); - }else { - reg = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBCR ); +if (stage1 && smmu->version >ARM_SMMU_V1 ) +writel_relaxed (cb->tcr[1 ], cb_base +ARM_SMMU_CB_TTBCR2 ); +writel_relaxed (cb->tcr[0 ], cb_base +ARM_SMMU_CB_TTBCR );/* TTBRs */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR0 ); - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR1 ); -writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); - }else { - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR1 ); - } +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { +writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); +writel_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writel_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }else { - reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }/* MAIRs (stage-1 only) */ if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.prrr; - reg2 = pgtbl_cfg->arm_v7s_cfg.nmrr; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_S1_MAIR0 ); -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_S1_MAIR1 ); +writel_relaxed (cb->mair[0 ], cb_base +ARM_SMMU_CB_S1_MAIR0 ); +writel_relaxed (cb->mair[1 ], cb_base +ARM_SMMU_CB_S1_MAIR1 ); }/* SCTLR */ - reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; -if (stage1) - reg |=SCTLR_S1_ASIDPNE ; -#ifdef __BIG_ENDIAN - reg |=SCTLR_E ; -#endif +if (!cb->cfg) { + reg =0 ; + }else { + reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; +if (stage1) + reg |=SCTLR_S1_ASIDPNE ; +if (IS_ENABLED (CONFIG_CPU_BIG_ENDIAN )) + reg |=SCTLR_E ; + }writel_relaxed (reg, cb_base +ARM_SMMU_CB_SCTLR ); }@@ -1035,6 +1066,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,/* Initialise the context bank with our page table cfg */ arm_smmu_init_context_bank (smmu_domain, &pgtbl_cfg); +arm_smmu_write_context_bank (smmu, cfg->cbndx);/* * Request context fault interrupt. Do this last to avoid the @@ -1067,7 +1099,6 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain);struct arm_smmu_device *smmu = smmu_domain->smmu;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -void __iomem *cb_base;int irq;if (!smmu || domain->type ==IOMMU_DOMAIN_IDENTITY )@@ -1077,8 +1108,8 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)* Disable the context bank and free the page tables before freeing * it. */ - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); + smmu->cbs[cfg->cbndx] = (struct arm_smmu_cb ){0 }; +arm_smmu_write_context_bank (smmu, cfg->cbndx);if (cfg->irptndx !=INVALID_IRPTNDX ) { irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];@@ -1722,7 +1753,6 @@ static struct iommu_ops arm_smmu_ops = {static void arm_smmu_device_reset (struct arm_smmu_device *smmu) {void __iomem *gr0_base =ARM_SMMU_GR0 (smmu); -void __iomem *cb_base;int i;u32 reg, major;@@ -1758,8 +1788,9 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)/* Make sure all context banks are disabled and clear CB_FSR */ for (i =0 ; i < smmu->num_context_banks; ++i) { - cb_base =ARM_SMMU_CB (smmu, i); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); +void __iomem *cb_base =ARM_SMMU_CB (smmu, i); + +arm_smmu_write_context_bank (smmu, i);writel_relaxed (FSR_FAULT , cb_base +ARM_SMMU_CB_FSR );/* * Disable MMU-500's not-particularly-beneficial next-page @@ -1964,6 +1995,10 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) smmu->cavium_id_base -= smmu->num_context_banks;dev_notice (smmu->dev,"\tenabling workaround for Cavium erratum 27704\n" ); } + smmu->cbs =devm_kcalloc (smmu->dev, smmu->num_context_banks, +sizeof (*smmu->cbs),GFP_KERNEL ); +if (!smmu->cbs) +return -ENOMEM ;/* ID2 */ id =readl_relaxed (gr0_base +ARM_SMMU_GR0_ID2 );
PATCH 1/2 iommu/arm-smmu: Track context bank state
In reply to Robin MurphyHi Robin,
I like the idea, but I think there are a few minor issues with the patch. Comments below.
Quoted 15 lines · Robin Murphy
Echoing what we do for Stream Map Entries, maintain a software shadow state for context bank configuration. With this in place, we are mere moments away from blissfully easy suspend/resume support.
Signed-off-by Robin Murphy <[email protected]>---
Since the runtime PM discussion has come back again, I figured it was probably about time to finish off my plan for system PM. Lightly tested on Juno (MMU-401) with hibernation.
Robin.
drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++++------------------ 1 file changed, 97 insertions(+), 62 deletions(-)
[...]
Quoted 78 lines
drivers/iommu/arm-smmu.c +17
-34+static void arm_smmu_write_context_bank (struct arm_smmu_device *smmu,int idx) +{ +u32 reg; +bool stage1; +struct arm_smmu_cb *cb = &smmu->cbs[idx]; +struct arm_smmu_cfg *cfg = cb->cfg; +struct arm_smmu_cfg default_cfg = {0 };void __iomem *cb_base, *gr1_base; +if (!cfg) + cfg = &default_cfg; + gr1_base =ARM_SMMU_GR1 (smmu); stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); + cb_base =ARM_SMMU_CB (smmu, idx); +/* CBA2R */ if (smmu->version >ARM_SMMU_V1 ) {if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) reg =CBA2R_RW64_64BIT ;@@ -788,7 +848,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,if (smmu->features &ARM_SMMU_FEAT_VMID16 ) reg |= cfg->vmid <<CBA2R_VMID_SHIFT ; -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (idx)); }/* CBAR */ @@ -807,72 +867,43 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,/* 8-bit VMIDs live in CBAR */ reg |= cfg->vmid <<CBAR_VMID_SHIFT ; } -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (idx));/* * TTBCR * We must write this before the TTBRs, since it determines the * access behaviour of some fields (in particular, ASID[15:8]). */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.tcr; - reg2 =0 ; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.tcr; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; - reg2 |=TTBCR2_SEP_UPSTREAM ; -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) - reg2 |=TTBCR2_AS ; - } -if (smmu->version >ARM_SMMU_V1 ) -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_TTBCR2 ); - }else { - reg = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBCR ); +if (stage1 && smmu->version >ARM_SMMU_V1 ) +writel_relaxed (cb->tcr[1 ], cb_base +ARM_SMMU_CB_TTBCR2 ); +writel_relaxed (cb->tcr[0 ], cb_base +ARM_SMMU_CB_TTBCR );/* TTBRs */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR0 ); - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR1 ); -writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); - }else { - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR1 );
drivers/iommu/arm-smmu.c static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, · line 888
Quoted 10 lines
drivers/iommu/arm-smmu.c +6
-3- } +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { +writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); +writel_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writel_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }else { - reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 );
This doesn't look right to me. For the LPAE S1 case, we don't set the ASID afaict, and for the S2 case we write to a RESERVED register (since we only have one TTBR).
Quoted 16 lines
drivers/iommu/arm-smmu.c +2
-9}/* MAIRs (stage-1 only) */ if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.prrr; - reg2 = pgtbl_cfg->arm_v7s_cfg.nmrr; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_S1_MAIR0 ); -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_S1_MAIR1 ); +writel_relaxed (cb->mair[0 ], cb_base +ARM_SMMU_CB_S1_MAIR0 ); +writel_relaxed (cb->mair[1 ], cb_base +ARM_SMMU_CB_S1_MAIR1 ); }
drivers/iommu/arm-smmu.c static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, · line 899
Quoted 10 lines
drivers/iommu/arm-smmu.c +2
-6/* SCTLR */ - reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; -if (stage1) - reg |=SCTLR_S1_ASIDPNE ; -#ifdef __BIG_ENDIAN - reg |=SCTLR_E ; -#endif +if (!cb->cfg) { + reg =0 ;
I think this might be too late. See below.
Quoted 27 lines
drivers/iommu/arm-smmu.c +8
-1+ }else { + reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; +if (stage1) + reg |=SCTLR_S1_ASIDPNE ; +if (IS_ENABLED (CONFIG_CPU_BIG_ENDIAN )) + reg |=SCTLR_E ; + }writel_relaxed (reg, cb_base +ARM_SMMU_CB_SCTLR ); }@@ -1035,6 +1066,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,/* Initialise the context bank with our page table cfg */ arm_smmu_init_context_bank (smmu_domain, &pgtbl_cfg); +arm_smmu_write_context_bank (smmu, cfg->cbndx);/* * Request context fault interrupt. Do this last to avoid the @@ -1067,7 +1099,6 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain);struct arm_smmu_device *smmu = smmu_domain->smmu;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -void __iomem *cb_base;int irq;if (!smmu || domain->type ==IOMMU_DOMAIN_IDENTITY )
drivers/iommu/arm-smmu.c static void arm_smmu_destroy_domain_context(struct iommu_domain *domain) · line 1112
Quoted 7 lines
drivers/iommu/arm-smmu.c +2
-2@@ -1077,8 +1108,8 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)* Disable the context bank and free the page tables before freeing * it. */ - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); + smmu->cbs[cfg->cbndx] = (struct arm_smmu_cb ){0 }; +arm_smmu_write_context_bank (smmu, cfg->cbndx);
So here, we end up passing a zeroed structure to arm_smmu_write_context_bank, but afaict that will write to the TTBR and TCR before SCTLR, which worries me. Couldn't we get whacky speculative table walks through physical address 0 with ASID 0 before we kill the SCTLR?
If the only time we pass something with a NULL cfg is on the destroy path, perhaps just do:
if (!cfg) {
writel_relaxed(0, cb_base + ARM_SMMU_CB_SCTLR);
return;}
instead?
Will
Robin Murphy
reviewerPATCH 1/2 iommu/arm-smmu: Track context bank state
Quoted 108 lines · Will Deacon
Hi Robin,
I like the idea, but I think there are a few minor issues with the patch. Comments below.
Quoted 15 lines · Robin Murphy
Echoing what we do for Stream Map Entries, maintain a software shadow state for context bank configuration. With this in place, we are mere moments away from blissfully easy suspend/resume support.
Signed-off-by Robin Murphy <[email protected]>---
Since the runtime PM discussion has come back again, I figured it was probably about time to finish off my plan for system PM. Lightly tested on Juno (MMU-401) with hibernation.
Robin.
drivers/iommu/arm-smmu.c | 159 +++++++++++++++++++++++++++++------------------ 1 file changed, 97 insertions(+), 62 deletions(-)[...]
Quoted 84 lines
drivers/iommu/arm-smmu.c +21
-35+static void arm_smmu_write_context_bank (struct arm_smmu_device *smmu,int idx) +{ +u32 reg; +bool stage1; +struct arm_smmu_cb *cb = &smmu->cbs[idx]; +struct arm_smmu_cfg *cfg = cb->cfg; +struct arm_smmu_cfg default_cfg = {0 };void __iomem *cb_base, *gr1_base; +if (!cfg) + cfg = &default_cfg; + gr1_base =ARM_SMMU_GR1 (smmu); stage1 = cfg->cbar !=CBAR_TYPE_S2_TRANS ; - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); + cb_base =ARM_SMMU_CB (smmu, idx); +/* CBA2R */ if (smmu->version >ARM_SMMU_V1 ) {if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) reg =CBA2R_RW64_64BIT ;@@ -788,7 +848,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,if (smmu->features &ARM_SMMU_FEAT_VMID16 ) reg |= cfg->vmid <<CBA2R_VMID_SHIFT ; -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBA2R (idx)); }/* CBAR */ @@ -807,72 +867,43 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,/* 8-bit VMIDs live in CBAR */ reg |= cfg->vmid <<CBAR_VMID_SHIFT ; } -writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (cfg->cbndx)); +writel_relaxed (reg, gr1_base +ARM_SMMU_GR1_CBAR (idx));/* * TTBCR * We must write this before the TTBRs, since it determines the * access behaviour of some fields (in particular, ASID[15:8]). */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.tcr; - reg2 =0 ; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.tcr; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.tcr >>32 ; - reg2 |=TTBCR2_SEP_UPSTREAM ; -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH64 ) - reg2 |=TTBCR2_AS ; - } -if (smmu->version >ARM_SMMU_V1 ) -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_TTBCR2 ); - }else { - reg = pgtbl_cfg->arm_lpae_s2_cfg.vtcr; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBCR ); +if (stage1 && smmu->version >ARM_SMMU_V1 ) +writel_relaxed (cb->tcr[1 ], cb_base +ARM_SMMU_CB_TTBCR2 ); +writel_relaxed (cb->tcr[0 ], cb_base +ARM_SMMU_CB_TTBCR );/* TTBRs */ -if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[0 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR0 ); - reg = pgtbl_cfg->arm_v7s_cfg.ttbr[1 ]; -writel_relaxed (reg, cb_base +ARM_SMMU_CB_TTBR1 ); -writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); - }else { - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); - reg64 = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1 ]; - reg64 |= (u64 )cfg->asid << TTBRn_ASID_SHIFT; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR1 ); - } +if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { +writel_relaxed (cfg->asid, cb_base +ARM_SMMU_CB_CONTEXTIDR ); +writel_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writel_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 ); }else {
Quoted 8 lines
drivers/iommu/arm-smmu.c static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, · line 888
Quoted 4 lines
drivers/iommu/arm-smmu.c +2
-2- reg64 = pgtbl_cfg->arm_lpae_s2_cfg.vttbr; -writeq_relaxed (reg64, cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[0 ], cb_base +ARM_SMMU_CB_TTBR0 ); +writeq_relaxed (cb->ttbr[1 ], cb_base +ARM_SMMU_CB_TTBR1 );This doesn't look right to me. For the LPAE S1 case, we don't set the ASID afaict, and for the S2 case we write to a RESERVED register (since we only have one TTBR).
Oops, yes, arm_smmu_init_context_bank() has indeed forgotten to munge the ASID into cb->ttbr[0] for that case.
TTBR1, though, is not an oversight - architecturally it is UNK/SBZP in stage 2 contexts, so since we never read it and cb->ttbr[1] will always be zero for stage 2, this is a valid thing to do and helps keep the code simple.
Quoted 17 lines
Quoted 17 lines
drivers/iommu/arm-smmu.c +2
-9}/* MAIRs (stage-1 only) */ if (stage1) { -if (cfg->fmt ==ARM_SMMU_CTX_FMT_AARCH32_S ) { - reg = pgtbl_cfg->arm_v7s_cfg.prrr; - reg2 = pgtbl_cfg->arm_v7s_cfg.nmrr; - }else { - reg = pgtbl_cfg->arm_lpae_s1_cfg.mair[0 ]; - reg2 = pgtbl_cfg->arm_lpae_s1_cfg.mair[1 ]; - } -writel_relaxed (reg, cb_base +ARM_SMMU_CB_S1_MAIR0 ); -writel_relaxed (reg2, cb_base +ARM_SMMU_CB_S1_MAIR1 ); +writel_relaxed (cb->mair[0 ], cb_base +ARM_SMMU_CB_S1_MAIR0 ); +writel_relaxed (cb->mair[1 ], cb_base +ARM_SMMU_CB_S1_MAIR1 ); }
Quoted 11 lines
drivers/iommu/arm-smmu.c static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, · line 899
Quoted 9 lines
drivers/iommu/arm-smmu.c +2
-6/* SCTLR */ - reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; -if (stage1) - reg |=SCTLR_S1_ASIDPNE ; -#ifdef __BIG_ENDIAN - reg |=SCTLR_E ; -#endif +if (!cb->cfg) { + reg =0 ;I think this might be too late. See below.
Ha, this bit did feel slightly awkward, and you are indeed quite right...
Quoted 43 lines
Quoted 27 lines
drivers/iommu/arm-smmu.c +8
-1+ }else { + reg =SCTLR_CFIE |SCTLR_CFRE |SCTLR_AFE |SCTLR_TRE |SCTLR_M ; +if (stage1) + reg |=SCTLR_S1_ASIDPNE ; +if (IS_ENABLED (CONFIG_CPU_BIG_ENDIAN )) + reg |=SCTLR_E ; + }writel_relaxed (reg, cb_base +ARM_SMMU_CB_SCTLR ); }@@ -1035,6 +1066,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,/* Initialise the context bank with our page table cfg */ arm_smmu_init_context_bank (smmu_domain, &pgtbl_cfg); +arm_smmu_write_context_bank (smmu, cfg->cbndx);/* * Request context fault interrupt. Do this last to avoid the @@ -1067,7 +1099,6 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)struct arm_smmu_domain *smmu_domain =to_smmu_domain (domain);struct arm_smmu_device *smmu = smmu_domain->smmu;struct arm_smmu_cfg *cfg = &smmu_domain->cfg; -void __iomem *cb_base;int irq;if (!smmu || domain->type ==IOMMU_DOMAIN_IDENTITY )drivers/iommu/arm-smmu.c static void arm_smmu_destroy_domain_context(struct iommu_domain *domain) · line 1112
Quoted 7 lines
drivers/iommu/arm-smmu.c +2
-2@@ -1077,8 +1108,8 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)* Disable the context bank and free the page tables before freeing * it. */ - cb_base =ARM_SMMU_CB (smmu, cfg->cbndx); -writel_relaxed (0 , cb_base +ARM_SMMU_CB_SCTLR ); + smmu->cbs[cfg->cbndx] = (struct arm_smmu_cb ){0 }; +arm_smmu_write_context_bank (smmu, cfg->cbndx);So here, we end up passing a zeroed structure to
arm_smmu_write_context_bank, but afaict that will write to the TTBR and TCR before SCTLR, which worries me. Couldn't we get whacky speculative table walks through physical address 0 with ASID 0 before we kill the SCTLR?If the only time we pass something with a NULL cfg is on the destroy path, perhaps just do:
Quoted 6 lines
if (!cfg) { writel_relaxed(0, cb_base + ARM_SMMU_CB_SCTLR); return;}
instead?
We also hit all banks with a NULL cfg in the initial reset from arm_smmu_device_probe(), but that still has the same semantics as teardown - if the context isn't assigned to a domain, there's no point even touching any hardware registers other than clearing SCTLR, so there's no need for the whole default_cfg silliness at all. Thanks for helping me see the obvious!
Robin.