Re: [PATCH v9 1/3] iommu/io-pgtable-arm-v7s: Add a quirk to allow pgtable PA up to 35bit

2022-06-16 Thread yf.wang--- via iommu
On Wed, 2022-06-15 at 18:03 +0100, Robin Murphy wrote:
> On 2022-06-15 17:12, yf.w...@mediatek.com wrote:
> > 
> >   static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
> >   struct io_pgtable_cfg *cfg)
> >   {
> > @@ -240,10 +245,17 @@ static void *__arm_v7s_alloc_table(int lvl,
> > gfp_t gfp,
> > dma_addr_t dma;
> > size_t size = ARM_V7S_TABLE_SIZE(lvl, cfg);
> > void *table = NULL;
> > +   gfp_t gfp_l1;
> > +
> > +   /*
> > +* ARM_MTK_TTBR_EXT extend the translation table base support
> > all
> > +* memory address.
> > +*/
> > +   gfp_l1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
> > +GFP_KERNEL : ARM_V7S_TABLE_GFP_DMA;
> >   
> > if (lvl == 1)
> > -   table = (void *)__get_free_pages(
> > -   __GFP_ZERO | ARM_V7S_TABLE_GFP_DMA,
> > get_order(size));
> > +   table = (void *)__get_free_pages(gfp_l1 | __GFP_ZERO,
> > get_order(size));
> > else if (lvl == 2)
> > table = kmem_cache_zalloc(data->l2_tables, gfp);
> >   
> > @@ -251,7 +263,8 @@ static void *__arm_v7s_alloc_table(int lvl,
> > gfp_t gfp,
> > return NULL;
> >   
> > phys = virt_to_phys(table);
> > -   if (phys != (arm_v7s_iopte)phys) {
> > +   if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
> > +   phys >= (1ULL << cfg->oas) : phys != (arm_v7s_iopte)phys) {
> 
> Given that the comment above says it supports all of memory, how
> would 
> phys >= (1ULL << cfg->oas) ever be true?
> 

Hi Robin,

Since Mediatek IOMMU hardware support at most 35bit PA in pgtable,
so add a quirk to allow the PA of pgtables support up to bit35,
but need to check oas do error hanlde.


> > /* Doesn't fit in PTE */
> > dev_err(dev, "Page table does not fit in PTE: %pa",
> > );
> > goto out_free;
> > arm_v7s_install_table(arm_v7s_iopte *table,
> >arm_v7s_iopte curr,
> >struct io_pgtable_cfg *cfg)

...
 
> > diff --git a/include/linux/io-pgtable.h b/include/linux/io-
> > pgtable.h
> > index 86af6f0a00a2..c9189716f6bd 100644
> > --- a/include/linux/io-pgtable.h
> > +++ b/include/linux/io-pgtable.h
> > @@ -74,17 +74,22 @@ struct io_pgtable_cfg {
> >  *  to support up to 35 bits PA where the bit32, bit33 and
> > bit34 are
> >  *  encoded in the bit9, bit4 and bit5 of the PTE respectively.
> >  *
> > +* IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT: (ARM v7s format) MediaTek
> > IOMMUs
> > +*  extend the translation table base support up to 35 bits PA,
> > the
> > +*  encoding format is same with IO_PGTABLE_QUIRK_ARM_MTK_EXT.
> > +*
> >  * IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the
> > table
> >  *  for use in the upper half of a split address space.
> >  *
> >  * IO_PGTABLE_QUIRK_ARM_OUTER_WBWA: Override the outer-
> > cacheability
> >  *  attributes set in the TCR for a non-coherent page-table
> > walker.
> >  */
> > -   #define IO_PGTABLE_QUIRK_ARM_NS BIT(0)
> > -   #define IO_PGTABLE_QUIRK_NO_PERMS   BIT(1)
> > -   #define IO_PGTABLE_QUIRK_ARM_MTK_EXTBIT(3)
> > -   #define IO_PGTABLE_QUIRK_ARM_TTBR1  BIT(5)
> > -   #define IO_PGTABLE_QUIRK_ARM_OUTER_WBWA BIT(6)
> > +   #define IO_PGTABLE_QUIRK_ARM_NS BIT(0)
> > +   #define IO_PGTABLE_QUIRK_NO_PERMS   BIT(1)
> > +   #define IO_PGTABLE_QUIRK_ARM_MTK_EXTBIT(3)
> > +   #define IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT   BIT(4)
> > +   #define IO_PGTABLE_QUIRK_ARM_TTBR1  BIT(5)
> > +   #define IO_PGTABLE_QUIRK_ARM_OUTER_WBWA BIT(6)
> > unsigned long   quirks;
> > unsigned long   pgsize_bitmap;
> > unsigned intias;
> > @@ -122,7 +127,7 @@ struct io_pgtable_cfg {
> > } arm_lpae_s2_cfg;
> >   
> > struct {
> > -   u32 ttbr;
> > +   u64 ttbr;
> 
> The point of this is to return an encoded TTBR register value, not a
> raw 
> base address. I see from the other patches that your register is
> still 
> 32 bits, so I'd prefer to follow the standard pattern and not need
> this 
> change.
> 
> Thanks,
> Robin.
> 

Hi Robin,
Thanks for your suggestion, next version will recovery ttbr to 32 bits,
will modify arm_v7s_alloc_pgtable to return an encoded TTBR, encoded
PA bits[34:32] to to lower bits, as follows:

/* TTBR */
phys_addr_t paddr;
paddr = virt_to_phys(data->pgd);
cfg->arm_v7s_cfg.ttbr = virt_to_phys(data->pgd) | ARM_V7S_TTBR_S |
(cfg->coherent_walk ? (ARM_V7S_TTBR_NOS |
 ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |
 ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) :
(ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) |
 ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC)));

if (cfg->quirks & 

Re: [PATCH v9 1/3] iommu/io-pgtable-arm-v7s: Add a quirk to allow pgtable PA up to 35bit

2022-06-15 Thread Robin Murphy

On 2022-06-15 17:12, yf.w...@mediatek.com wrote:

From: Yunfei Wang 

Single memory zone feature will remove ZONE_DMA32 and ZONE_DMA and
cause pgtable PA size larger than 32bit.

Since Mediatek IOMMU hardware support at most 35bit PA in pgtable,
so add a quirk to allow the PA of pgtables support up to bit35.

Signed-off-by: Ning Li 
Signed-off-by: Yunfei Wang 
---
  drivers/iommu/io-pgtable-arm-v7s.c | 58 +++---
  include/linux/io-pgtable.h | 17 +
  2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c 
b/drivers/iommu/io-pgtable-arm-v7s.c
index be066c1503d3..39e5503ac75a 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -182,14 +182,8 @@ static bool arm_v7s_is_mtk_enabled(struct io_pgtable_cfg 
*cfg)
(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT);
  }
  
-static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int lvl,

-   struct io_pgtable_cfg *cfg)
+static arm_v7s_iopte to_mtk_iopte(phys_addr_t paddr, arm_v7s_iopte pte)
  {
-   arm_v7s_iopte pte = paddr & ARM_V7S_LVL_MASK(lvl);
-
-   if (!arm_v7s_is_mtk_enabled(cfg))
-   return pte;
-
if (paddr & BIT_ULL(32))
pte |= ARM_V7S_ATTR_MTK_PA_BIT32;
if (paddr & BIT_ULL(33))
@@ -199,6 +193,17 @@ static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int 
lvl,
return pte;
  }
  
+static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int lvl,

+   struct io_pgtable_cfg *cfg)
+{
+   arm_v7s_iopte pte = paddr & ARM_V7S_LVL_MASK(lvl);
+
+   if (arm_v7s_is_mtk_enabled(cfg))
+   return to_mtk_iopte(paddr, pte);
+
+   return pte;
+}
+
  static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
  struct io_pgtable_cfg *cfg)
  {
@@ -240,10 +245,17 @@ static void *__arm_v7s_alloc_table(int lvl, gfp_t gfp,
dma_addr_t dma;
size_t size = ARM_V7S_TABLE_SIZE(lvl, cfg);
void *table = NULL;
+   gfp_t gfp_l1;
+
+   /*
+* ARM_MTK_TTBR_EXT extend the translation table base support all
+* memory address.
+*/
+   gfp_l1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
+GFP_KERNEL : ARM_V7S_TABLE_GFP_DMA;
  
  	if (lvl == 1)

-   table = (void *)__get_free_pages(
-   __GFP_ZERO | ARM_V7S_TABLE_GFP_DMA, get_order(size));
+   table = (void *)__get_free_pages(gfp_l1 | __GFP_ZERO, 
get_order(size));
else if (lvl == 2)
table = kmem_cache_zalloc(data->l2_tables, gfp);
  
@@ -251,7 +263,8 @@ static void *__arm_v7s_alloc_table(int lvl, gfp_t gfp,

return NULL;
  
  	phys = virt_to_phys(table);

-   if (phys != (arm_v7s_iopte)phys) {
+   if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
+   phys >= (1ULL << cfg->oas) : phys != (arm_v7s_iopte)phys) {


Given that the comment above says it supports all of memory, how would 
phys >= (1ULL << cfg->oas) ever be true?



/* Doesn't fit in PTE */
dev_err(dev, "Page table does not fit in PTE: %pa", );
goto out_free;
@@ -457,9 +470,14 @@ static arm_v7s_iopte arm_v7s_install_table(arm_v7s_iopte 
*table,
   arm_v7s_iopte curr,
   struct io_pgtable_cfg *cfg)
  {
+   phys_addr_t phys = virt_to_phys(table);
arm_v7s_iopte old, new;
  
-	new = virt_to_phys(table) | ARM_V7S_PTE_TYPE_TABLE;

+   new = phys | ARM_V7S_PTE_TYPE_TABLE;
+
+   if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT)
+   new = to_mtk_iopte(phys, new);
+
if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS)
new |= ARM_V7S_ATTR_NS_TABLE;
  
@@ -779,6 +797,7 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,

void *cookie)
  {
struct arm_v7s_io_pgtable *data;
+   slab_flags_t slab_flag;
  
  	if (cfg->ias > (arm_v7s_is_mtk_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS))

return NULL;
@@ -788,7 +807,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct 
io_pgtable_cfg *cfg,
  
  	if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |

IO_PGTABLE_QUIRK_NO_PERMS |
-   IO_PGTABLE_QUIRK_ARM_MTK_EXT))
+   IO_PGTABLE_QUIRK_ARM_MTK_EXT |
+   IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT))
return NULL;
  
  	/* If ARM_MTK_4GB is enabled, the NO_PERMS is also expected. */

@@ -796,15 +816,27 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct 
io_pgtable_cfg *cfg,
!(cfg->quirks & IO_PGTABLE_QUIRK_NO_PERMS))
return NULL;
  
+	if ((cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT) &&

+   

[PATCH v9 1/3] iommu/io-pgtable-arm-v7s: Add a quirk to allow pgtable PA up to 35bit

2022-06-15 Thread yf.wang--- via iommu
From: Yunfei Wang 

Single memory zone feature will remove ZONE_DMA32 and ZONE_DMA and
cause pgtable PA size larger than 32bit.

Since Mediatek IOMMU hardware support at most 35bit PA in pgtable,
so add a quirk to allow the PA of pgtables support up to bit35.

Signed-off-by: Ning Li 
Signed-off-by: Yunfei Wang 
---
 drivers/iommu/io-pgtable-arm-v7s.c | 58 +++---
 include/linux/io-pgtable.h | 17 +
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c 
b/drivers/iommu/io-pgtable-arm-v7s.c
index be066c1503d3..39e5503ac75a 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -182,14 +182,8 @@ static bool arm_v7s_is_mtk_enabled(struct io_pgtable_cfg 
*cfg)
(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT);
 }
 
-static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int lvl,
-   struct io_pgtable_cfg *cfg)
+static arm_v7s_iopte to_mtk_iopte(phys_addr_t paddr, arm_v7s_iopte pte)
 {
-   arm_v7s_iopte pte = paddr & ARM_V7S_LVL_MASK(lvl);
-
-   if (!arm_v7s_is_mtk_enabled(cfg))
-   return pte;
-
if (paddr & BIT_ULL(32))
pte |= ARM_V7S_ATTR_MTK_PA_BIT32;
if (paddr & BIT_ULL(33))
@@ -199,6 +193,17 @@ static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int 
lvl,
return pte;
 }
 
+static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int lvl,
+   struct io_pgtable_cfg *cfg)
+{
+   arm_v7s_iopte pte = paddr & ARM_V7S_LVL_MASK(lvl);
+
+   if (arm_v7s_is_mtk_enabled(cfg))
+   return to_mtk_iopte(paddr, pte);
+
+   return pte;
+}
+
 static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
  struct io_pgtable_cfg *cfg)
 {
@@ -240,10 +245,17 @@ static void *__arm_v7s_alloc_table(int lvl, gfp_t gfp,
dma_addr_t dma;
size_t size = ARM_V7S_TABLE_SIZE(lvl, cfg);
void *table = NULL;
+   gfp_t gfp_l1;
+
+   /*
+* ARM_MTK_TTBR_EXT extend the translation table base support all
+* memory address.
+*/
+   gfp_l1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
+GFP_KERNEL : ARM_V7S_TABLE_GFP_DMA;
 
if (lvl == 1)
-   table = (void *)__get_free_pages(
-   __GFP_ZERO | ARM_V7S_TABLE_GFP_DMA, get_order(size));
+   table = (void *)__get_free_pages(gfp_l1 | __GFP_ZERO, 
get_order(size));
else if (lvl == 2)
table = kmem_cache_zalloc(data->l2_tables, gfp);
 
@@ -251,7 +263,8 @@ static void *__arm_v7s_alloc_table(int lvl, gfp_t gfp,
return NULL;
 
phys = virt_to_phys(table);
-   if (phys != (arm_v7s_iopte)phys) {
+   if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
+   phys >= (1ULL << cfg->oas) : phys != (arm_v7s_iopte)phys) {
/* Doesn't fit in PTE */
dev_err(dev, "Page table does not fit in PTE: %pa", );
goto out_free;
@@ -457,9 +470,14 @@ static arm_v7s_iopte arm_v7s_install_table(arm_v7s_iopte 
*table,
   arm_v7s_iopte curr,
   struct io_pgtable_cfg *cfg)
 {
+   phys_addr_t phys = virt_to_phys(table);
arm_v7s_iopte old, new;
 
-   new = virt_to_phys(table) | ARM_V7S_PTE_TYPE_TABLE;
+   new = phys | ARM_V7S_PTE_TYPE_TABLE;
+
+   if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT)
+   new = to_mtk_iopte(phys, new);
+
if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS)
new |= ARM_V7S_ATTR_NS_TABLE;
 
@@ -779,6 +797,7 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct 
io_pgtable_cfg *cfg,
void *cookie)
 {
struct arm_v7s_io_pgtable *data;
+   slab_flags_t slab_flag;
 
if (cfg->ias > (arm_v7s_is_mtk_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS))
return NULL;
@@ -788,7 +807,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct 
io_pgtable_cfg *cfg,
 
if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NO_PERMS |
-   IO_PGTABLE_QUIRK_ARM_MTK_EXT))
+   IO_PGTABLE_QUIRK_ARM_MTK_EXT |
+   IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT))
return NULL;
 
/* If ARM_MTK_4GB is enabled, the NO_PERMS is also expected. */
@@ -796,15 +816,27 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct 
io_pgtable_cfg *cfg,
!(cfg->quirks & IO_PGTABLE_QUIRK_NO_PERMS))
return NULL;
 
+   if ((cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT) &&
+   !arm_v7s_is_mtk_enabled(cfg))
+   return NULL;
+
data = kmalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return NULL;