From: "Cédric Le Goater" <clg@redhat.com>
To: qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: "Ashish Anand" <ashish.a6@samsung.com>,
"Jamin Lin" <jamin_lin@aspeedtech.com>,
"Cédric Le Goater" <clg@redhat.com>
Subject: [PULL 9/9] hw/i3c/dw-i3c: Fix BCR/DCR extraction and PID assembly during ENTDAA
Date: Tue, 12 May 2026 19:13:54 +0200 [thread overview]
Message-ID: <20260512171354.4183887-10-clg@redhat.com> (raw)
In-Reply-To: <20260512171354.4183887-1-clg@redhat.com>
From: Ashish Anand <ashish.a6@samsung.com>
The target_info union in dw_i3c_addr_assign_cmd() declares pid, bcr,
and dcr as separate union members, causing them to all alias b[0]
rather than their correct positions in the ENTDAA response buffer.
This results in dw_i3c_update_char_table() being called with BCR and
DCR both read from b[0] instead of b[6] and b[7] respectively,
corrupting the device characteristics table on every ENTDAA operation.
Fix by replacing the broken members with uint64_t d and extracting fields
per the I3C spec ENTDAA wire format.
Additionally, dw_i3c_update_char_table() incorrectly splits PID across
LOC1 and LOC2 at bit 32. Per the Linux kernel HCI driver
(drivers/i3c/master/mipi-i3c-hci/dct_v1.c), the DCT layout requires
LOC1 to hold pid[47:16] and LOC2 to hold pid[15:0]. Fix the split
accordingly.
Signed-off-by: Ashish Anand <ashish.a6@samsung.com>
Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com>
Link: https://lore.kernel.org/qemu-devel/20260505134002.509037-1-ashish.a6@samsung.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
include/hw/i3c/i3c.h | 7 +++++++
hw/i3c/dw-i3c.c | 16 +++++++---------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/hw/i3c/i3c.h b/include/hw/i3c/i3c.h
index 6ba90793ad01..dcf8d9b1435b 100644
--- a/include/hw/i3c/i3c.h
+++ b/include/hw/i3c/i3c.h
@@ -138,6 +138,13 @@ struct I3CTarget {
uint8_t static_address;
uint8_t dcr;
uint8_t bcr;
+ /*
+ * Provisioned ID. Since core.c sends this LSB-first during ENTDAA
+ * via (pid >> (offset * 8)) & 0xff, targets must store it
+ * pre-reversed so that pid[47:40] goes on the wire first, as
+ * required by the I3C spec.
+ * e.g. for a device with pid 0xAABBCCDDEEFF, store 0xFFEEDDCCBBAA.
+ */
uint64_t pid;
/* CCC State tracking. */
diff --git a/hw/i3c/dw-i3c.c b/hw/i3c/dw-i3c.c
index d87d42be8914..17ff484c5df1 100644
--- a/hw/i3c/dw-i3c.c
+++ b/hw/i3c/dw-i3c.c
@@ -1459,11 +1459,10 @@ static void dw_i3c_update_char_table(DWI3C *s, uint8_t offset, uint64_t pid,
P_DEV_CHAR_TABLE_START_ADDR) /
sizeof(uint32_t)) +
(offset * sizeof(uint32_t));
- s->regs[dev_index] = pid & 0xffffffff;
- pid >>= 32;
+ s->regs[dev_index] = (pid >> 16) & 0xffffffff;
s->regs[dev_index + 1] = FIELD_DP32(s->regs[dev_index + 1],
DEVICE_CHARACTERISTIC_TABLE_LOC2,
- MSB_PID, pid);
+ MSB_PID, pid & 0xffff);
s->regs[dev_index + 2] = FIELD_DP32(s->regs[dev_index + 2],
DEVICE_CHARACTERISTIC_TABLE_LOC3, DCR,
dcr);
@@ -1507,10 +1506,9 @@ static void dw_i3c_addr_assign_cmd(DWI3C *s, DWI3CAddrAssignCmd cmd)
for (i = 0; i < cmd.dev_count; i++) {
uint8_t addr = dw_i3c_target_addr(s, cmd.dev_index + i);
union {
- uint64_t pid:48;
- uint8_t bcr;
- uint8_t dcr;
+ uint64_t d;
uint32_t w[2];
+ /* Per I3C spec: b[0]=PID MSB, b[5]=PID LSB, b[6]=BCR, b[7]=DCR */
uint8_t b[8];
} target_info;
@@ -1544,9 +1542,9 @@ static void dw_i3c_addr_assign_cmd(DWI3C *s, DWI3CAddrAssignCmd cmd)
err = DW_I3C_RESP_QUEUE_ERR_DAA_NACK;
break;
}
- dw_i3c_update_char_table(s, cmd.dev_index + i,
- target_info.pid, target_info.bcr,
- target_info.dcr, addr);
+ uint64_t pid = be64_to_cpu(target_info.d) >> 16;
+ dw_i3c_update_char_table(s, cmd.dev_index + i, pid, target_info.b[6],
+ target_info.b[7], addr);
/* Push the PID, BCR, and DCR to the RX queue. */
dw_i3c_push_rx(s, target_info.w[0]);
--
2.54.0
next prev parent reply other threads:[~2026-05-12 17:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-12 17:13 [PULL 0/9] aspeed queue Cédric Le Goater
2026-05-12 17:13 ` [PULL 1/9] hw/misc/aspeed_sbc: Add bounds checking for OTP write operations Cédric Le Goater
2026-05-12 17:13 ` [PULL 2/9] aspeed/hace: Fix out-of-bounds read in has_padding() Cédric Le Goater
2026-05-12 17:13 ` [PULL 3/9] aspeed/hace: Prevent total_req_len overflow Cédric Le Goater
2026-05-12 17:13 ` [PULL 4/9] aspeed/hace: Fix mapped address may not be unmapped issue Cédric Le Goater
2026-05-12 17:13 ` [PULL 5/9] hw/arm: Remove sonorapass-bmc machine Cédric Le Goater
2026-05-12 17:13 ` [PULL 6/9] hw/arm: Remove qcom-dc-scm-v1-bmc and qcom-firework-bmc machines Cédric Le Goater
2026-05-12 17:13 ` [PULL 7/9] hw/arm: Remove fp5280g2-bmc machine Cédric Le Goater
2026-05-12 17:13 ` [PULL 8/9] hw/arm: Remove fby35 machine Cédric Le Goater
2026-05-12 17:13 ` Cédric Le Goater [this message]
2026-05-14 16:26 ` [PULL 0/9] aspeed queue Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260512171354.4183887-10-clg@redhat.com \
--to=clg@redhat.com \
--cc=ashish.a6@samsung.com \
--cc=jamin_lin@aspeedtech.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.