Re: [PATCH V3 5/5] accel/amdxdna: Add zero check for pad in ioctl input structures
From: Lizhi Hou
Date: Mon Dec 16 2024 - 16:18:09 EST
On 12/16/24 12:38, Mario Limonciello wrote:
On 12/13/2024 17:29, Lizhi Hou wrote:
For input structures, it is better to check if the pad is zero.
Thus, the pad bytes might be usable in the future.
IIRC you should pick up:
Suggested-by: Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>
Sure. Will add it.
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
---
drivers/accel/amdxdna/aie2_ctx.c | 3 +++
drivers/accel/amdxdna/aie2_message.c | 3 +++
drivers/accel/amdxdna/amdxdna_ctx.c | 6 ++++++
drivers/accel/amdxdna/amdxdna_gem.c | 2 +-
include/uapi/drm/amdxdna_accel.h | 11 +++++------
5 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_ctx.c
b/drivers/accel/amdxdna/aie2_ctx.c
index cdeead75c6f5..9facf45818f9 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -690,6 +690,9 @@ static int aie2_hwctx_cu_config(struct
amdxdna_hwctx *hwctx, void *buf, u32 size
int ret;
XDNA_DBG(xdna, "Config %d CU to %s", config->num_cus,
hwctx->name);
+ if (XDNA_MBZ_DBG(xdna, config->pad, sizeof(config->pad)))
+ return -EINVAL;
+
if (hwctx->status != HWCTX_STAT_INIT) {
XDNA_ERR(xdna, "Not support re-config CU");
return -EINVAL;
diff --git a/drivers/accel/amdxdna/aie2_message.c
b/drivers/accel/amdxdna/aie2_message.c
index b2ca78cfd0a7..9e2c9a44f76a 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -395,6 +395,9 @@ int aie2_config_cu(struct amdxdna_hwctx *hwctx)
for (i = 0; i < hwctx->cus->num_cus; i++) {
struct amdxdna_cu_config *cu = &hwctx->cus->cu_configs[i];
+ if (XDNA_MBZ_DBG(xdna, cu->pad, sizeof(cu->pad)))
+ return -EINVAL;
+
gobj = drm_gem_object_lookup(hwctx->client->filp, cu->cu_bo);
if (!gobj) {
XDNA_ERR(xdna, "Lookup GEM object failed");
diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c
b/drivers/accel/amdxdna/amdxdna_ctx.c
index 324f35c43f6c..d11b1c83d9c3 100644
--- a/drivers/accel/amdxdna/amdxdna_ctx.c
+++ b/drivers/accel/amdxdna/amdxdna_ctx.c
@@ -243,6 +243,9 @@ int amdxdna_drm_destroy_hwctx_ioctl(struct
drm_device *dev, void *data, struct d
struct amdxdna_hwctx *hwctx;
int ret = 0, idx;
+ if (XDNA_MBZ_DBG(xdna, &args->pad, sizeof(args->pad)))
+ return -EINVAL;
+
if (!drm_dev_enter(dev, &idx))
return -ENODEV;
@@ -277,6 +280,9 @@ int amdxdna_drm_config_hwctx_ioctl(struct
drm_device *dev, void *data, struct dr
void *buf;
u64 val;
+ if (XDNA_MBZ_DBG(xdna, &args->pad, sizeof(args->pad)))
+ return -EINVAL;
+
if (!xdna->dev_info->ops->hwctx_config)
return -EOPNOTSUPP;
diff --git a/drivers/accel/amdxdna/amdxdna_gem.c
b/drivers/accel/amdxdna/amdxdna_gem.c
index 4dfeca306d98..606433d73236 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -552,7 +552,7 @@ int amdxdna_drm_get_bo_info_ioctl(struct
drm_device *dev, void *data, struct drm
struct drm_gem_object *gobj;
int ret = 0;
- if (args->ext || args->ext_flags)
+ if (args->ext || args->ext_flags || args->pad)
return -EINVAL;
gobj = drm_gem_object_lookup(filp, args->handle);
diff --git a/include/uapi/drm/amdxdna_accel.h
b/include/uapi/drm/amdxdna_accel.h
index e4edb52bc27b..a706ead39082 100644
--- a/include/uapi/drm/amdxdna_accel.h
+++ b/include/uapi/drm/amdxdna_accel.h
@@ -87,7 +87,7 @@ struct amdxdna_drm_create_hwctx {
/**
* struct amdxdna_drm_destroy_hwctx - Destroy hardware context.
* @handle: Hardware context handle.
- * @pad: Structure padding.
+ * @pad: MBZ.
*/
struct amdxdna_drm_destroy_hwctx {
__u32 handle;
@@ -98,7 +98,7 @@ struct amdxdna_drm_destroy_hwctx {
* struct amdxdna_cu_config - configuration for one CU
* @cu_bo: CU configuration buffer bo handle.
* @cu_func: Function of a CU.
- * @pad: Structure padding.
+ * @pad: MBZ.
*/
struct amdxdna_cu_config {
__u32 cu_bo;
@@ -109,7 +109,7 @@ struct amdxdna_cu_config {
/**
* struct amdxdna_hwctx_param_config_cu - configuration for CUs in
hardware context
* @num_cus: Number of CUs to configure.
- * @pad: Structure padding.
+ * @pad: MBZ.
* @cu_configs: Array of CU configurations of struct
amdxdna_cu_config.
*/
struct amdxdna_hwctx_param_config_cu {
@@ -122,7 +122,6 @@ enum amdxdna_drm_config_hwctx_param {
DRM_AMDXDNA_HWCTX_CONFIG_CU,
DRM_AMDXDNA_HWCTX_ASSIGN_DBG_BUF,
DRM_AMDXDNA_HWCTX_REMOVE_DBG_BUF,
- DRM_AMDXDNA_HWCTX_CONFIG_NUM
This change actually meant here? It seems like it should be it's own
change.
Agree. Hopefully, the patch 1 - 4 are good to go and I do not need to
resend them. :)
Thanks,
Lizhi
};
/**
@@ -133,7 +132,7 @@ enum amdxdna_drm_config_hwctx_param {
* @param_val: A structure specified by the param_type struct member.
* @param_val_size: Size of the parameter buffer pointed to by the
param_val.
* If param_val is not a pointer, driver can ignore this.
- * @pad: Structure padding.
+ * @pad: MBZ.
*
* Note: if the param_val is a pointer pointing to a buffer, the
maximum size
* of the buffer is 4KiB(PAGE_SIZE).
@@ -175,7 +174,7 @@ struct amdxdna_drm_create_bo {
* @ext: MBZ.
* @ext_flags: MBZ.
* @handle: DRM buffer object handle.
- * @pad: Structure padding.
+ * @pad: MBZ.
* @map_offset: Returned DRM fake offset for mmap().
* @vaddr: Returned user VA of buffer. 0 in case user needs mmap().
* @xdna_addr: Returned XDNA device virtual address.