lkml.org 
[lkml]   [2022]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH v2] PM: opp: Fix NULL pointer exception on a v2 table combined with v1 opps
Date
dev_pm_opp_add() adds a v1 OPP.  If the Devicetree contains an OPP v2
table with required-opps, but the driver uses dev_pm_opp_add(), the
table might have required_opp_count!=0 but the opp->required_opps will
be NULL. This leads to NULL pointer exception, e.g. with ufs-qcom.c
driver and v2 OPP table in DTS:

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
...
Call trace:
_required_opps_available+0x20/0x80
_opp_add_v1+0xa8/0x110
dev_pm_opp_add+0x54/0xcc
ufshcd_async_scan+0x190/0x31c
async_run_entry_fn+0x38/0x144

The case of mixing v1 and v2 OPP tables is not supported. Add a version
field to the OPP table to handle such cases:
1. Disallow mixing v1 and v2 OPPs,
2. Skip parsing required opps when dealing with a v1 OPP table.

Fixes: cf65948d62c6 ("opp: Filter out OPPs based on availability of a required-OPP")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes since v1:
1. Add version field.
2. Skip allocating required fields in _opp_table_alloc_required_tables()
instead of iterating over it later.
---
drivers/opp/core.c | 35 +++++++++++++++++++++++------------
drivers/opp/of.c | 22 +++++++++++++++++++++-
drivers/opp/opp.h | 12 +++++++++++-
3 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 740407252298..0b5357b9d342 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1227,7 +1227,8 @@ struct opp_device *_add_opp_dev(const struct device *dev,
return opp_dev;
}

-static struct opp_table *_allocate_opp_table(struct device *dev, int index)
+static struct opp_table *_allocate_opp_table(struct device *dev, int index,
+ enum opp_table_version version)
{
struct opp_table *opp_table;
struct opp_device *opp_dev;
@@ -1248,6 +1249,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)

/* Mark regulator count uninitialized */
opp_table->regulator_count = -1;
+ opp_table->version = version;

opp_dev = _add_opp_dev(dev, opp_table);
if (!opp_dev) {
@@ -1332,7 +1334,8 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
* of adding an OPP table and others should wait for it to finish.
*/
struct opp_table *_add_opp_table_indexed(struct device *dev, int index,
- bool getclk)
+ bool getclk,
+ enum opp_table_version version)
{
struct opp_table *opp_table;

@@ -1367,7 +1370,7 @@ struct opp_table *_add_opp_table_indexed(struct device *dev, int index,

mutex_lock(&opp_table_lock);
} else {
- opp_table = _allocate_opp_table(dev, index);
+ opp_table = _allocate_opp_table(dev, index, version);

mutex_lock(&opp_table_lock);
if (!IS_ERR(opp_table))
@@ -1382,9 +1385,10 @@ struct opp_table *_add_opp_table_indexed(struct device *dev, int index,
return _update_opp_table_clk(dev, opp_table, getclk);
}

-static struct opp_table *_add_opp_table(struct device *dev, bool getclk)
+static struct opp_table *_add_opp_table(struct device *dev, bool getclk,
+ enum opp_table_version version)
{
- return _add_opp_table_indexed(dev, 0, getclk);
+ return _add_opp_table_indexed(dev, 0, getclk, version);
}

struct opp_table *dev_pm_opp_get_opp_table(struct device *dev)
@@ -1794,6 +1798,13 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,
unsigned long tol;
int ret;

+ if (opp_table->version != OPP_TABLE_VERSION_UNKNOWN &&
+ opp_table->version != OPP_TABLE_VERSION_1) {
+ dev_warn(dev, "%s: Mixing v1 and v2 OPP bindings not supported (%lu)\n",
+ __func__, freq);
+ return -EINVAL;
+ }
+
new_opp = _opp_allocate(opp_table);
if (!new_opp)
return -ENOMEM;
@@ -1844,7 +1855,7 @@ struct opp_table *dev_pm_opp_set_supported_hw(struct device *dev,
{
struct opp_table *opp_table;

- opp_table = _add_opp_table(dev, false);
+ opp_table = _add_opp_table(dev, false, OPP_TABLE_VERSION_2);
if (IS_ERR(opp_table))
return opp_table;

@@ -1932,7 +1943,7 @@ struct opp_table *dev_pm_opp_set_prop_name(struct device *dev, const char *name)
{
struct opp_table *opp_table;

- opp_table = _add_opp_table(dev, false);
+ opp_table = _add_opp_table(dev, false, OPP_TABLE_VERSION_2);
if (IS_ERR(opp_table))
return opp_table;

@@ -1994,7 +2005,7 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
struct regulator *reg;
int ret, i;

- opp_table = _add_opp_table(dev, false);
+ opp_table = _add_opp_table(dev, false, OPP_TABLE_VERSION_UNKNOWN);
if (IS_ERR(opp_table))
return opp_table;

@@ -2149,7 +2160,7 @@ struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char *name)
struct opp_table *opp_table;
int ret;

- opp_table = _add_opp_table(dev, false);
+ opp_table = _add_opp_table(dev, false, OPP_TABLE_VERSION_UNKNOWN);
if (IS_ERR(opp_table))
return opp_table;

@@ -2247,7 +2258,7 @@ struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev,
if (!set_opp)
return ERR_PTR(-EINVAL);

- opp_table = _add_opp_table(dev, false);
+ opp_table = _add_opp_table(dev, false, OPP_TABLE_VERSION_UNKNOWN);
if (IS_ERR(opp_table))
return opp_table;

@@ -2380,7 +2391,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
int index = 0, ret = -EINVAL;
const char * const *name = names;

- opp_table = _add_opp_table(dev, false);
+ opp_table = _add_opp_table(dev, false, OPP_TABLE_VERSION_UNKNOWN);
if (IS_ERR(opp_table))
return opp_table;

@@ -2637,7 +2648,7 @@ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
struct opp_table *opp_table;
int ret;

- opp_table = _add_opp_table(dev, true);
+ opp_table = _add_opp_table(dev, true, OPP_TABLE_VERSION_1);
if (IS_ERR(opp_table))
return PTR_ERR(opp_table);

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 440ab5a03df9..aa757f73277a 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -169,6 +169,9 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table,
return;
}

+ if (opp_table->version == OPP_TABLE_VERSION_1)
+ goto skip_required_opp;
+
count = of_count_phandle_with_args(np, "required-opps", NULL);
if (count <= 0)
goto put_np;
@@ -193,6 +196,7 @@ static void _opp_table_alloc_required_tables(struct opp_table *opp_table,
lazy = true;
}

+skip_required_opp:
/* Let's do the linking later on */
if (lazy)
list_add(&opp_table->lazy, &lazy_opp_tables);
@@ -883,6 +887,13 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
int ret;
bool rate_not_available = false;

+ if (opp_table->version != OPP_TABLE_VERSION_UNKNOWN &&
+ opp_table->version != OPP_TABLE_VERSION_2) {
+ dev_warn(dev, "%s: Mixing v1 and v2 OPP bindings not supported\n",
+ __func__);
+ return ERR_PTR(-EINVAL);
+ }
+
new_opp = _opp_allocate(opp_table);
if (!new_opp)
return ERR_PTR(-ENOMEM);
@@ -985,6 +996,9 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
opp_table->parsed_static_opps = 1;
mutex_unlock(&opp_table->lock);

+ WARN_ON(opp_table->version != OPP_TABLE_VERSION_UNKNOWN);
+ opp_table->version = OPP_TABLE_VERSION_2;
+
/* We have opp-table node now, iterate over it and add OPPs */
for_each_available_child_of_node(opp_table->np, np) {
opp = _opp_add_static_v2(opp_table, dev, np);
@@ -1020,6 +1034,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)

remove_static_opp:
_opp_remove_all_static(opp_table);
+ opp_table->version = OPP_TABLE_VERSION_UNKNOWN;

return ret;
}
@@ -1041,6 +1056,9 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)
opp_table->parsed_static_opps = 1;
mutex_unlock(&opp_table->lock);

+ WARN_ON(opp_table->version != OPP_TABLE_VERSION_UNKNOWN);
+ opp_table->version = OPP_TABLE_VERSION_1;
+
prop = of_find_property(dev->of_node, "operating-points", NULL);
if (!prop) {
ret = -ENODEV;
@@ -1080,6 +1098,7 @@ static int _of_add_opp_table_v1(struct device *dev, struct opp_table *opp_table)

remove_static_opp:
_opp_remove_all_static(opp_table);
+ opp_table->version = OPP_TABLE_VERSION_UNKNOWN;

return ret;
}
@@ -1100,7 +1119,8 @@ static int _of_add_table_indexed(struct device *dev, int index, bool getclk)
index = 0;
}

- opp_table = _add_opp_table_indexed(dev, index, getclk);
+ opp_table = _add_opp_table_indexed(dev, index, getclk,
+ OPP_TABLE_VERSION_UNKNOWN);
if (IS_ERR(opp_table))
return PTR_ERR(opp_table);

diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 45e3a55239a1..bd8349f8c7b0 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -124,6 +124,12 @@ enum opp_table_access {
OPP_TABLE_ACCESS_SHARED = 2,
};

+enum opp_table_version {
+ OPP_TABLE_VERSION_UNKNOWN = 0,
+ OPP_TABLE_VERSION_1 = 1,
+ OPP_TABLE_VERSION_2 = 2,
+};
+
/**
* struct opp_table - Device opp structure
* @node: table node - contains the devices with OPPs that
@@ -138,6 +144,7 @@ enum opp_table_access {
* @clock_latency_ns_max: Max clock latency in nanoseconds.
* @parsed_static_opps: Count of devices for which OPPs are initialized from DT.
* @shared_opp: OPP is shared between multiple devices.
+ * @version: OPP bindings version to disallow mixing (e.g. v1 and v2).
* @current_rate: Currently configured frequency.
* @current_opp: Currently configured OPP for the table.
* @suspend_opp: Pointer to OPP to be used during device suspend.
@@ -188,6 +195,7 @@ struct opp_table {

unsigned int parsed_static_opps;
enum opp_table_access shared_opp;
+ enum opp_table_version version;
unsigned long current_rate;
struct dev_pm_opp *current_opp;
struct dev_pm_opp *suspend_opp;
@@ -232,7 +240,9 @@ int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2);
int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available);
int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);
void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, int last_cpu);
-struct opp_table *_add_opp_table_indexed(struct device *dev, int index, bool getclk);
+struct opp_table *_add_opp_table_indexed(struct device *dev, int index,
+ bool getclk,
+ enum opp_table_version version);
void _put_opp_list_kref(struct opp_table *opp_table);
void _required_opps_available(struct dev_pm_opp *opp, int count);

--
2.32.0
\
 
 \ /
  Last update: 2022-04-04 14:38    [W:0.066 / U:1.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site