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

From: yf.wang
Date: Thu Jun 16 2022 - 02:33:34 EST


On Wed, 2022-06-15 at 18:03 +0100, Robin Murphy wrote:
> On 2022-06-15 17:12, yf.wang@xxxxxxxxxxxx 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",
> > &phys);
> > 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_EXT BIT(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_EXT BIT(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 int ias;
> > @@ -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 & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT)
cfg->arm_v7s_cfg.ttbr = (paddr & GENMASK(31, 7)) |
upper_32_bits(paddr);

Thanks,
Yunfei.

> > u32 tcr;
> > u32 nmrr;
> > u32 prrr;