| Message ID | 20230920-rmtfs-mem-guard-pages-v3-3-305b37219b78@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | soc: qcom: rmtfs: Support dynamic allocation | expand |
On Wed, Sep 20, 2023 at 07:37:32PM -0700, Bjorn Andersson wrote: > In the even that Linux failed to allocate the reserved memory range > specified in the DeviceTree, the size of the reserved_mem will be 0, > which results in a oops when memory remapping is attempted. > > Detect this and report that the memory region was not found instead. > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> I dropped these checks in my remoteproc patches because Caleb suggested maybe putting this check directly in of_reserved_mem_lookup() (or similar) given that almost none of the users verify this [1]. Do you have any opinion on that? I asked back then too but you did not reply yet [2]. :-) [1]: https://lore.kernel.org/linux-arm-msm/c3f59fb4-4dd8-f27a-d3f5-b1870006a75c@linaro.org/ [2]: https://lore.kernel.org/linux-arm-msm/ZIsld-MAdkKvdzTx@gerhold.net/ > --- > drivers/soc/qcom/rmtfs_mem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c > index 83bba9321e72..13823abd85c2 100644 > --- a/drivers/soc/qcom/rmtfs_mem.c > +++ b/drivers/soc/qcom/rmtfs_mem.c > @@ -180,7 +180,7 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) > int ret, i; > > rmem = of_reserved_mem_lookup(node); > - if (!rmem) { > + if (!rmem || !rmem->size) { > dev_err(&pdev->dev, "failed to acquire memory region\n"); > return -EINVAL; > } > > -- > 2.25.1 >
On Thu, Sep 21, 2023 at 08:11:23PM +0200, Stephan Gerhold wrote: > On Wed, Sep 20, 2023 at 07:37:32PM -0700, Bjorn Andersson wrote: > > In the even that Linux failed to allocate the reserved memory range > > specified in the DeviceTree, the size of the reserved_mem will be 0, > > which results in a oops when memory remapping is attempted. > > > > Detect this and report that the memory region was not found instead. > > > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > > I dropped these checks in my remoteproc patches because Caleb suggested > maybe putting this check directly in of_reserved_mem_lookup() (or > similar) given that almost none of the users verify this [1]. > > Do you have any opinion on that? I asked back then too but you did not > reply yet [2]. :-) > I'm struggling to come up with a use case where one would like to get hold of the rmem when it wasn't properly initialized. So, let's make an attempt at returning NULL from of_reserved_mem_lookup() instead. Thanks, Bjorn > [1]: https://lore.kernel.org/linux-arm-msm/c3f59fb4-4dd8-f27a-d3f5-b1870006a75c@linaro.org/ > [2]: https://lore.kernel.org/linux-arm-msm/ZIsld-MAdkKvdzTx@gerhold.net/ > > > --- > > drivers/soc/qcom/rmtfs_mem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c > > index 83bba9321e72..13823abd85c2 100644 > > --- a/drivers/soc/qcom/rmtfs_mem.c > > +++ b/drivers/soc/qcom/rmtfs_mem.c > > @@ -180,7 +180,7 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) > > int ret, i; > > > > rmem = of_reserved_mem_lookup(node); > > - if (!rmem) { > > + if (!rmem || !rmem->size) { > > dev_err(&pdev->dev, "failed to acquire memory region\n"); > > return -EINVAL; > > } > > > > -- > > 2.25.1 > >
diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c index 83bba9321e72..13823abd85c2 100644 --- a/drivers/soc/qcom/rmtfs_mem.c +++ b/drivers/soc/qcom/rmtfs_mem.c @@ -180,7 +180,7 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev) int ret, i; rmem = of_reserved_mem_lookup(node); - if (!rmem) { + if (!rmem || !rmem->size) { dev_err(&pdev->dev, "failed to acquire memory region\n"); return -EINVAL; }
In the even that Linux failed to allocate the reserved memory range specified in the DeviceTree, the size of the reserved_mem will be 0, which results in a oops when memory remapping is attempted. Detect this and report that the memory region was not found instead. Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- drivers/soc/qcom/rmtfs_mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)