Update cert firmware version to print hot fix and build numbers#1164
Update cert firmware version to print hot fix and build numbers#1164NishadSaraf merged 1 commit intoamd:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the VE2 “cert/UC firmware version” reporting path so that hotfix (patch) and build numbers are surfaced end-to-end (kernel → ioctl/uapi struct → shim query output).
Changes:
- Populate shim’s
firmware_versionpatch/build from the ioctl’shotfix/buildfields instead of forcing them to 0. - Extend the kernel’s cached firmware version and debug logging to include hotfix/build.
- Update the local uAPI query struct to include
hotfix/buildwhile keeping the overall struct size stable.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/shim_ve2/xdna_device.cpp |
Uses fw_version.hotfix/build to fill patch/build in the shim query result. |
src/include/uapi/drm_local/amdxdna_accel.h |
Adds hotfix/build to the VE2 firmware query struct and adjusts git_hash length to preserve struct size. |
src/driver/amdxdna/ve2_fw.h |
Extends the kernel-side cached firmware version struct with hotfix/build. |
src/driver/amdxdna/ve2_fw.c |
Stores hotfix/build from firmware and logs full major.minor.hotfix.build. |
src/driver/amdxdna/ve2_debug.c |
Returns hotfix/build to userspace via the query ioctl. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct ve2_firmware_version { | ||
| u8 major; | ||
| u8 minor; | ||
| char git_hash[VE2_FW_HASH_STRING_LENGTH]; | ||
| char date[VE2_FW_DATE_STRING_LENGTH]; | ||
| u8 hotfix; | ||
| u8 build; | ||
| }; |
There was a problem hiding this comment.
struct ve2_firmware_version is smaller than VE2_CERT_VERSION_SIZE (0x40). In ve2_store_firmware_version() the driver reads VE2_CERT_VERSION_SIZE bytes from firmware into a buffer sized as sizeof(struct ve2_firmware_version), which can overflow and corrupt memory. Consider defining an on-wire/padded 0x40-byte struct (or adding an explicit reserved/padding array + static assert on size), or changing the read size to sizeof(*version)/allocating VE2_CERT_VERSION_SIZE bytes and then parsing safely.
| c_version->hotfix = version->hotfix; | ||
| c_version->build = version->build; |
There was a problem hiding this comment.
Assigning c_version->hotfix/build from version->hotfix/build relies on version containing valid data, but ve2_store_firmware_version() currently reads VE2_CERT_VERSION_SIZE (0x40) bytes into version allocated with sizeof(*version) (see earlier in the function). This can overflow and also make these new fields unreliable. Please make the read/allocation size match the buffer size (or read only sizeof(*version) / use a padded on-wire struct).
There was a problem hiding this comment.
Please remove the mention of "ve2" from all data structures in amdxdna_accel.h since this file contains interfaces common across all aie2p/aie4/ve2 drivers.
3094315 to
d1e892c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/driver/amdxdna/ve2_debug.c:431
- For GET_INFO queries, other handlers in this repo either copy out
min(args->buffer_size, sizeof(version))or return-ENOBUFSand updateargs->buffer_sizewith the required size. Hereve2_get_firmware_version()returns-EINVALwhen the buffer is too small and does not report the required size, which makes the ioctl harder to use and inconsistent with e.g.aie4_query_firmware_version()/aie2_query_firmware_version().
if (args->buffer_size < sizeof(version))
return -EINVAL;
if (copy_to_user((u64_to_user_ptr(args->buffer)), &version, sizeof(version)))
return -EFAULT;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| u8 build; | ||
| }; | ||
|
|
There was a problem hiding this comment.
ve2_store_firmware_version() reads VE2_CERT_VERSION_SIZE (0x40) bytes into a struct ve2_firmware_version buffer, but this struct is smaller (currently 56 bytes). This creates an out-of-bounds write in the read path. Fix by making the struct layout match the on-device blob size (e.g., add explicit padding / __packed + static assert), or change the read size to sizeof(struct ve2_firmware_version) if 0x40 is not required.
| u8 build; | |
| }; | |
| u8 build; | |
| /* Pad to match VE2_CERT_VERSION_SIZE (0x40) bytes from firmware blob */ | |
| u8 reserved[8]; | |
| }; | |
| _Static_assert(sizeof(struct ve2_firmware_version) == VE2_CERT_VERSION_SIZE, | |
| "ve2_firmware_version size must match VE2_CERT_VERSION_SIZE"); |
| struct amdxdna_dev_hdl *hdl = xdna->dev_handle; | ||
| struct ve2_firmware_version *fver = &hdl->fw_version; | ||
| struct amdxdna_drm_query_ve2_firmware_version version; | ||
| struct amdxdna_drm_query_firmware_version version; | ||
|
|
||
| if (!fver) |
There was a problem hiding this comment.
fver is assigned as &hdl->fw_version, so the if (!fver) check can never be true. If the intent is to validate pointers, the code should check xdna/xdna->dev_handle before dereferencing xdna->dev_handle/hdl->fw_version (otherwise a NULL dev_handle would crash before the check).
2955651 to
67a86f6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
67a86f6 to
6f4594d
Compare
|
Hi @maxzhen, I’ve removed the structure completely and am now using the existing amdxdna_drm_query_firmware_version, since the date and hash are no longer required in the examine report (we are already printing these values in the dmesg log). Could you please review and let me know if any further changes are needed? |
0695189
0695189 to
96d726f
Compare
Update cert version to print hot fix and build numbers. Temporarily remove cert firmware version query. Signed-off-by: Takasi, Manoj <Manoj.Takasi@amd.com> Signed-off-by: Nishad Saraf <nishads@amd.com>
96d726f to
f48590b
Compare
Update cert firmware version to print hot fix and build numbers.
XRT
Version : 2.23.0
Branch : HEAD
Hash : f8e923a4168dd552e02810de81288c34b416a0b4
Hash Date : Tue, 3 Mar 2026 10:59:04 -0800
amdxdna Version : 2.23.0_20260310, d410d8e3690a1362baff460d638c398f7ade4158
zocl Version : 2.21.0, f8e923a4168dd552e02810de81288c34b416a0b4
UC Firmware Version : 1.0.0.28, c8fbd0929042060d0fa4cffeff4dbc9577c7d5fe
UC Build Date : 2026-03-10