[v10,09/23] iommu/io-pgtable-arm-v7s: Extend to support PA[33:32] for MediaTek
diff mbox series

Message ID 1566395606-7975-10-git-send-email-yong.wu@mediatek.com
State New
Headers show
Series
  • MT8183 IOMMU SUPPORT
Related show

Commit Message

Yong Wu Aug. 21, 2019, 1:53 p.m. UTC
MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
respectively. Meanwhile the iova still is 32bits.

Regarding whether the pagetable address could be over 4GB, the mt8183
support it while the previous mt8173 don't, thus keep it as is.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
 include/linux/io-pgtable.h         |  7 +++----
 2 files changed, 28 insertions(+), 11 deletions(-)

Comments

Will Deacon Aug. 21, 2019, 3:24 p.m. UTC | #1
On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> respectively. Meanwhile the iova still is 32bits.
> 
> Regarding whether the pagetable address could be over 4GB, the mt8183
> support it while the previous mt8173 don't, thus keep it as is.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
>  include/linux/io-pgtable.h         |  7 +++----
>  2 files changed, 28 insertions(+), 11 deletions(-)

[...]

> @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
>  {
>  	struct arm_v7s_io_pgtable *data;
>  
> -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))

Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?

With that change:

Acked-by: Will Deacon <will@kernel.org>

Will
Robin Murphy Aug. 21, 2019, 3:34 p.m. UTC | #2
On 21/08/2019 16:24, Will Deacon wrote:
> On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
>> MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
>> the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
>> respectively. Meanwhile the iova still is 32bits.
>>
>> Regarding whether the pagetable address could be over 4GB, the mt8183
>> support it while the previous mt8173 don't, thus keep it as is.
>>
>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>> ---
>>   drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
>>   include/linux/io-pgtable.h         |  7 +++----
>>   2 files changed, 28 insertions(+), 11 deletions(-)
> 
> [...]
> 
>> @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
>>   {
>>   	struct arm_v7s_io_pgtable *data;
>>   
>> -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
>> +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
>> +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
>> +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> 
> Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?

You mean oas, right? I believe the hardware *does* actually support a 
32-bit ias as well, but we shouldn't pretend to support that while 
__arm_v7s_alloc_table() still only knows how to allocate normal-sized 
tables.

Robin.

> 
> With that change:
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> Will
>
Will Deacon Aug. 21, 2019, 4:35 p.m. UTC | #3
On Wed, Aug 21, 2019 at 04:34:27PM +0100, Robin Murphy wrote:
> On 21/08/2019 16:24, Will Deacon wrote:
> > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > > respectively. Meanwhile the iova still is 32bits.
> > > 
> > > Regarding whether the pagetable address could be over 4GB, the mt8183
> > > support it while the previous mt8173 don't, thus keep it as is.
> > > 
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > ---
> > >   drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> > >   include/linux/io-pgtable.h         |  7 +++----
> > >   2 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > [...]
> > 
> > > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> > >   {
> > >   	struct arm_v7s_io_pgtable *data;
> > > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > 
> > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> 
> You mean oas, right? I believe the hardware *does* actually support a 32-bit
> ias as well, but we shouldn't pretend to support that while
> __arm_v7s_alloc_table() still only knows how to allocate normal-sized
> tables.

Sorry, yes, oas.

Will
Yong Wu Aug. 22, 2019, 8:56 a.m. UTC | #4
On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
> On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > respectively. Meanwhile the iova still is 32bits.
> > 
> > Regarding whether the pagetable address could be over 4GB, the mt8183
> > support it while the previous mt8173 don't, thus keep it as is.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> >  drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> >  include/linux/io-pgtable.h         |  7 +++----
> >  2 files changed, 28 insertions(+), 11 deletions(-)
> 
> [...]
> 
> > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> >  {
> >  	struct arm_v7s_io_pgtable *data;
> >  
> > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> 
> Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?

Here I only simply skip the oas checking for our case. then which way do
your prefer?  something like you commented before:?


	if (cfg->ias > ARM_V7S_ADDR_BITS)
		return NULL;

	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
		else if (cfg->oas > 34)
			return NULL;
	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
		return NULL;
	}


> 
> With that change:
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> Will
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Yong Wu Aug. 22, 2019, 8:59 a.m. UTC | #5
On Wed, 2019-08-21 at 16:34 +0100, Robin Murphy wrote:
> On 21/08/2019 16:24, Will Deacon wrote:
> > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> >> MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> >> the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> >> respectively. Meanwhile the iova still is 32bits.
> >>
> >> Regarding whether the pagetable address could be over 4GB, the mt8183
> >> support it while the previous mt8173 don't, thus keep it as is.
> >>
> >> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> >> ---
> >>   drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> >>   include/linux/io-pgtable.h         |  7 +++----
> >>   2 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > [...]
> > 
> >> @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> >>   {
> >>   	struct arm_v7s_io_pgtable *data;
> >>   
> >> -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> >> +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> >> +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> >> +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > 
> > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> 
> You mean oas, right? I believe the hardware *does* actually support a 
> 32-bit ias as well, but we shouldn't pretend to support that while 
> __arm_v7s_alloc_table() still only knows how to allocate normal-sized 
> tables.

Yes. The HW double the lvl1 pgtable, thus it supports 33bit iova
actually. We may extend ias in the future.

> 
> Robin.
> 
> > 
> > With that change:
> > 
> > Acked-by: Will Deacon <will@kernel.org>
> > 
> > Will
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Will Deacon Aug. 22, 2019, 10:08 a.m. UTC | #6
On Thu, Aug 22, 2019 at 04:56:26PM +0800, Yong Wu wrote:
> On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
> > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > > respectively. Meanwhile the iova still is 32bits.
> > > 
> > > Regarding whether the pagetable address could be over 4GB, the mt8183
> > > support it while the previous mt8173 don't, thus keep it as is.
> > > 
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > ---
> > >  drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> > >  include/linux/io-pgtable.h         |  7 +++----
> > >  2 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > [...]
> > 
> > > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> > >  {
> > >  	struct arm_v7s_io_pgtable *data;
> > >  
> > > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > 
> > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> 
> Here I only simply skip the oas checking for our case. then which way do
> your prefer?  something like you commented before:?
> 
> 
> 	if (cfg->ias > ARM_V7S_ADDR_BITS)
> 		return NULL;
> 
> 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
> 		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> 			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);

Isn't this always 32 for your IOMMUs?

> 		else if (cfg->oas > 34)
> 			return NULL;
> 	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
> 		return NULL;
> 	}

How about:

	unsigned int oas_max = ARM_V7S_ADDR_BITS;

	if (cfg->ias > ARM_V7S_ADDR_BITS)
		return NULL;

	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
	    cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
		oas_max = 34;

	if (cfg->oas > oas_max)
		return NULL;

Will
Robin Murphy Aug. 22, 2019, 10:08 a.m. UTC | #7
On 2019-08-22 9:56 am, Yong Wu wrote:
> On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
>> On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
>>> MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
>>> the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
>>> respectively. Meanwhile the iova still is 32bits.
>>>
>>> Regarding whether the pagetable address could be over 4GB, the mt8183
>>> support it while the previous mt8173 don't, thus keep it as is.
>>>
>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>>> ---
>>>   drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
>>>   include/linux/io-pgtable.h         |  7 +++----
>>>   2 files changed, 28 insertions(+), 11 deletions(-)
>>
>> [...]
>>
>>> @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
>>>   {
>>>   	struct arm_v7s_io_pgtable *data;
>>>   
>>> -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
>>> +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
>>> +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
>>> +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
>>
>> Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
>> ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> 
> Here I only simply skip the oas checking for our case. then which way do
> your prefer?  something like you commented before:?
> 
> 
> 	if (cfg->ias > ARM_V7S_ADDR_BITS)
> 		return NULL;
> 
> 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
> 		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> 			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
> 		else if (cfg->oas > 34)
> 			return NULL;
> 	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
> 		return NULL;
> 	}

All it should take is something like:

	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
		max_oas = 34;
	else
		max_oas = 32;
	if (cfg->oas > max_oas)
		return NULL;

or even just:

	if (cfg->oas > 32 ||
	    (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT && cfg->oas > 34))
		return NULL;

(and if we prefer the latter style, perhaps we could introduce some kind 
of "is_mtk_4gb()" helper to save on verbosity)

We shouldn't need to care about the size of phys_addr_t either way - the 
fact is that the MTK format can still encode up to 34 bits of PA 
regardless of whether callers can actually pass addresses that large.

Robin.
Will Deacon Aug. 22, 2019, 10:17 a.m. UTC | #8
On Thu, Aug 22, 2019 at 11:08:58AM +0100, Robin Murphy wrote:
> On 2019-08-22 9:56 am, Yong Wu wrote:
> > On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
> > > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > > > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > > > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > > > respectively. Meanwhile the iova still is 32bits.
> > > > 
> > > > Regarding whether the pagetable address could be over 4GB, the mt8183
> > > > support it while the previous mt8173 don't, thus keep it as is.
> > > > 
> > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > ---
> > > >   drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> > > >   include/linux/io-pgtable.h         |  7 +++----
> > > >   2 files changed, 28 insertions(+), 11 deletions(-)
> > > 
> > > [...]
> > > 
> > > > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> > > >   {
> > > >   	struct arm_v7s_io_pgtable *data;
> > > > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > > > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > > > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > > > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > > 
> > > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> > 
> > Here I only simply skip the oas checking for our case. then which way do
> > your prefer?  something like you commented before:?
> > 
> > 
> > 	if (cfg->ias > ARM_V7S_ADDR_BITS)
> > 		return NULL;
> > 
> > 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
> > 		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> > 			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
> > 		else if (cfg->oas > 34)
> > 			return NULL;
> > 	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
> > 		return NULL;
> > 	}
> 
> All it should take is something like:
> 
> 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
> 		max_oas = 34;
> 	else
> 		max_oas = 32;
> 	if (cfg->oas > max_oas)
> 		return NULL;
> 
> or even just:
> 
> 	if (cfg->oas > 32 ||
> 	    (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT && cfg->oas > 34))
> 		return NULL;
> 
> (and if we prefer the latter style, perhaps we could introduce some kind of
> "is_mtk_4gb()" helper to save on verbosity)

I wondered the same thing, but another place we'd want the check is in
iopte_to_paddr() which probably needs the PHYS_ADDR_T check to avoid GCC
warnings, although I didn't try it.

So if we did:

static bool cfg_mtk_ext_enabled(struct io_pgtable_cfg *cfg)
{
	return IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
	       cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT;
}

Then I suppose we could do this in _alloc():

	if (cfg->oas > cfg_mtk_ext_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS)
		return NULL;

and then this in iopte_to_paddr():

	[...]

	paddr = pte & mask;
	if (!cfg_mtk_ext_enabled(cfg))
		return paddr;

	if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
		paddr |= ...

	[...]

What do you reckon?

Will
Robin Murphy Aug. 22, 2019, 10:57 a.m. UTC | #9
On 2019-08-22 11:17 am, Will Deacon wrote:
> On Thu, Aug 22, 2019 at 11:08:58AM +0100, Robin Murphy wrote:
>> On 2019-08-22 9:56 am, Yong Wu wrote:
>>> On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
>>>> On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
>>>>> MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
>>>>> the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
>>>>> respectively. Meanwhile the iova still is 32bits.
>>>>>
>>>>> Regarding whether the pagetable address could be over 4GB, the mt8183
>>>>> support it while the previous mt8173 don't, thus keep it as is.
>>>>>
>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>>>>> ---
>>>>>    drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
>>>>>    include/linux/io-pgtable.h         |  7 +++----
>>>>>    2 files changed, 28 insertions(+), 11 deletions(-)
>>>>
>>>> [...]
>>>>
>>>>> @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
>>>>>    {
>>>>>    	struct arm_v7s_io_pgtable *data;
>>>>> -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
>>>>> +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
>>>>> +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
>>>>> +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
>>>>
>>>> Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
>>>> ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
>>>
>>> Here I only simply skip the oas checking for our case. then which way do
>>> your prefer?  something like you commented before:?
>>>
>>>
>>> 	if (cfg->ias > ARM_V7S_ADDR_BITS)
>>> 		return NULL;
>>>
>>> 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
>>> 		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
>>> 			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
>>> 		else if (cfg->oas > 34)
>>> 			return NULL;
>>> 	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
>>> 		return NULL;
>>> 	}
>>
>> All it should take is something like:
>>
>> 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
>> 		max_oas = 34;
>> 	else
>> 		max_oas = 32;
>> 	if (cfg->oas > max_oas)
>> 		return NULL;
>>
>> or even just:
>>
>> 	if (cfg->oas > 32 ||
>> 	    (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT && cfg->oas > 34))
>> 		return NULL;
>>
>> (and if we prefer the latter style, perhaps we could introduce some kind of
>> "is_mtk_4gb()" helper to save on verbosity)
> 
> I wondered the same thing, but another place we'd want the check is in
> iopte_to_paddr() which probably needs the PHYS_ADDR_T check to avoid GCC
> warnings, although I didn't try it.

I'm pretty sure I confirmed that "paddr |= BIT_ULL(32)" doesn't warn 
when phys_addt_t is 32-bit - it's well-defined unsigned integer 
truncation after all, and if GCC starts warning about all the valid 
no-op code it optimises away then it's going to run up against 
IS_ENABLED() first and foremost ;)

> So if we did:
> 
> static bool cfg_mtk_ext_enabled(struct io_pgtable_cfg *cfg)
> {
> 	return IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
> 	       cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT;
> }
> 
> Then I suppose we could do this in _alloc():
> 
> 	if (cfg->oas > cfg_mtk_ext_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS)
> 		return NULL;
> 
> and then this in iopte_to_paddr():
> 
> 	[...]
> 
> 	paddr = pte & mask;
> 	if (!cfg_mtk_ext_enabled(cfg))
> 		return paddr;
> 
> 	if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
> 		paddr |= ...
> 
> 	[...]
> 
> What do you reckon?

Yeah, that's the general shape of things I was picturing - I'm not that 
fussed about the PHYS_ADDR_T_64BIT thing, especially if it's wrapped up 
in just one place, so if you do want to keep it as belt-and-braces I'll 
just consider it a slight code size optimisation for 32-bit builds.

Robin.
Will Deacon Aug. 22, 2019, 11:28 a.m. UTC | #10
On Thu, Aug 22, 2019 at 11:57:11AM +0100, Robin Murphy wrote:
> On 2019-08-22 11:17 am, Will Deacon wrote:
> > On Thu, Aug 22, 2019 at 11:08:58AM +0100, Robin Murphy wrote:
> > > On 2019-08-22 9:56 am, Yong Wu wrote:
> > > > On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
> > > > > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > > > > > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > > > > > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > > > > > respectively. Meanwhile the iova still is 32bits.
> > > > > > 
> > > > > > Regarding whether the pagetable address could be over 4GB, the mt8183
> > > > > > support it while the previous mt8173 don't, thus keep it as is.
> > > > > > 
> > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > ---
> > > > > >    drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> > > > > >    include/linux/io-pgtable.h         |  7 +++----
> > > > > >    2 files changed, 28 insertions(+), 11 deletions(-)
> > > > > 
> > > > > [...]
> > > > > 
> > > > > > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> > > > > >    {
> > > > > >    	struct arm_v7s_io_pgtable *data;
> > > > > > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > > > > > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > > > > > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > > > > > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > > > > 
> > > > > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > > > > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> > > > 
> > > > Here I only simply skip the oas checking for our case. then which way do
> > > > your prefer?  something like you commented before:?
> > > > 
> > > > 
> > > > 	if (cfg->ias > ARM_V7S_ADDR_BITS)
> > > > 		return NULL;
> > > > 
> > > > 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
> > > > 		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> > > > 			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
> > > > 		else if (cfg->oas > 34)
> > > > 			return NULL;
> > > > 	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
> > > > 		return NULL;
> > > > 	}
> > > 
> > > All it should take is something like:
> > > 
> > > 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
> > > 		max_oas = 34;
> > > 	else
> > > 		max_oas = 32;
> > > 	if (cfg->oas > max_oas)
> > > 		return NULL;
> > > 
> > > or even just:
> > > 
> > > 	if (cfg->oas > 32 ||
> > > 	    (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT && cfg->oas > 34))
> > > 		return NULL;
> > > 
> > > (and if we prefer the latter style, perhaps we could introduce some kind of
> > > "is_mtk_4gb()" helper to save on verbosity)
> > 
> > I wondered the same thing, but another place we'd want the check is in
> > iopte_to_paddr() which probably needs the PHYS_ADDR_T check to avoid GCC
> > warnings, although I didn't try it.
> 
> I'm pretty sure I confirmed that "paddr |= BIT_ULL(32)" doesn't warn when
> phys_addt_t is 32-bit - it's well-defined unsigned integer truncation after
> all, and if GCC starts warning about all the valid no-op code it optimises
> away then it's going to run up against IS_ENABLED() first and foremost ;)

You're quite right, although we live in a world where GCC shouts at us about
missing comments in switch statements so I think my worry was justified!

> > So if we did:
> > 
> > static bool cfg_mtk_ext_enabled(struct io_pgtable_cfg *cfg)
> > {
> > 	return IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
> > 	       cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT;
> > }
> > 
> > Then I suppose we could do this in _alloc():
> > 
> > 	if (cfg->oas > cfg_mtk_ext_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS)
> > 		return NULL;

^^ Apparantly, I left the bracketting here as an exercise to the reader.

> > 
> > and then this in iopte_to_paddr():
> > 
> > 	[...]
> > 
> > 	paddr = pte & mask;
> > 	if (!cfg_mtk_ext_enabled(cfg))
> > 		return paddr;
> > 
> > 	if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
> > 		paddr |= ...
> > 
> > 	[...]
> > 
> > What do you reckon?
> 
> Yeah, that's the general shape of things I was picturing - I'm not that
> fussed about the PHYS_ADDR_T_64BIT thing, especially if it's wrapped up in
> just one place, so if you do want to keep it as belt-and-braces I'll just
> consider it a slight code size optimisation for 32-bit builds.

Ok, great. Yong Wu -- are you ok respinning with the above + missing
brackets?

Will
Yong Wu Aug. 22, 2019, 12:05 p.m. UTC | #11
Thanks very much for viewing this so quickly.

On Thu, 2019-08-22 at 12:28 +0100, Will Deacon wrote:
> On Thu, Aug 22, 2019 at 11:57:11AM +0100, Robin Murphy wrote:
> > On 2019-08-22 11:17 am, Will Deacon wrote:
> > > On Thu, Aug 22, 2019 at 11:08:58AM +0100, Robin Murphy wrote:
> > > > On 2019-08-22 9:56 am, Yong Wu wrote:
> > > > > On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
> > > > > > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > > > > > > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > > > > > > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > > > > > > respectively. Meanwhile the iova still is 32bits.
> > > > > > > 
> > > > > > > Regarding whether the pagetable address could be over 4GB, the mt8183
> > > > > > > support it while the previous mt8173 don't, thus keep it as is.
> > > > > > > 
> > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > > > > ---
> > > > > > >    drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> > > > > > >    include/linux/io-pgtable.h         |  7 +++----
> > > > > > >    2 files changed, 28 insertions(+), 11 deletions(-)
> > > > > > 
> > > > > > [...]
> > > > > > 
> > > > > > > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> > > > > > >    {
> > > > > > >    	struct arm_v7s_io_pgtable *data;
> > > > > > > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > > > > > > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > > > > > > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > > > > > > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > > > > > 
> > > > > > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > > > > > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> > > > > 
> > > > > Here I only simply skip the oas checking for our case. then which way do
> > > > > your prefer?  something like you commented before:?
> > > > > 
> > > > > 
> > > > > 	if (cfg->ias > ARM_V7S_ADDR_BITS)
> > > > > 		return NULL;
> > > > > 
> > > > > 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
> > > > > 		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> > > > > 			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
> > > > > 		else if (cfg->oas > 34)
> > > > > 			return NULL;
> > > > > 	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
> > > > > 		return NULL;
> > > > > 	}
> > > > 
> > > > All it should take is something like:
> > > > 
> > > > 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
> > > > 		max_oas = 34;
> > > > 	else
> > > > 		max_oas = 32;
> > > > 	if (cfg->oas > max_oas)
> > > > 		return NULL;
> > > > 
> > > > or even just:
> > > > 
> > > > 	if (cfg->oas > 32 ||
> > > > 	    (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT && cfg->oas > 34))
> > > > 		return NULL;
> > > > 
> > > > (and if we prefer the latter style, perhaps we could introduce some kind of
> > > > "is_mtk_4gb()" helper to save on verbosity)
> > > 
> > > I wondered the same thing, but another place we'd want the check is in
> > > iopte_to_paddr() which probably needs the PHYS_ADDR_T check to avoid GCC
> > > warnings, although I didn't try it.
> > 
> > I'm pretty sure I confirmed that "paddr |= BIT_ULL(32)" doesn't warn when
> > phys_addt_t is 32-bit - it's well-defined unsigned integer truncation after
> > all, and if GCC starts warning about all the valid no-op code it optimises
> > away then it's going to run up against IS_ENABLED() first and foremost ;)
> 
> You're quite right, although we live in a world where GCC shouts at us about
> missing comments in switch statements so I think my worry was justified!
> 
> > > So if we did:
> > > 
> > > static bool cfg_mtk_ext_enabled(struct io_pgtable_cfg *cfg)
> > > {
> > > 	return IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
> > > 	       cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT;
> > > }
> > > 
> > > Then I suppose we could do this in _alloc():
> > > 
> > > 	if (cfg->oas > cfg_mtk_ext_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS)
> > > 		return NULL;
> 
> ^^ Apparantly, I left the bracketting here as an exercise to the reader.
> 
> > > 
> > > and then this in iopte_to_paddr():
> > > 
> > > 	[...]
> > > 
> > > 	paddr = pte & mask;
> > > 	if (!cfg_mtk_ext_enabled(cfg))
> > > 		return paddr;
> > > 
> > > 	if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
> > > 		paddr |= ...
> > > 
> > > 	[...]
> > > 
> > > What do you reckon?
> > 
> > Yeah, that's the general shape of things I was picturing - I'm not that
> > fussed about the PHYS_ADDR_T_64BIT thing, especially if it's wrapped up in
> > just one place, so if you do want to keep it as belt-and-braces I'll just
> > consider it a slight code size optimisation for 32-bit builds.
> 
> Ok, great. Yong Wu -- are you ok respinning with the above + missing
> brackets?

Of course I can.

NearlyAll the interface in this file is prefixed with "arm_v7s_", so
does the new interface also need it?, like arm_v7s_is_mtk_enabled. And
keep the iopte_to_paddr and paddr_to_iopte symmetrical.


Then the final patch would looks like below, is it ok?

 
+static bool arm_v7s_is_mtk_enabled(struct io_pgtable_cfg *cfg)
+{
+	return IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
+		(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)
 {
-	return paddr & ARM_V7S_LVL_MASK(lvl);
+	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))
+		pte |= ARM_V7S_ATTR_MTK_PA_BIT33;
+	return pte;
 }
 
 static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
 				  struct io_pgtable_cfg *cfg)
 {
 	arm_v7s_iopte mask;
+	phys_addr_t paddr;
 
 	if (ARM_V7S_PTE_IS_TABLE(pte, lvl))
 		mask = ARM_V7S_TABLE_MASK;
@@ -194,7 +212,15 @@ static phys_addr_t iopte_to_paddr(arm_v7s_iopte
pte, int lvl,
 	else
 		mask = ARM_V7S_LVL_MASK(lvl);
 
-	return pte & mask;
+	paddr = pte & mask;
+	if (!arm_v7s_is_mtk_enabled(cfg))
+		return paddr;
+
+	if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
+		paddr |= BIT_ULL(32);
+	if (pte & ARM_V7S_ATTR_MTK_PA_BIT33)
+		paddr |= BIT_ULL(33);
+	return paddr;
 }
 
 static arm_v7s_iopte *iopte_deref(arm_v7s_iopte pte, int lvl,
@@ -315,9 +341,6 @@ static arm_v7s_iopte arm_v7s_prot_to_pte(int prot,
int lvl,
 	if (lvl == 1 && (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS))
 		pte |= ARM_V7S_ATTR_NS_SECTION;
 
-	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
-		pte |= ARM_V7S_ATTR_MTK_4GB;
-
 	return pte;
 }
 
@@ -731,7 +754,10 @@ static struct io_pgtable
*arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
 {
 	struct arm_v7s_io_pgtable *data;
 
-	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
+	if (cfg->ias > ARM_V7S_ADDR_BITS)
+		return NULL;
+
+	if (cfg->oas > (arm_v7s_is_mtk_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS))
 		return NULL;
 


> 
> Will
Will Deacon Aug. 22, 2019, 5:14 p.m. UTC | #12
On Thu, Aug 22, 2019 at 08:05:33PM +0800, Yong Wu wrote:
> On Thu, 2019-08-22 at 12:28 +0100, Will Deacon wrote:
> > Ok, great. Yong Wu -- are you ok respinning with the above + missing
> > brackets?
> 
> Of course I can.
> 
> NearlyAll the interface in this file is prefixed with "arm_v7s_", so
> does the new interface also need it?, like arm_v7s_is_mtk_enabled. And
> keep the iopte_to_paddr and paddr_to_iopte symmetrical.
> 
> 
> Then the final patch would looks like below, is it ok?

Looks good to me:

Acked-by: Will Deacon <will@kernel.org>

Will

Patch
diff mbox series

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
index 77cc1eb..4a084f0 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -112,7 +112,9 @@ 
 #define ARM_V7S_TEX_MASK		0x7
 #define ARM_V7S_ATTR_TEX(val)		(((val) & ARM_V7S_TEX_MASK) << ARM_V7S_TEX_SHIFT)
 
-#define ARM_V7S_ATTR_MTK_4GB		BIT(9) /* MTK extend it for 4GB mode */
+/* MediaTek extend the two bits for PA 32bit/33bit */
+#define ARM_V7S_ATTR_MTK_PA_BIT32	BIT(9)
+#define ARM_V7S_ATTR_MTK_PA_BIT33	BIT(4)
 
 /* *well, except for TEX on level 2 large pages, of course :( */
 #define ARM_V7S_CONT_PAGE_TEX_SHIFT	6
@@ -179,13 +181,22 @@  static dma_addr_t __arm_v7s_dma_addr(void *pages)
 static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int lvl,
 				    struct io_pgtable_cfg *cfg)
 {
-	return paddr & ARM_V7S_LVL_MASK(lvl);
+	arm_v7s_iopte pte = paddr & ARM_V7S_LVL_MASK(lvl);
+
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
+		if (paddr & BIT_ULL(32))
+			pte |= ARM_V7S_ATTR_MTK_PA_BIT32;
+		if (paddr & BIT_ULL(33))
+			pte |= ARM_V7S_ATTR_MTK_PA_BIT33;
+	}
+	return pte;
 }
 
 static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
 				  struct io_pgtable_cfg *cfg)
 {
 	arm_v7s_iopte mask;
+	phys_addr_t paddr;
 
 	if (ARM_V7S_PTE_IS_TABLE(pte, lvl))
 		mask = ARM_V7S_TABLE_MASK;
@@ -194,7 +205,15 @@  static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
 	else
 		mask = ARM_V7S_LVL_MASK(lvl);
 
-	return pte & mask;
+	paddr = pte & mask;
+	if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) &&
+	    (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)) {
+		if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
+			paddr |= BIT_ULL(32);
+		if (pte & ARM_V7S_ATTR_MTK_PA_BIT33)
+			paddr |= BIT_ULL(33);
+	}
+	return paddr;
 }
 
 static arm_v7s_iopte *iopte_deref(arm_v7s_iopte pte, int lvl,
@@ -315,9 +334,6 @@  static arm_v7s_iopte arm_v7s_prot_to_pte(int prot, int lvl,
 	if (lvl == 1 && (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS))
 		pte |= ARM_V7S_ATTR_NS_SECTION;
 
-	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)
-		pte |= ARM_V7S_ATTR_MTK_4GB;
-
 	return pte;
 }
 
@@ -731,7 +747,9 @@  static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
 {
 	struct arm_v7s_io_pgtable *data;
 
-	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
+	if (cfg->ias > ARM_V7S_ADDR_BITS ||
+	    (cfg->oas > ARM_V7S_ADDR_BITS &&
+	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
 		return NULL;
 
 	if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 915fb73..a2a52c3 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -65,10 +65,9 @@  struct io_pgtable_cfg {
 	 *	(unmapped) entries but the hardware might do so anyway, perform
 	 *	TLB maintenance when mapping as well as when unmapping.
 	 *
-	 * IO_PGTABLE_QUIRK_ARM_MTK_EXT: (ARM v7s format) Set bit 9 in all
-	 *	PTEs, for Mediatek IOMMUs which treat it as a 33rd address bit
-	 *	when the SoC is in "4GB mode" and they can only access the high
-	 *	remap of DRAM (0x1_00000000 to 0x1_ffffffff).
+	 * IO_PGTABLE_QUIRK_ARM_MTK_EXT: (ARM v7s format) MediaTek IOMMUs extend
+	 *	to support up to 34 bits PA where the bit32 and bit33 are
+	 *	encoded in the bit9 and bit4 of the PTE respectively.
 	 *
 	 * IO_PGTABLE_QUIRK_NON_STRICT: Skip issuing synchronous leaf TLBIs
 	 *	on unmap, for DMA domains using the flush queue mechanism for