| 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
| ||
|
Message-Id: <20250117-drm-rework-dpcd-access-v1-2-7fc020e04dbc@linaro.org>
Date: Fri, 17 Jan 2025 10:56:37 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>,
Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
Jani Nikula <jani.nikula@...ux.intel.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org
Subject: [PATCH RFC 2/7] drm/display: dp: implement new access helpers
Existing DPCD access functions return an error code or the number of
bytes being read / write in case of partial access. However a lot of
drivers either (incorrectly) ignore partial access or mishandle error
codes. In other cases this results in a boilerplate code which compares
returned value with the size.
Implement new set of DPCD access helpers, which ignore partial access,
always return 0 or an error code. Implement existing helpers using the
new functions to ensure backwards compatibility.
Suggested-by: Jani Nikula <jani.nikula@...ux.intel.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
---
drivers/gpu/drm/display/drm_dp_helper.c | 42 +++++++-------
drivers/gpu/drm/display/drm_dp_mst_topology.c | 27 +++++----
include/drm/display/drm_dp_helper.h | 81 ++++++++++++++++++++++++++-
include/drm/display/drm_dp_mst_helper.h | 10 ++--
4 files changed, 119 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 809c65dcb58983693fb335b88759a66919410114..5a693f2779284467e2d05b9d8b2c2bee0ad6c112 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -495,13 +495,13 @@ EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
static inline void
drm_dp_dump_access(const struct drm_dp_aux *aux,
- u8 request, uint offset, void *buffer, int ret)
+ u8 request, uint offset, void *buffer, size_t size, int ret)
{
const char *arrow = request == DP_AUX_NATIVE_READ ? "->" : "<-";
- if (ret > 0)
+ if (ret == 0)
drm_dbg_dp(aux->drm_dev, "%s: 0x%05x AUX %s (ret=%3d) %*ph\n",
- aux->name, offset, arrow, ret, min(ret, 20), buffer);
+ aux->name, offset, arrow, ret, min_t(int, size, 20), buffer);
else
drm_dbg_dp(aux->drm_dev, "%s: 0x%05x AUX %s (ret=%3d)\n",
aux->name, offset, arrow, ret);
@@ -559,8 +559,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
if (ret >= 0) {
native_reply = msg.reply & DP_AUX_NATIVE_REPLY_MASK;
if (native_reply == DP_AUX_NATIVE_REPLY_ACK) {
- if (ret == size)
+ if (ret == size) {
+ ret = 0;
goto unlock;
+ }
ret = -EPROTO;
} else
@@ -602,9 +604,9 @@ int drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset)
int ret;
ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, &buffer, 1);
- WARN_ON(ret == 0);
+ WARN_ON(ret == -EPROTO);
- drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, &buffer, ret);
+ drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, &buffer, 1, ret);
return ret < 0 ? ret : 0;
}
@@ -634,21 +636,21 @@ void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered)
EXPORT_SYMBOL(drm_dp_dpcd_set_powered);
/**
- * drm_dp_dpcd_read() - read a series of bytes from the DPCD
+ * drm_dp_dpcd_read_data() - read a series of bytes from the DPCD
* @aux: DisplayPort AUX channel (SST or MST)
* @offset: address of the (first) register to read
* @buffer: buffer to store the register values
* @size: number of bytes in @buffer
*
- * Returns the number of bytes transferred on success, or a negative error
+ * Returns zero (0) on success, or a negative error
* code on failure. -EIO is returned if the request was NAKed by the sink or
* if the retry count was exceeded. If not all bytes were transferred, this
* function returns -EPROTO. Errors from the underlying AUX channel transfer
* function, with the exception of -EBUSY (which causes the transaction to
* be retried), are propagated to the caller.
*/
-ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
- void *buffer, size_t size)
+int drm_dp_dpcd_read_data(struct drm_dp_aux *aux, unsigned int offset,
+ void *buffer, size_t size)
{
int ret;
@@ -671,45 +673,45 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
}
if (aux->is_remote)
- ret = drm_dp_mst_dpcd_read(aux, offset, buffer, size);
+ ret = drm_dp_mst_dpcd_read_data(aux, offset, buffer, size);
else
ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset,
buffer, size);
- drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, ret);
+ drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, size, ret);
return ret;
}
-EXPORT_SYMBOL(drm_dp_dpcd_read);
+EXPORT_SYMBOL(drm_dp_dpcd_read_data);
/**
- * drm_dp_dpcd_write() - write a series of bytes to the DPCD
+ * drm_dp_dpcd_write_data() - write a series of bytes to the DPCD
* @aux: DisplayPort AUX channel (SST or MST)
* @offset: address of the (first) register to write
* @buffer: buffer containing the values to write
* @size: number of bytes in @buffer
*
- * Returns the number of bytes transferred on success, or a negative error
+ * Returns zero (0) on success, or a negative error
* code on failure. -EIO is returned if the request was NAKed by the sink or
* if the retry count was exceeded. If not all bytes were transferred, this
* function returns -EPROTO. Errors from the underlying AUX channel transfer
* function, with the exception of -EBUSY (which causes the transaction to
* be retried), are propagated to the caller.
*/
-ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
- void *buffer, size_t size)
+int drm_dp_dpcd_write_data(struct drm_dp_aux *aux, unsigned int offset,
+ void *buffer, size_t size)
{
int ret;
if (aux->is_remote)
- ret = drm_dp_mst_dpcd_write(aux, offset, buffer, size);
+ ret = drm_dp_mst_dpcd_write_data(aux, offset, buffer, size);
else
ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset,
buffer, size);
- drm_dp_dump_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, ret);
+ drm_dp_dump_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, size, ret);
return ret;
}
-EXPORT_SYMBOL(drm_dp_dpcd_write);
+EXPORT_SYMBOL(drm_dp_dpcd_write_data);
/**
* drm_dp_dpcd_read_link_status() - read DPCD link status (bytes 0x202-0x207)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index f8cd094efa3c0bd6f75b52a0410b0910d8026a76..f8db5be53a33e87e94b864ba48151354e091f5aa 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2128,20 +2128,20 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 new_pdt,
}
/**
- * drm_dp_mst_dpcd_read() - read a series of bytes from the DPCD via sideband
+ * drm_dp_mst_dpcd_read_data() - read a series of bytes from the DPCD via sideband
* @aux: Fake sideband AUX CH
* @offset: address of the (first) register to read
* @buffer: buffer to store the register values
* @size: number of bytes in @buffer
*
* Performs the same functionality for remote devices via
- * sideband messaging as drm_dp_dpcd_read() does for local
+ * sideband messaging as drm_dp_dpcd_read_data() does for local
* devices via actual AUX CH.
*
- * Return: Number of bytes read, or negative error code on failure.
+ * Return: Zero (0) on success, or negative error code on failure.
*/
-ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
- unsigned int offset, void *buffer, size_t size)
+int drm_dp_mst_dpcd_read_data(struct drm_dp_aux *aux,
+ unsigned int offset, void *buffer, size_t size)
{
struct drm_dp_mst_port *port = container_of(aux, struct drm_dp_mst_port,
aux);
@@ -2151,20 +2151,20 @@ ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
}
/**
- * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via sideband
+ * drm_dp_mst_dpcd_write_data() - write a series of bytes to the DPCD via sideband
* @aux: Fake sideband AUX CH
* @offset: address of the (first) register to write
* @buffer: buffer containing the values to write
* @size: number of bytes in @buffer
*
* Performs the same functionality for remote devices via
- * sideband messaging as drm_dp_dpcd_write() does for local
+ * sideband messaging as drm_dp_dpcd_write_data() does for local
* devices via actual AUX CH.
*
- * Return: number of bytes written on success, negative error code on failure.
+ * Return: zero (0) on success, negative error code on failure.
*/
-ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
- unsigned int offset, void *buffer, size_t size)
+int drm_dp_mst_dpcd_write_data(struct drm_dp_aux *aux,
+ unsigned int offset, void *buffer, size_t size)
{
struct drm_dp_mst_port *port = container_of(aux, struct drm_dp_mst_port,
aux);
@@ -3490,9 +3490,8 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
goto fail_free;
}
- ret = min_t(size_t, txmsg->reply.u.remote_dpcd_read_ack.num_bytes,
- size);
- memcpy(bytes, txmsg->reply.u.remote_dpcd_read_ack.bytes, ret);
+ memcpy(bytes, txmsg->reply.u.remote_dpcd_read_ack.bytes, size);
+ ret = 0;
fail_free:
kfree(txmsg);
@@ -3530,7 +3529,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK)
ret = -EIO;
else
- ret = size;
+ ret = 0;
}
kfree(txmsg);
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 8f4054a560396a43750570a8c2e95624039ab8ad..548237a81ef0359dab1ed7df6ef0fd1e0c76e0c5 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -522,10 +522,85 @@ struct drm_dp_aux {
int drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset);
void drm_dp_dpcd_set_powered(struct drm_dp_aux *aux, bool powered);
-ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
- void *buffer, size_t size);
-ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
+
+int drm_dp_dpcd_read_data(struct drm_dp_aux *aux, unsigned int offset,
void *buffer, size_t size);
+int drm_dp_dpcd_write_data(struct drm_dp_aux *aux, unsigned int offset,
+ void *buffer, size_t size);
+
+/**
+ * drm_dp_dpcd_read() - read a series of bytes from the DPCD
+ * @aux: DisplayPort AUX channel (SST or MST)
+ * @offset: address of the (first) register to read
+ * @buffer: buffer to store the register values
+ * @size: number of bytes in @buffer
+ *
+ * Deprecated wrapper around drm_dp_dpcd_read().
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure.
+ */
+static inline ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux,
+ unsigned int offset,
+ void *buffer, size_t size)
+{
+ int ret = drm_dp_dpcd_read_data(aux, offset, buffer, size);
+
+ if (ret < 0)
+ return ret;
+
+ return size;
+}
+
+/**
+ * drm_dp_dpcd_read_byte() - read a single byte from the DPCD
+ * @aux: DisplayPort AUX channel
+ * @offset: address of the register to read
+ * @valuep: location where the value of the register will be stored
+ *
+ * Returns zero (0) on success, or a negative error code on failure.
+ */
+static inline int drm_dp_dpcd_read_byte(struct drm_dp_aux *aux,
+ unsigned int offset, u8 *valuep)
+{
+ return drm_dp_dpcd_read_data(aux, offset, valuep, 1);
+}
+
+/**
+ * drm_dp_dpcd_write_byte() - write a single byte to the DPCD
+ * @aux: DisplayPort AUX channel
+ * @offset: address of the register to write
+ * @value: value to write to the register
+ *
+ * Returns zero (0) on success, or a negative error code on failure.
+ */
+static inline int drm_dp_dpcd_write_byte(struct drm_dp_aux *aux,
+ unsigned int offset, u8 value)
+{
+ return drm_dp_dpcd_write_data(aux, offset, &value, 1);
+}
+
+/**
+ * drm_dp_dpcd_write() - write a series of bytes from the DPCD
+ * @aux: DisplayPort AUX channel (SST or MST)
+ * @offset: address of the (first) register to write
+ * @buffer: buffer containing the values to write
+ * @size: number of bytes in @buffer
+ *
+ * Deprecated wrapper around drm_dp_dpcd_write().
+ * Returns the number of bytes transferred on success, or a negative error
+ * code on failure.
+ */
+static inline ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux,
+ unsigned int offset,
+ void *buffer, size_t size)
+{
+ int ret = drm_dp_dpcd_write_data(aux, offset, buffer, size);
+
+ if (ret < 0)
+ return ret;
+
+ return size;
+}
/**
* drm_dp_dpcd_readb() - read a single byte from the DPCD
diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
index a80ba457a858f36ac2110a6fdd91d8a1570b58e1..d527b323a7a8c92b93280fcc8cd3025e21cdcf02 100644
--- a/include/drm/display/drm_dp_mst_helper.h
+++ b/include/drm/display/drm_dp_mst_helper.h
@@ -899,10 +899,12 @@ int __must_check
drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr,
bool sync);
-ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
- unsigned int offset, void *buffer, size_t size);
-ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
- unsigned int offset, void *buffer, size_t size);
+int drm_dp_mst_dpcd_read_data(struct drm_dp_aux *aux,
+ unsigned int offset,
+ void *buffer, size_t size);
+int drm_dp_mst_dpcd_write_data(struct drm_dp_aux *aux,
+ unsigned int offset,
+ void *buffer, size_t size);
int drm_dp_mst_connector_late_register(struct drm_connector *connector,
struct drm_dp_mst_port *port);
--
2.39.5
Powered by blists - more mailing lists