struct amdxdna_cmd::header packs STATE, OPCODE, COUNT and EXTRA_CU_MASK into one u32 that lives in a BO user space keeps mapped. The driver re-reads it on every accessor call and read-modify-writes STATE in place, with plain accesses that let the compiler split or refetch either side.
Annotate them, as the UMQ ring indices already are. This does not make the update atomic against user space, it only stops the compiler from making it worse. Signed-off-by: Taimuraz Kaitmazov <[email protected]> --- drivers/accel/amdxdna/amdxdna_ctx.c | 14 +++++++++----- drivers/accel/amdxdna/amdxdna_ctx.h | 11 +++++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c index 9f44e3918bc1..148ae51db0f1 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.c +++ b/drivers/accel/amdxdna/amdxdna_ctx.c @@ -123,9 +123,10 @@ void *amdxdna_cmd_get_payload(struct amdxdna_gem_obj *abo, u32 *size) if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) num_masks = 0; else - num_masks = 1 + FIELD_GET(AMDXDNA_CMD_EXTRA_CU_MASK, cmd->header); + num_masks = 1 + FIELD_GET(AMDXDNA_CMD_EXTRA_CU_MASK, + READ_ONCE(cmd->header)); - count = FIELD_GET(AMDXDNA_CMD_COUNT, cmd->header); + count = FIELD_GET(AMDXDNA_CMD_COUNT, READ_ONCE(cmd->header)); if (unlikely(count <= num_masks || count * sizeof(u32) + offsetof(struct amdxdna_cmd, data[0]) > @@ -173,7 +174,7 @@ u32 amdxdna_cmd_get_cu_idx(struct amdxdna_gem_obj *abo) if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) return INVALID_CU_IDX; - num_masks = 1 + FIELD_GET(AMDXDNA_CMD_EXTRA_CU_MASK, cmd->header); + num_masks = 1 + FIELD_GET(AMDXDNA_CMD_EXTRA_CU_MASK, READ_ONCE(cmd->header)); cu_mask = cmd->data; for (i = 0; i < num_masks; i++) { if (cu_mask[i]) @@ -191,12 +192,15 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo, struct amdxdna_client *client = job->hwctx->client; struct amdxdna_cmd *cmd = amdxdna_gem_vmap(abo); struct amdxdna_cmd_chain *cc = NULL; + u32 header; if (!cmd) return -ENOMEM; - cmd->header &= ~AMDXDNA_CMD_STATE; - cmd->header |= FIELD_PREP(AMDXDNA_CMD_STATE, error_state); + header = READ_ONCE(cmd->header); + header &= ~AMDXDNA_CMD_STATE; + header |= FIELD_PREP(AMDXDNA_CMD_STATE, error_state); + WRITE_ONCE(cmd->header, header); if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) { u32 ccnt; diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h index f6529d512217..48e1fcfd818f 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.h +++ b/drivers/accel/amdxdna/amdxdna_ctx.h @@ -169,19 +169,22 @@ amdxdna_cmd_get_op(struct amdxdna_gem_obj *abo) if (!cmd) return ERT_INVALID_CMD; - return FIELD_GET(AMDXDNA_CMD_OPCODE, cmd->header); + return FIELD_GET(AMDXDNA_CMD_OPCODE, READ_ONCE(cmd->header)); } static inline void amdxdna_cmd_set_state(struct amdxdna_gem_obj *abo, enum ert_cmd_state s) { struct amdxdna_cmd *cmd = amdxdna_gem_vmap(abo); + u32 header; if (!cmd) return; - cmd->header &= ~AMDXDNA_CMD_STATE; - cmd->header |= FIELD_PREP(AMDXDNA_CMD_STATE, s); + header = READ_ONCE(cmd->header); + header &= ~AMDXDNA_CMD_STATE; + header |= FIELD_PREP(AMDXDNA_CMD_STATE, s); + WRITE_ONCE(cmd->header, header); } static inline enum ert_cmd_state @@ -192,7 +195,7 @@ amdxdna_cmd_get_state(struct amdxdna_gem_obj *abo) if (!cmd) return ERT_CMD_STATE_INVALID; - return FIELD_GET(AMDXDNA_CMD_STATE, cmd->header); + return FIELD_GET(AMDXDNA_CMD_STATE, READ_ONCE(cmd->header)); } void *amdxdna_cmd_get_payload(struct amdxdna_gem_obj *abo, u32 *size); -- 2.55.0
