linux-kernel - Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg

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]

Message-ID: <cc04ce89-9790-b127-19e5-8f2f1b91ad4f@redhat.com>
Date:   Tue, 21 Jul 2020 11:10:55 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Miles Chen <miles.chen@...iatek.com>,
        Joerg Roedel <joro@...tes.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh@...nel.org>
Cc:     iommu@...ts.linux-foundation.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
        wsd_upstream@...iatek.com, Mike Rapoport <rppt@...ux.ibm.com>,
        Yong Wu <yong.wu@...iatek.com>,
        Yingjoe Chen <yingjoe.chen@...iatek.com>,
        Christoph Hellwig <hch@....de>,
        Chao Hao <chao.hao@...iatek.com>
Subject: Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg

On 21.07.20 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode

Nit: We tend to place such information below the "---" (adding a second
one) such that this information is discarded when the patch is picked up.

> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@...ux.ibm.com>
> Cc: David Hildenbrand <david@...hat.com>
> Cc: Yong Wu <yong.wu@...iatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@...iatek.com>
> Cc: Christoph Hellwig <hch@....de>
> Cc: Yong Wu <yong.wu@...iatek.com>
> Cc: Chao Hao <chao.hao@...iatek.com>
> Cc: Rob Herring <robh@...nel.org>
> Cc: Matthias Brugger <matthias.bgg@...il.com>
> Signed-off-by: Miles Chen <miles.chen@...iatek.com>
> ---
>  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>  include/linux/soc/mediatek/infracfg.h |  3 +++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>   * Copyright (c) 2015-2016 MediaTek Inc.
>   * Author: Yong Wu <yong.wu@...iatek.com>
>   */
> -#include <linux/memblock.h>
>  #include <linux/bug.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
> @@ -15,13 +14,16 @@
>  #include <linux/iommu.h>
>  #include <linux/iopoll.h>
>  #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
>  #include <linux/of_iommu.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>  #include <asm/barrier.h>
>  #include <soc/mediatek/smi.h>
>  
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	struct resource         *res;
>  	resource_size_t		ioaddr;
>  	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>  	void                    *protect;
>  	int                     i, larb_nr, ret;
> +	u32			val;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>  
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);
> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);

(I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
missing some context, so sorry for the stupid questions)

Who sets the regmap value and based on what? Who decides that 4GB mode
is supported or not? And who decides if 4GB mode is *required* or not?

-- 
Thanks,

David / dhildenb