Skip to content

accel/amdxdna: Add iommu_bypass debugfs for aie4 only#1297

Open
xdavidz wants to merge 1 commit into
amd:mainfrom
xdavidz:aie4_fw
Open

accel/amdxdna: Add iommu_bypass debugfs for aie4 only#1297
xdavidz wants to merge 1 commit into
amd:mainfrom
xdavidz:aie4_fw

Conversation

@xdavidz
Copy link
Copy Markdown
Contributor

@xdavidz xdavidz commented May 8, 2026

Add debug only debugfs for aie4.

Add debug only debugfs for aie4.

Signed-off-by: David Zhang <yidong.zhang@amd.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an AIE4-specific debugfs hook to the amdxdna driver so device-specific debugfs entries can be registered (intended for an iommu_bypass debug control on AIE4).

Changes:

  • Extend struct amdxdna_dev_ops with a per-device debugfs() callback and invoke it from the common debugfs init path.
  • Add AIE4 debugfs support (aie4_debugfs.c) and wire it into AIE4 ops.
  • Extend AIE4 mailbox protocol definitions with an ECHO opcode and request/response structs.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
drivers/accel/amdxdna/Kbuild Adds aie4_debugfs.o to the DEBUG_FS build.
drivers/accel/amdxdna/amdxdna_pci_drv.h Adds a debugfs callback to device ops.
drivers/accel/amdxdna/amdxdna_pci_drv.c No functional change (whitespace only in shown hunk).
drivers/accel/amdxdna/amdxdna_debugfs.c Calls the device-specific debugfs hook if present.
drivers/accel/amdxdna/aie4_pci.h Declares AIE4 debugfs init entrypoint.
drivers/accel/amdxdna/aie4_pci.c Wires .debugfs = aie4_debugfs_init into AIE4 ops.
drivers/accel/amdxdna/aie4_msg_priv.h Adds AIE4 echo opcode and message structs.
drivers/accel/amdxdna/aie4_debugfs.c Introduces AIE4 debugfs file iommu_bypass and echo-based write path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +83 to +84
/* aie4_debugfs.c */
void aie4_debugfs_init(struct amdxdna_dev *xdna);
Comment on lines +86 to +87
if (ret)
XDNA_ERR(xdna, "echo failed: %d", ret);
XDNA_INFO(xdna, "echo finished, response correct value.");
else
XDNA_WARN(xdna, "echo finished, expect: 0x%x,0x%x, got: 0x%x,0x%x",
req.val1, req.val1, resp.val1, resp.val2);
}

/*
* Input/output format: <carveout_size>@<carveout_address>
Copy link
Copy Markdown
Collaborator

@maxzhen maxzhen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the bypass mode value differing from system to system? If not, you don't really need a debugfs node for this. You can have a global macro to define the mode and make compile time decision. This is much simpler and easier to upstream, I think.

@xdavidz
Copy link
Copy Markdown
Contributor Author

xdavidz commented May 11, 2026

Is the bypass mode value differing from system to system? If not, you don't really need a debugfs node for this. You can have a global macro to define the mode and make compile time decision. This is much simpler and easier to upstream, I think.

I would keep this debug only for bringup. We do not need to upstream this.
@houlz0507 you are ok with this not being up-streamed right? This is bringup only feature, that the very first step is alawys the iommu_bypass mode, but it requires ipulib(jtag) to setup a register when mpnpufw is already loaded by the driver. We are just trying to make this a little user-friendly. Any end-user of repleased product would not need to use this feature.

@xdavidz
Copy link
Copy Markdown
Contributor Author

xdavidz commented May 11, 2026

Is the bypass mode value differing from system to system? If not, you don't really need a debugfs node for this. You can have a global macro to define the mode and make compile time decision. This is much simpler and easier to upstream, I think.

I would keep this debug only for bringup. We do not need to upstream this. @houlz0507 you are ok with this not being up-streamed right? This is bringup only feature, that the very first step is alawys the iommu_bypass mode, but it requires ipulib(jtag) to setup a register when mpnpufw is already loaded by the driver. We are just trying to make this a little user-friendly. Any end-user of repleased product would not need to use this feature.

after talking with Max. I think we need to do 2 things:

  1. covert debugfs to a ifdef macro to minimize the engineering complicity.
  2. using OOT driver for using cma for now. I will update this PR later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants