On Mon, 23 Feb 2026 13:49:45 -0500 Alejandro Jimenez <[email protected]> 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]> with this Acked-by: Igor Mammedov <[email protected]> > > If that works for everyone, I can add apply it; no need to send a new > revision. > > Thank you, > Alejandro > > > David >
