lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 15 Jun 2022 18:03:21 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     yf.wang@...iatek.com, Will Deacon <will@...nel.org>,
        Joerg Roedel <joro@...tes.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Georgi Djakov <quic_c_gdjako@...cinc.com>,
        "Isaac J. Manjarres" <isaacm@...eaurora.org>,
        Ning Li <ning.li@...iatek.com>,
        Sven Peter <sven@...npeter.dev>,
        "moderated list:ARM SMMU DRIVERS" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>
Cc:     wsd_upstream@...iatek.com, Libo Kang <Libo.Kang@...iatek.com>,
        Yong Wu <Yong.Wu@...iatek.com>,
        Miles Chen <miles.chen@...iatek.com>
Subject: Re: [PATCH v9 1/3] iommu/io-pgtable-arm-v7s: Add a quirk to allow
 pgtable PA up to 35bit

On 2022-06-15 17:12, yf.wang@...iatek.com wrote:
> From: Yunfei Wang <yf.wang@...iatek.com>
> 
> 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 <ning.li@...iatek.com>
> Signed-off-by: Yunfei Wang <yf.wang@...iatek.com>
> ---
>   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", &phys);
>   		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;
>   
>   	spin_lock_init(&data->split_lock);
> +
> +	/*
> +	 * ARM_MTK_TTBR_EXT extend the translation table base support all
> +	 * memory address.
> +	 */
> +	slab_flag = cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT ?
> +		    0 : ARM_V7S_TABLE_SLAB_FLAGS;
> +
>   	data->l2_tables = kmem_cache_create("io-pgtable_armv7s_l2",
>   					    ARM_V7S_TABLE_SIZE(2, cfg),
>   					    ARM_V7S_TABLE_SIZE(2, cfg),
> -					    ARM_V7S_TABLE_SLAB_FLAGS, NULL);
> +					    slab_flag, NULL);
>   	if (!data->l2_tables)
>   		goto out_free_data;
>   
> 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.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ