[PATCH v7 20/21] iommu/mediatek: Fix iova_to_phys PA start for 4GB mode

2019-06-10 Thread Yong Wu
In the 4GB mode, the physical address is remapped,

Here is the detailed remap relationship.
CPU PA ->HW PA
0x4000_  0x1_4000_ (Add bit32)
0x8000_  0x1_8000_ ...
0xc000_  0x1_c000_ ...
0x1__0x1__ (No change)

Thus, we always add bit32 for PA when entering mtk_iommu_map.
But in the iova_to_phys, the CPU don't need this bit32 if the
PA is from 0x1_4000_ to 0x1__.
This patch discards the bit32 in this iova_to_phys in the 4GB mode.

Fixes: 30e2fccf9512 ("iommu/mediatek: Enlarge the validate PA range
for 4GB mode")
Signed-off-by: Yong Wu 
---
 drivers/iommu/mtk_iommu.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 67cab2d..34f2e40 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -119,6 +119,19 @@ struct mtk_iommu_domain {
 
 static const struct iommu_ops mtk_iommu_ops;
 
+/*
+ * In M4U 4GB mode, the physical address is remapped as below:
+ *  CPU PA ->   M4U HW PA
+ *  0x4000_ 0x1_4000_ (Add bit32)
+ *  0x8000_ 0x1_8000_ ...
+ *  0xc000_ 0x1_c000_ ...
+ *  0x1__   0x1__ (No change)
+ *
+ * Thus, We always add BIT32 in the iommu_map and disable BIT32 if PA is >=
+ * 0x1_4000_ in the iova_to_phys.
+ */
+#define MTK_IOMMU_4GB_MODE_PA_14000 0x14000UL
+
 static LIST_HEAD(m4ulist); /* List all the M4U HWs */
 
 #define for_each_m4u(data) list_for_each_entry(data, &m4ulist, list)
@@ -415,6 +428,7 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
iommu_domain *domain,
  dma_addr_t iova)
 {
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+   struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
unsigned long flags;
phys_addr_t pa;
 
@@ -422,6 +436,10 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
iommu_domain *domain,
pa = dom->iop->iova_to_phys(dom->iop, iova);
spin_unlock_irqrestore(&dom->pgtlock, flags);
 
+   if (data->plat_data->has_4gb_mode && data->dram_is_4gb &&
+   pa >= MTK_IOMMU_4GB_MODE_PA_14000)
+   pa &= ~BIT_ULL(32);
+
return pa;
 }
 
-- 
1.9.1



Re: [PATCH v7 20/21] iommu/mediatek: Fix iova_to_phys PA start for 4GB mode

2019-06-18 Thread Matthias Brugger



On 10/06/2019 14:17, Yong Wu wrote:
> In the 4GB mode, the physical address is remapped,
> 
> Here is the detailed remap relationship.
> CPU PA ->HW PA
> 0x4000_  0x1_4000_ (Add bit32)
> 0x8000_  0x1_8000_ ...
> 0xc000_  0x1_c000_ ...
> 0x1__0x1__ (No change)
> 
> Thus, we always add bit32 for PA when entering mtk_iommu_map.
> But in the iova_to_phys, the CPU don't need this bit32 if the
> PA is from 0x1_4000_ to 0x1__.
> This patch discards the bit32 in this iova_to_phys in the 4GB mode.
> 
> Fixes: 30e2fccf9512 ("iommu/mediatek: Enlarge the validate PA range
> for 4GB mode")
> Signed-off-by: Yong Wu 
> ---
>  drivers/iommu/mtk_iommu.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 67cab2d..34f2e40 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -119,6 +119,19 @@ struct mtk_iommu_domain {
>  
>  static const struct iommu_ops mtk_iommu_ops;
>  
> +/*
> + * In M4U 4GB mode, the physical address is remapped as below:
> + *  CPU PA ->   M4U HW PA
> + *  0x4000_ 0x1_4000_ (Add bit32)
> + *  0x8000_ 0x1_8000_ ...
> + *  0xc000_ 0x1_c000_ ...
> + *  0x1__   0x1__ (No change)
> + *
> + * Thus, We always add BIT32 in the iommu_map and disable BIT32 if PA is >=
> + * 0x1_4000_ in the iova_to_phys.
> + */
> +#define MTK_IOMMU_4GB_MODE_PA_14000 0x14000UL
> +
>  static LIST_HEAD(m4ulist);   /* List all the M4U HWs */
>  
>  #define for_each_m4u(data)   list_for_each_entry(data, &m4ulist, list)
> @@ -415,6 +428,7 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
> iommu_domain *domain,
> dma_addr_t iova)
>  {
>   struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> + struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
>   unsigned long flags;
>   phys_addr_t pa;
>  
> @@ -422,6 +436,10 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
> iommu_domain *domain,
>   pa = dom->iop->iova_to_phys(dom->iop, iova);
>   spin_unlock_irqrestore(&dom->pgtlock, flags);
>  
> + if (data->plat_data->has_4gb_mode && data->dram_is_4gb &&
> + pa >= MTK_IOMMU_4GB_MODE_PA_14000)
> + pa &= ~BIT_ULL(32);
> +

Hm, I wonder if we could fix this as first patch in the series, especially 
before:
"[PATCH 06/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode"

This would make it easier for the stable maintainer to cherry-pick the fix.
Without 100% understanding the code, it seems suspicious to me, that you first
move the setting of the bit32 and bit33 into v7s and later explicitly clean the
bits here.

So my take on this is, that patch 6/21 introduced the regression you are trying
to fix here. As said that is speculation as I don't understand the code in its
whole.

Any clarification would be useful.

Regards,
Matthias

>   return pa;
>  }
>  
> 


Re: [PATCH v7 20/21] iommu/mediatek: Fix iova_to_phys PA start for 4GB mode

2019-06-20 Thread Yong Wu
On Tue, 2019-06-18 at 18:35 +0200, Matthias Brugger wrote:
> 
> On 10/06/2019 14:17, Yong Wu wrote:
> > In the 4GB mode, the physical address is remapped,
> > 
> > Here is the detailed remap relationship.
> > CPU PA ->HW PA
> > 0x4000_  0x1_4000_ (Add bit32)
> > 0x8000_  0x1_8000_ ...
> > 0xc000_  0x1_c000_ ...
> > 0x1__0x1__ (No change)
> > 
> > Thus, we always add bit32 for PA when entering mtk_iommu_map.
> > But in the iova_to_phys, the CPU don't need this bit32 if the
> > PA is from 0x1_4000_ to 0x1__.
> > This patch discards the bit32 in this iova_to_phys in the 4GB mode.
> > 
> > Fixes: 30e2fccf9512 ("iommu/mediatek: Enlarge the validate PA range
> > for 4GB mode")
> > Signed-off-by: Yong Wu 
> > ---
> >  drivers/iommu/mtk_iommu.c | 18 ++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 67cab2d..34f2e40 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -119,6 +119,19 @@ struct mtk_iommu_domain {
> >  
> >  static const struct iommu_ops mtk_iommu_ops;
> >  
> > +/*
> > + * In M4U 4GB mode, the physical address is remapped as below:
> > + *  CPU PA ->   M4U HW PA
> > + *  0x4000_ 0x1_4000_ (Add bit32)
> > + *  0x8000_ 0x1_8000_ ...
> > + *  0xc000_ 0x1_c000_ ...
> > + *  0x1__   0x1__ (No change)
> > + *
> > + * Thus, We always add BIT32 in the iommu_map and disable BIT32 if PA is >=
> > + * 0x1_4000_ in the iova_to_phys.
> > + */
> > +#define MTK_IOMMU_4GB_MODE_PA_14000 0x14000UL
> > +
> >  static LIST_HEAD(m4ulist); /* List all the M4U HWs */
> >  
> >  #define for_each_m4u(data) list_for_each_entry(data, &m4ulist, list)
> > @@ -415,6 +428,7 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
> > iommu_domain *domain,
> >   dma_addr_t iova)
> >  {
> > struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> > +   struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
> > unsigned long flags;
> > phys_addr_t pa;
> >  
> > @@ -422,6 +436,10 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct 
> > iommu_domain *domain,
> > pa = dom->iop->iova_to_phys(dom->iop, iova);
> > spin_unlock_irqrestore(&dom->pgtlock, flags);
> >  
> > +   if (data->plat_data->has_4gb_mode && data->dram_is_4gb &&
> > +   pa >= MTK_IOMMU_4GB_MODE_PA_14000)
> > +   pa &= ~BIT_ULL(32);
> > +
> 
> Hm, I wonder if we could fix this as first patch in the series, especially 
> before:
> "[PATCH 06/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode"

OK.

> 
> This would make it easier for the stable maintainer to cherry-pick the fix.
> Without 100% understanding the code, it seems suspicious to me, that you first
> move the setting of the bit32 and bit33 into v7s and later explicitly clean 
> the
> bits here.
> 
> So my take on this is, that patch 6/21 introduced the regression you are 
> trying
> to fix here. As said that is speculation as I don't understand the code in its
> whole.
> 
> Any clarification would be useful.

I guess the commit message in [06/21] will be helpful.

In the previous mt8173 and mt2712, the M4U HW support "4GB mode" in
which the range of dram is from 0x4000_ to 0x1_3fff_ and it was
remapped to 0x1__ ~0x1__(For readable, I have wrote the
re-map relationship into the code in this patch.). but mt8183 don't need
remap the dram address(0x4000_ ~ 0x3__).

In order to unify the code, we add bit32 for "4GB mode". But actually
the PA doesn't always have bit32, thus, I have to remove bit32 when PA >
0x1_4000_.

So sorry that the "4GB mode" is a little unreadable and special, And the
4GB patch(30e2fccf9512 ("iommu/mediatek: Enlarge the validate PA range
for 4GB mode") has introduced several fix patches.

> 
> Regards,
> Matthias
> 
> > return pa;
> >  }
> >  
> > 
> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek