[PATCH] accel/qaic Fix HELLO_RESP mode sourced from wrong rx field

[PATCH] accel/qaic Fix HELLO_RESP mode sourced from wrong rx field

[Alok Tiwari]

sahara_hello() handles SAHARA_HELLO_CMD and validates fields from
context->rx->hello. When building the HELLO_RESP, copy the mode from the
received HELLO request (rx->hello.mode) rather than rx->hello_resp.mode.

Fixes: 76b801aa2198 ("accel/qaic: Add Sahara implementation for firmware loading")
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
 drivers/accel/qaic/sahara.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/accel/qaic/sahara.c b/drivers/accel/qaic/sahara.c
index fd3c3b2d1fd3..99bf832e2825 100644
--- a/drivers/accel/qaic/sahara.c
+++ b/drivers/accel/qaic/sahara.c
@@ -325,7 +325,7 @@ static void sahara_hello(struct sahara_context *context)
 	context->tx[0]->hello_resp.version = cpu_to_le32(SAHARA_VERSION);
 	context->tx[0]->hello_resp.version_compat = cpu_to_le32(SAHARA_VERSION);
 	context->tx[0]->hello_resp.status = cpu_to_le32(SAHARA_SUCCESS);
-	context->tx[0]->hello_resp.mode = context->rx->hello_resp.mode;
+	context->tx[0]->hello_resp.mode = context->rx->hello.mode;
 
 	ret = mhi_queue_buf(context->mhi_dev, DMA_TO_DEVICE, context->tx[0],
 			    SAHARA_HELLO_LENGTH, MHI_EOT);
-- 
2.50.1

Claude review: accel/qaic Fix HELLO_RESP mode sourced from wrong rx field

[Claude Code Review Bot]

Overall Series Review

Subject: accel/qaic Fix HELLO_RESP mode sourced from wrong rx field
Author: Alok Tiwari <alok.a.tiwari@oracle.com>
Patches: 1
Reviewed: 2026-03-03T13:59:36.041240

---

This is a single-patch submission fixing a copy-paste / wrong-union-member bug in the QAIC Sahara firmware loading protocol handler. The fix is correct, clearly described, and minimal. The bug likely had no practical impact due to overlapping union layout, but it is still the right thing to fix for correctness and code clarity.

---
Generated by Claude Code Patch Reviewer

Claude review: accel/qaic Fix HELLO_RESP mode sourced from wrong rx field

[Claude Code Review Bot]

Patch Review

**Verdict: Correct fix, minor subject-line nit.**

The bug: In `sahara_hello()`, when building the `HELLO_RESP` packet to send back, the code was reading the mode from `context->rx->hello_resp.mode` instead of `context->rx->hello.mode`. Since this function handles an incoming `HELLO_CMD` (not a `HELLO_RESP`), the received packet's payload should be accessed via the `.hello` union member.

Looking at the struct definition at `sahara.c:63-98`:

```c
union {
    struct {
        __le32 version;
        __le32 version_compat;
        __le32 max_length;
        __le32 mode;
    } hello;
    struct {
        __le32 version;
        __le32 version_compat;
        __le32 status;
        __le32 mode;
    } hello_resp;
    ...
};
```

Both `.hello` and `.hello_resp` have `.mode` at the same offset (the 4th `__le32` field in the union), so **in practice the old code would read the correct value**. This makes it a correctness/readability fix rather than a functional bug fix. The commit message says "Fix" which implies a functional bug; it might be more accurate to describe this as "Use correct union member" to avoid overstating the impact.

**Nits:**

1. **Subject line missing colon**: The subject reads `accel/qaic Fix HELLO_RESP...` but kernel convention is `accel/qaic: Fix HELLO_RESP...` (colon after subsystem prefix).

2. **Commit message could note no runtime impact**: Since the union layout means both members alias to the same offset, the commit message could mention that the old code happened to produce the correct value but accessed the wrong semantic field. This helps reviewers understand the actual risk level.

Otherwise, the one-line change is obviously correct — the entire rest of `sahara_hello()` consistently uses `context->rx->hello.*` to access the received packet fields (lines 299–302, 306, 309–311, 315–319), and only the mode assignment on the old line 322 was the odd one out.

**Reviewed-by worthy**: Yes, with the subject-line colon fix.

---
Generated by Claude Code Patch Reviewer