On 2/23/26 7:49 PM, Alejandro Jimenez wrote:
Hi David
On 2/23/26 8:06 AM, David Hoppenbrouwers wrote:
On 2/23/26 8:32 AM, Igor Mammedov wrote:
On Fri, 20 Feb 2026 19:14:37 +0100
David Hoppenbrouwers <[email protected]> wrote:
The command ID is in cmd[0], not cmd[1].
pls add a reference to the spec,
and if any what impact it does have on a guest/how it manifests.
Figure 43 "Generic Command Buffer Entry Format" in
https://docs.amd.com/v/u/en-US/48882_3.10_PUB
Unfortunately, the documentation links tend to be broken very often, so it
is not a good idea to include them in commit messages. That is why I would
opt for the longer, more verbose (but hopefully easier to find after a
quick search) choice of listing the name of the spec with the revision,
chapter and relevant keywords. See my proposed commit message at the end.
But the switch() above also uses cmd[0]:
switch (extract64(cmd[0], 60, 4)) {
case AMDVI_CMD_COMPLETION_WAIT:
amdvi_completion_wait(s, cmd);
break;
It has no effect on the guest, only on the -trace option. It may be
confusing to developers if the actual command ID doesn't match what is
shown by -trace.
I think it could have an effect on (some) guest too, although this is
unlikely given the number of things that need to go wrong, but perhaps a
test flow in a guest driver like:
- Guest triggers an IOMMU command with an invalid opcode.
- Guest reads the event log (incorrectly filled by the vIOMMU) and reports
an error on mismatched opcode.
Using cmd[1] seems clearly a typo from the original code. To honor Igor's
request to add details, I'd propose we change the commit message to:
amd_iommu: Fix opcode reported in invalid command handling
According to the AMD I/O Virtualization Technology (IOMMU) Specification
(Rev 3.10), Section 2.4 Commands, the Generic Command Buffer Entry Format
encodes the opcode in bits [63:60] of the command buffer.
When handling illegal opcodes, the traces for unhandled commands and event
log info extract the opcode from an incorrect offset in the command buffer.
Fix this issue to avoid potential confusion with mismatched opcodes in
traces and unlikely errors in guest event processing.
Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: David Hoppenbrouwers <[email protected]>
Reviewed-by: Sairaj Kodilkar <[email protected]>
If that works for everyone, I can add apply it; no need to send a new revision.
Thank you,
Alejandro
David
It is fine with me.
