When processing commands, the internal cmdbuf_head state is correctly reset to 0 on wrap, but the MMIO CmdHeadPtr register was written before the wrap check, leaving the guest-visible register stuck at the buffer length instead of 0 after a full command buffer pass.
Move the register write after the wrap check so the MMIO value stays in sync with the internal state. The Linux kernel AMD IOMMU driver is not affected by this bug because it uses COMPLETION_WAIT with a memory store doorbell to detect command progress. Add a test to lock this down. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3338 Signed-off-by: Costas Argyris <[email protected]> --- MAINTAINERS | 1 + hw/i386/amd_iommu.c | 2 +- tests/qtest/amd-iommu-test.c | 71 ++++++++++++++++++++++++++++++++++++ tests/qtest/meson.build | 1 + 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tests/qtest/amd-iommu-test.c diff --git a/MAINTAINERS b/MAINTAINERS index 80d28e618d..750d415389 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4052,6 +4052,7 @@ M: Alejandro Jimenez <[email protected]> R: Sairaj Kodilkar <[email protected]> S: Supported F: hw/i386/amd_iommu* +F: tests/qtest/amd-iommu-test.c OpenSBI Firmware L: [email protected] diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 789e09d6f2..ce18cb453f 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -1475,12 +1475,12 @@ static void amdvi_cmdbuf_run(AMDVIState *s) trace_amdvi_command_exec(s->cmdbuf_head, s->cmdbuf_tail, s->cmdbuf); amdvi_cmdbuf_exec(s); s->cmdbuf_head += AMDVI_COMMAND_SIZE; - amdvi_writeq_raw(s, AMDVI_MMIO_COMMAND_HEAD, s->cmdbuf_head); /* wrap head pointer */ if (s->cmdbuf_head >= s->cmdbuf_len * AMDVI_COMMAND_SIZE) { s->cmdbuf_head = 0; } + amdvi_writeq_raw(s, AMDVI_MMIO_COMMAND_HEAD, s->cmdbuf_head); } } diff --git a/tests/qtest/amd-iommu-test.c b/tests/qtest/amd-iommu-test.c new file mode 100644 index 0000000000..0438ee9586 --- /dev/null +++ b/tests/qtest/amd-iommu-test.c @@ -0,0 +1,71 @@ +/* + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "libqtest.h" +#include "hw/i386/amd_iommu.h" + +#define CMDBUF_ADDR 0x200000 +#define CMDBUF_ENTRIES 256 +#define CMDBUF_LEN_FIELD 8 + +static inline uint64_t amdvi_reg_readq(QTestState *s, uint64_t offset) +{ + return qtest_readq(s, AMDVI_BASE_ADDR + offset); +} + +static inline void amdvi_reg_writeq(QTestState *s, uint64_t offset, + uint64_t val) +{ + qtest_writeq(s, AMDVI_BASE_ADDR + offset, val); +} + +static void test_cmdbuf_head_wrap(void) +{ + QTestState *s; + uint64_t head; + int i; + /* 16 bytes per command */ + struct { + uint64_t qw0; + uint64_t qw1; + } cmdbuf[CMDBUF_ENTRIES]; + + s = qtest_init("-M q35 -device amd-iommu"); + + /* write 256 COMPLETION_WAIT (no-op) commands to guest RAM */ + for (i = 0; i < CMDBUF_ENTRIES; i++) { + cmdbuf[i].qw0 = (uint64_t)AMDVI_CMD_COMPLETION_WAIT << 60; + cmdbuf[i].qw1 = 0; + } + qtest_memwrite(s, CMDBUF_ADDR, cmdbuf, sizeof(cmdbuf)); + + /* point the IOMMU at the command buffer and set its length */ + amdvi_reg_writeq(s, AMDVI_MMIO_COMMAND_BASE, + CMDBUF_ADDR | ((uint64_t)CMDBUF_LEN_FIELD << 56)); + + /* enable the IOMMU and its command buffer processor */ + amdvi_reg_writeq(s, AMDVI_MMIO_CONTROL, + AMDVI_MMIO_CONTROL_AMDVIEN | AMDVI_MMIO_CONTROL_CMDBUFLEN); + + /* advance tail to the last entry, consuming entries 0..254 */ + amdvi_reg_writeq(s, AMDVI_MMIO_COMMAND_TAIL, + (CMDBUF_ENTRIES - 1) * AMDVI_COMMAND_SIZE); + + /* wrap tail to 0, consuming entry 255 and completing the buffer */ + amdvi_reg_writeq(s, AMDVI_MMIO_COMMAND_TAIL, 0); + + /* after consuming all 256 entries the IOMMU must wrap CmdHeadPtr to 0 */ + head = amdvi_reg_readq(s, AMDVI_MMIO_COMMAND_HEAD); + g_assert((head & AMDVI_MMIO_CMDBUF_HEAD_MASK) == 0); + + qtest_quit(s); +} + +int main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + qtest_add_func("/q35/amd-iommu/cmdbuf-head-wrap", test_cmdbuf_head_wrap); + return g_test_run(); +} diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 728dde54b3..67eea5c71a 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -95,6 +95,7 @@ qtests_i386 = \ (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \ (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) + \ (config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) + \ + (config_all_devices.has_key('CONFIG_AMD_IOMMU') ? ['amd-iommu-test'] : []) + \ (config_all_devices.has_key('CONFIG_VTD') ? ['intel-iommu-test'] : []) + \ (config_all_devices.has_key('CONFIG_VTD') and config_all_devices.has_key('CONFIG_IOMMU_TESTDEV') ? ['iommu-intel-test'] : []) + \ -- 2.43.0
