Re: [PATCH 1/1] iommu/arm-smmu: Add support to use Last level cache
Hi Robin, On Fri, Dec 7, 2018 at 2:54 PM Vivek Gautam wrote: > > Hi Robin, > > On Tue, Dec 4, 2018 at 8:51 PM Robin Murphy wrote: > > > > On 04/12/2018 11:01, Vivek Gautam wrote: > > > Qualcomm SoCs have an additional level of cache called as > > > System cache, aka. Last level cache (LLC). This cache sits right > > > before the DDR, and is tightly coupled with the memory controller. > > > The cache is available to all the clients present in the SoC system. > > > The clients request their slices from this system cache, make it > > > active, and can then start using it. > > > For these clients with smmu, to start using the system cache for > > > buffers and, related page tables [1], memory attributes need to be > > > set accordingly. > > > This change updates the MAIR and TCR configurations with correct > > > attributes to use this system cache. > > > > > > To explain a little about memory attribute requirements here: > > > > > > Non-coherent I/O devices can't look-up into inner caches. However, > > > coherent I/O devices can. But both can allocate in the system cache > > > based on system policy and configured memory attributes in page > > > tables. > > > CPUs can access both inner and outer caches (including system cache, > > > aka. Last level cache), and can allocate into system cache too > > > based on memory attributes, and system policy. > > > > > > Further looking at memory types, we have following - > > > a) Normal uncached :- MAIR 0x44, inner non-cacheable, > > >outer non-cacheable; > > > b) Normal cached :- MAIR 0xff, inner read write-back non-transient, > > >outer read write-back non-transient; > > >attribute setting for coherenet I/O devices. > > > > > > and, for non-coherent i/o devices that can allocate in system cache > > > another type gets added - > > > c) Normal sys-cached/non-inner-cached :- > > >MAIR 0xf4, inner non-cacheable, > > >outer read write-back non-transient > > > > > > So, CPU will automatically use the system cache for memory marked as > > > normal cached. The normal sys-cached is downgraded to normal non-cached > > > memory for CPUs. > > > Coherent I/O devices can use system cache by marking the memory as > > > normal cached. > > > Non-coherent I/O devices, to use system cache, should mark the memory as > > > normal sys-cached in page tables. > > > > > > This change is a realisation of following changes > > > from downstream msm-4.9: > > > iommu: io-pgtable-arm: Support DOMAIN_ATTRIBUTE_USE_UPSTREAM_HINT[2] > > > iommu: io-pgtable-arm: Implement IOMMU_USE_UPSTREAM_HINT[3] > > > > > > [1] https://patchwork.kernel.org/patch/10302791/ > > > [2] > > > https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/?h=msm-4.9=bf762276796e79ca90014992f4d9da5593fa7d51 > > > [3] > > > https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/?h=msm-4.9=d4c72c413ea27c43f60825193d4de9cb8ffd9602 > > > > > > Signed-off-by: Vivek Gautam > > > --- > > > > > > Changes since v1: > > > - Addressed Tomasz's comments for basing the change on > > > "NO_INNER_CACHE" concept for non-coherent I/O devices > > > rather than capturing "SYS_CACHE". This is to indicate > > > clearly the intent of non-coherent I/O devices that > > > can't access inner caches. > > > > That seems backwards to me - there is already a fundamental assumption > > that non-coherent devices can't access caches. What we're adding here is > > a weird exception where they *can* use some level of cache despite still > > being non-coherent overall. > > > > In other words, it's not a case of downgrading coherent devices' > > accesses to bypass inner caches, it's upgrading non-coherent devices' > > accesses to hit the outer cache. That's certainly the understanding I > > got from talking with Pratik at Plumbers, and it does appear to fit with > > your explanation above despite the final conclusion you draw being > > different. > > Thanks for the thorough review of the change. > Right, I guess it's rather an upgrade for non-coherent devices to use > an outer cache than a downgrade for coherent devices. > > > > > I do see what Tomasz meant in terms of the TCR attributes, but what we > > currently do there is a little unintuitive and not at all representative > > of actual mapping attributes - I'll come back to that inline. > > > > > drivers/iommu/arm-smmu.c | 15 +++ > > > drivers/iommu/dma-iommu.c | 3 +++ > > > drivers/iommu/io-pgtable-arm.c | 22 +- > > > drivers/iommu/io-pgtable.h | 5 + > > > include/linux/iommu.h | 3 +++ > > > 5 files changed, 43 insertions(+), 5 deletions(-) > > > > As a minor nit, I'd prefer this as at least two patches to separate the > > io-pgtable changes and arm-smmu changes - basically I'd expect it to > > look much the same as the non-strict mode support did. > > Sure, will split the patch. > >
Re: [PATCH 1/1] iommu/arm-smmu: Add support to use Last level cache
On Thu, Dec 13, 2018 at 9:20 AM Tomasz Figa wrote: > > On Fri, Dec 7, 2018 at 6:25 PM Vivek Gautam > wrote: > > > > Hi Robin, > > > > On Tue, Dec 4, 2018 at 8:51 PM Robin Murphy wrote: > > > > > > On 04/12/2018 11:01, Vivek Gautam wrote: > > > > Qualcomm SoCs have an additional level of cache called as > > > > System cache, aka. Last level cache (LLC). This cache sits right > > > > before the DDR, and is tightly coupled with the memory controller. > > > > The cache is available to all the clients present in the SoC system. > > > > The clients request their slices from this system cache, make it > > > > active, and can then start using it. > > > > For these clients with smmu, to start using the system cache for > > > > buffers and, related page tables [1], memory attributes need to be > > > > set accordingly. > > > > This change updates the MAIR and TCR configurations with correct > > > > attributes to use this system cache. > > > > > > > > To explain a little about memory attribute requirements here: > > > > > > > > Non-coherent I/O devices can't look-up into inner caches. However, > > > > coherent I/O devices can. But both can allocate in the system cache > > > > based on system policy and configured memory attributes in page > > > > tables. > > > > CPUs can access both inner and outer caches (including system cache, > > > > aka. Last level cache), and can allocate into system cache too > > > > based on memory attributes, and system policy. > > > > > > > > Further looking at memory types, we have following - > > > > a) Normal uncached :- MAIR 0x44, inner non-cacheable, > > > >outer non-cacheable; > > > > b) Normal cached :- MAIR 0xff, inner read write-back non-transient, > > > >outer read write-back non-transient; > > > >attribute setting for coherenet I/O devices. > > > > > > > > and, for non-coherent i/o devices that can allocate in system cache > > > > another type gets added - > > > > c) Normal sys-cached/non-inner-cached :- > > > >MAIR 0xf4, inner non-cacheable, > > > >outer read write-back non-transient > > > > > > > > So, CPU will automatically use the system cache for memory marked as > > > > normal cached. The normal sys-cached is downgraded to normal non-cached > > > > memory for CPUs. > > > > Coherent I/O devices can use system cache by marking the memory as > > > > normal cached. > > > > Non-coherent I/O devices, to use system cache, should mark the memory as > > > > normal sys-cached in page tables. > > > > > > > > This change is a realisation of following changes > > > > from downstream msm-4.9: > > > > iommu: io-pgtable-arm: Support DOMAIN_ATTRIBUTE_USE_UPSTREAM_HINT[2] > > > > iommu: io-pgtable-arm: Implement IOMMU_USE_UPSTREAM_HINT[3] > > > > > > > > [1] https://patchwork.kernel.org/patch/10302791/ > > > > [2] > > > > https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/?h=msm-4.9=bf762276796e79ca90014992f4d9da5593fa7d51 > > > > [3] > > > > https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/?h=msm-4.9=d4c72c413ea27c43f60825193d4de9cb8ffd9602 > > > > > > > > Signed-off-by: Vivek Gautam > > > > --- > > > > > > > > Changes since v1: > > > > - Addressed Tomasz's comments for basing the change on > > > > "NO_INNER_CACHE" concept for non-coherent I/O devices > > > > rather than capturing "SYS_CACHE". This is to indicate > > > > clearly the intent of non-coherent I/O devices that > > > > can't access inner caches. > > > > > > That seems backwards to me - there is already a fundamental assumption > > > that non-coherent devices can't access caches. What we're adding here is > > > a weird exception where they *can* use some level of cache despite still > > > being non-coherent overall. > > > > > > In other words, it's not a case of downgrading coherent devices' > > > accesses to bypass inner caches, it's upgrading non-coherent devices' > > > accesses to hit the outer cache. That's certainly the understanding I > > > got from talking with Pratik at Plumbers, and it does appear to fit with > > > your explanation above despite the final conclusion you draw being > > > different. > > > > Thanks for the thorough review of the change. > > Right, I guess it's rather an upgrade for non-coherent devices to use > > an outer cache than a downgrade for coherent devices. > > > > Note that it was not my suggestion to use "NO_INNER_CACHE" for > enabling the system cache, sorry for not being clear. What I was > asking for in my comment was regarding the previous patch disabling > inner cache if system cache is requested, which may not make for > coherent devices, which could benefit from using both inner and system > cache. Sorry for not taking the cue correctly. The intention of the change was to let coherent devices use system cache as well. But I guess the change wasn't designed correctly. > > So note that there are several cases here: > -
Re: [PATCH v5 18/20] iommu/mediatek: Fix VLD_PA_RANGE register backup when suspend
On Tue, Jan 1, 2019 at 11:59 AM Yong Wu wrote: > > The register VLD_PA_RNG(0x118) was forgot to backup while adding 4GB > mode support for mt2712. this patch add it. > > Fixes: 30e2fccf9512 ("iommu/mediatek: Enlarge the validate PA range > for 4GB mode") > Signed-off-by: Yong Wu > --- > drivers/iommu/mtk_iommu.c | 2 ++ > drivers/iommu/mtk_iommu.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 7fcef19..ddf1969 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -716,6 +716,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device > *dev) > reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0); > reg->int_main_control = readl_relaxed(base + > REG_MMU_INT_MAIN_CONTROL); > reg->ivrp_paddr = readl_relaxed(base + REG_MMU_IVRP_PADDR); > + reg->vld_pa_range = readl_relaxed(base + REG_MMU_VLD_PA_RNG); Don't we want to add: if (data->plat_data->vld_pa_rng) before this save/restore operation? Or it doesn't matter? > clk_disable_unprepare(data->bclk); > return 0; > } > @@ -740,6 +741,7 @@ static int __maybe_unused mtk_iommu_resume(struct device > *dev) > writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0); > writel_relaxed(reg->int_main_control, base + > REG_MMU_INT_MAIN_CONTROL); > writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR); > + writel_relaxed(reg->vld_pa_range, base + REG_MMU_VLD_PA_RNG); > if (m4u_dom) > writel(m4u_dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK, >base + REG_MMU_PT_BASE_ADDR); > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index 0a7c463..c500bfd 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -33,6 +33,7 @@ struct mtk_iommu_suspend_reg { > u32 int_control0; > u32 int_main_control; > u32 ivrp_paddr; > + u32 vld_pa_range; Well, please be consistent ,-) Either vld_pa_rng, or valid_pa_range ,-) > }; > > enum mtk_iommu_plat { > -- > 1.9.1 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 11/20] iommu/mediatek: Move vld_pa_rng into plat_data
On Tue, Jan 1, 2019 at 11:58 AM Yong Wu wrote: > > Both mt8173 and mt8183 don't have this vld_pa_rng(valid physical address > range) register while mt2712 have. Move it into the plat_data. > > Signed-off-by: Yong Wu > --- > drivers/iommu/mtk_iommu.c | 3 ++- > drivers/iommu/mtk_iommu.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 8d8ab21..2913ddb 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -548,7 +548,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > upper_32_bits(data->protect_base); > writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR); > > - if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) { > + if (data->enable_4GB && data->plat_data->vld_pa_rng) { > /* > * If 4GB mode is enabled, the validate PA range is from > * 0x1__ to 0x1__. here record bit[32:30]. > @@ -741,6 +741,7 @@ static int __maybe_unused mtk_iommu_resume(struct device > *dev) > .m4u_plat = M4U_MT2712, > .has_4gb_mode = true, > .has_bclk = true, > + .vld_pa_rng = true, > .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, > }; > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index b46aeaa..a8c5d1e 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -48,6 +48,7 @@ struct mtk_iommu_plat_data { > /* HW will use the EMI clock if there isn't the "bclk". */ > boolhas_bclk; > boolreset_axi; > + boolvld_pa_rng; Since this is not a register name, maybe we can use something more readable, like valid_pa_range? (or at the very least describe it in a comment in the struct?) > unsigned char larbid_remap[MTK_LARB_NR_MAX]; > }; > > -- > 1.9.1 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 10/20] iommu/mediatek: Move reset_axi into plat_data
On Tue, Jan 1, 2019 at 11:58 AM Yong Wu wrote: > > In mt8173 and mt8183, 0x48 is REG_MMU_STANDARD_AXI_MODE while > it is extended to REG_MMU_CTRL which contains _STANDARD_AXI_MODE in > the other SoCs. I move this property to plat_data since both mt8173 > and mt8183 use this property. > > It is a preparing patch for mt8183. > > Signed-off-by: Yong Wu Reviewed-by: Nicolas Boichat > --- > drivers/iommu/mtk_iommu.c | 4 ++-- > drivers/iommu/mtk_iommu.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 35a1263..8d8ab21 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -558,8 +558,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > } > writel_relaxed(0, data->base + REG_MMU_DCM_DIS); > > - /* It's MISC control register whose default value is ok except > mt8173.*/ > - if (data->plat_data->m4u_plat == M4U_MT8173) > + if (data->plat_data->reset_axi) > writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE); > > if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0, > @@ -749,6 +748,7 @@ static int __maybe_unused mtk_iommu_resume(struct device > *dev) > .m4u_plat = M4U_MT8173, > .has_4gb_mode = true, > .has_bclk = true, > + .reset_axi= true, > .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */ > }; > > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index eec19a6..b46aeaa 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -47,7 +47,7 @@ struct mtk_iommu_plat_data { > > /* HW will use the EMI clock if there isn't the "bclk". */ > boolhas_bclk; > - > + boolreset_axi; > unsigned char larbid_remap[MTK_LARB_NR_MAX]; > }; > > -- > 1.9.1 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 09/20] iommu/mediatek: Refine protect memory definition
On Tue, Jan 1, 2019 at 11:58 AM Yong Wu wrote: > > The protect memory setting is a little different in the different SoCs. > In the register REG_MMU_CTRL_REG(0x110), the TF_PROT(translation fault > protect) shift bit is normally 4 while it shift 5 bits only in the > mt8173. This patch delete the complex MACRO and use a common if-else > instead. > > Also, use "F_MMU_TF_PROT_TO_PROGRAM_ADDR" instead of the hard code(2) > which means the M4U will output the dirty data to the programmed > address that we allocated dynamically when translation fault occurs. > > Signed-off-by: Yong Wu > --- > @Nicalos, I don't put it in the plat_data since only the previous mt8173 > shift 5. As I know, the latest SoC always use the new setting like mt2712 > and mt8183. Thus, I think it is unnecessary to put it in plat_data and > let all the latest SoC set it. Hence, I still keep "== mt8173" for this > like the reg REG_MMU_CTRL_REG. Should be ok this way. But maybe one way to avoid hard-coding 4/5 below is to have 2 macros: #define F_MMU_TF_PROT_TO_PROGRAM_ADDR (2 << 4) #define F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173 (2 << 5) And still use the if below? > --- > drivers/iommu/mtk_iommu.c | 12 +--- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index eca1536..35a1263 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -53,11 +53,7 @@ > > #define REG_MMU_CTRL_REG 0x110 > #define F_MMU_PREFETCH_RT_REPLACE_MOD BIT(4) > -#define F_MMU_TF_PROTECT_SEL_SHIFT(data) \ > - ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5) > -/* It's named by F_MMU_TF_PROT_SEL in mt2712. */ > -#define F_MMU_TF_PROTECT_SEL(prot, data) \ > - (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data)) > +#define F_MMU_TF_PROT_TO_PROGRAM_ADDR 2 > > #define REG_MMU_IVRP_PADDR 0x114 > > @@ -521,9 +517,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data > *data) > return ret; > } > > - regval = F_MMU_TF_PROTECT_SEL(2, data); > if (data->plat_data->m4u_plat == M4U_MT8173) > - regval |= F_MMU_PREFETCH_RT_REPLACE_MOD; > + regval = F_MMU_PREFETCH_RT_REPLACE_MOD | > + (F_MMU_TF_PROT_TO_PROGRAM_ADDR << 5); > + else > + regval = F_MMU_TF_PROT_TO_PROGRAM_ADDR << 4; > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG); > > regval = F_L2_MULIT_HIT_EN | > -- > 1.9.1 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 08/20] iommu/mediatek: Add larb-id remapped support
On Tue, Jan 1, 2019 at 11:58 AM Yong Wu wrote: > > The larb-id may be remapped in the smi-common, this means the > larb-id reported in the mtk_iommu_isr isn't the real larb-id, > > Take mt8183 as a example: >M4U > | > - > | SMI common | > -0-7-5-6-1-2--3-4- <- Id remapped > | | | | | | | | > larb0 larb1 IPU0 IPU1 larb4 larb5 larb6 CCU > disp vdec img cam venc imgcam > As above, larb0 connects with the id 0 in smi-common. > larb1 connects with the id 7 in smi-common. > ... > If the larb-id reported in the isr is 7, actually it's larb1(vdec). > In order to output the right larb-id in the isr, we add a larb-id > remapping relationship in this patch. > > If there is no this larb-id remapping in some SoCs, use the linear > mapping array instead. > > This also is a preparing patch for mt8183. > > Signed-off-by: Yong Wu I think it's a little cleaner this way, thanks. Reviewed-by: Nicolas Boichat > --- > drivers/iommu/mtk_iommu.c | 4 > drivers/iommu/mtk_iommu.h | 2 ++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 847082c..eca1536 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -220,6 +220,8 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id) > fault_larb = F_MMU0_INT_ID_LARB_ID(regval); > fault_port = F_MMU0_INT_ID_PORT_ID(regval); > > + fault_larb = data->plat_data->larbid_remap[fault_larb]; > + > if (report_iommu_fault(>domain, data->dev, fault_iova, >write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) > { > dev_err_ratelimited( > @@ -742,12 +744,14 @@ static int __maybe_unused mtk_iommu_resume(struct > device *dev) > .m4u_plat = M4U_MT2712, > .has_4gb_mode = true, > .has_bclk = true, > + .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, > }; > > static const struct mtk_iommu_plat_data mt8173_data = { > .m4u_plat = M4U_MT8173, > .has_4gb_mode = true, > .has_bclk = true, > + .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */ > }; > > static const struct of_device_id mtk_iommu_of_ids[] = { > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h > index b8749ac..eec19a6 100644 > --- a/drivers/iommu/mtk_iommu.h > +++ b/drivers/iommu/mtk_iommu.h > @@ -47,6 +47,8 @@ struct mtk_iommu_plat_data { > > /* HW will use the EMI clock if there isn't the "bclk". */ > boolhas_bclk; > + > + unsigned char larbid_remap[MTK_LARB_NR_MAX]; > }; > > struct mtk_iommu_domain; > -- > 1.9.1 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [git pull] IOMMU Updates for Linux v4.21
The pull request you sent on Mon, 31 Dec 2018 17:36:58 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git > tags/iommu-updates-v4.21 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/8e143b90e4d45cca3dc53760d3cfab988bc74571 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu