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: <38a627a2-040d-23e2-5637-32f199d0fc31@quicinc.com>
Date:   Wed, 31 May 2023 11:36:52 +0530
From:   Vikash Garodia <quic_vgarodia@...cinc.com>
To:     Stephan Gerhold <stephan@...hold.net>,
        Stanimir Varbanov <stanimir.k.varbanov@...il.com>
CC:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] media: venus: firmware: Use of_reserved_mem_lookup()

Hi Stephan,

On 5/29/2023 11:46 PM, Stephan Gerhold wrote:
> Reserved memory can be either looked up using the generic function
> of_address_to_resource() or using the special of_reserved_mem_lookup().
> The latter has the advantage that it ensures that the referenced memory
> region was really reserved and is not e.g. status = "disabled".
> 
> of_reserved_mem also supports allocating reserved memory dynamically at
> boot time. This works only when using of_reserved_mem_lookup() since
> there won't be a fixed address in the device tree.
IIUC, this would avoid precomputing the hard range for different firmware
regions and also make it more flexible to adjust the sizes, if anyone wants a
bigger size later.
Incase a specific firmware needs a dedicate start address, do we have an option
to specify the same ?

Thanks,
Vikash
> Switch the code to use of_reserved_mem_lookup(). There is no functional
> difference for static reserved memory allocations.
> 
> Signed-off-by: Stephan Gerhold <stephan@...hold.net>
> ---
> See e.g. [1] for an example of dynamically allocated reserved memory.
> (This patch does *not* depend on [1] and is useful without as well...)
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20230510-dt-resv-bottom-up-v1-5-3bf68873dbed@gerhold.net/
> ---
>  drivers/media/platform/qcom/venus/firmware.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/firmware.c b/drivers/media/platform/qcom/venus/firmware.c
> index cfb11c551167..2e7ffdaff7b2 100644
> --- a/drivers/media/platform/qcom/venus/firmware.c
> +++ b/drivers/media/platform/qcom/venus/firmware.c
> @@ -10,6 +10,7 @@
>  #include <linux/io.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  #include <linux/of_device.h>
>  #include <linux/firmware/qcom/qcom_scm.h>
> @@ -82,9 +83,9 @@ static int venus_load_fw(struct venus_core *core, const char *fwname,
>  			 phys_addr_t *mem_phys, size_t *mem_size)
>  {
>  	const struct firmware *mdt;
> +	struct reserved_mem *rmem;
>  	struct device_node *node;
>  	struct device *dev;
> -	struct resource r;
>  	ssize_t fw_size;
>  	void *mem_va;
>  	int ret;
> @@ -99,13 +100,16 @@ static int venus_load_fw(struct venus_core *core, const char *fwname,
>  		return -EINVAL;
>  	}
>  
> -	ret = of_address_to_resource(node, 0, &r);
> -	if (ret)
> -		goto err_put_node;
> +	rmem = of_reserved_mem_lookup(node);
> +	of_node_put(node);
> +	if (!rmem) {
> +		dev_err(dev, "failed to lookup reserved memory-region\n");
> +		return -EINVAL;
> +	}
>  
>  	ret = request_firmware(&mdt, fwname, dev);
>  	if (ret < 0)
> -		goto err_put_node;
> +		return ret;
>  
>  	fw_size = qcom_mdt_get_size(mdt);
>  	if (fw_size < 0) {
> @@ -113,17 +117,17 @@ static int venus_load_fw(struct venus_core *core, const char *fwname,
>  		goto err_release_fw;
>  	}
>  
> -	*mem_phys = r.start;
> -	*mem_size = resource_size(&r);
> +	*mem_phys = rmem->base;
> +	*mem_size = rmem->size;
>  
>  	if (*mem_size < fw_size || fw_size > VENUS_FW_MEM_SIZE) {
>  		ret = -EINVAL;
>  		goto err_release_fw;
>  	}
>  
> -	mem_va = memremap(r.start, *mem_size, MEMREMAP_WC);
> +	mem_va = memremap(*mem_phys, *mem_size, MEMREMAP_WC);
>  	if (!mem_va) {
> -		dev_err(dev, "unable to map memory region: %pR\n", &r);
> +		dev_err(dev, "unable to map memory region %pa size %#zx\n", mem_phys, *mem_size);
>  		ret = -ENOMEM;
>  		goto err_release_fw;
>  	}
> @@ -138,8 +142,6 @@ static int venus_load_fw(struct venus_core *core, const char *fwname,
>  	memunmap(mem_va);
>  err_release_fw:
>  	release_firmware(mdt);
> -err_put_node:
> -	of_node_put(node);
>  	return ret;
>  }
>  
> 
> ---
> base-commit: 9f9f8ca6f012d25428f8605cb36369a449db8508
> change-id: 20230529-venus-of-rmem-f649885114fd
> 
> Best regards,

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ