Hi Paolo, A couple of comments/questions..
I'd suggest replacing the "fix infinite loop" commit subject with something a bit more informative e.g. "restrict command buffer head/tail ranges to ring size" On 5/11/26 7:39 AM, Paolo Bonzini wrote: > The AMD IOMMU command buffer is a ring buffer of cmdbuf_len (a power > of two) entries. Each entries is 16 bytes and the head pointer cycles > through the set: > Not sure if the ":" above meant the intention was to give an example here with a series of offsets e.g.: [0, 16, 32, ..., (cmdbuf_len - 1) * AMDVI_COMMAND_SIZE] or just end the sentence there. I can fix it either way. > The tail pointer is written by the guest through the COMMAND_TAIL MMIO > register (offset 0x2008); the while loop in amdvi_cmdbuf_run() only > terminates when head == tail. If tail is set to a value higher than > cmdbuf_len * 16, head will cycle through all the elements of the ring > buffer indefinitely, without ever matching tail. Fix this by further > masking tail (and head, for consistency) against the size of the > ring buffer. > > Reported-by: Yunhe Wang <[email protected]> Was this reported in a public thread? I have not been able to find an earlier discussion. I will send a follow up change patching an issue with the head pointer handling in amdvi_cmdbuf_run(), so I'd like to check if I missed any other similar issues. Thank you, Alejandro > Cc: [email protected] > Signed-off-by: Paolo Bonzini <[email protected]> > --- > hw/i386/amd_iommu.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c > index 789e09d6f2b..197e452e3c3 100644 > --- a/hw/i386/amd_iommu.c > +++ b/hw/i386/amd_iommu.c > @@ -1578,7 +1578,8 @@ static inline void amdvi_handle_devtab_write(AMDVIState > *s) > static inline void amdvi_handle_cmdhead_write(AMDVIState *s) > { > s->cmdbuf_head = amdvi_readq(s, AMDVI_MMIO_COMMAND_HEAD) > - & AMDVI_MMIO_CMDBUF_HEAD_MASK; > + & AMDVI_MMIO_CMDBUF_HEAD_MASK > + & (s->cmdbuf_len * AMDVI_COMMAND_SIZE - 1); > amdvi_cmdbuf_run(s); > } > > @@ -1594,7 +1595,8 @@ static inline void > amdvi_handle_cmdbase_write(AMDVIState *s) > static inline void amdvi_handle_cmdtail_write(AMDVIState *s) > { > s->cmdbuf_tail = amdvi_readq(s, AMDVI_MMIO_COMMAND_TAIL) > - & AMDVI_MMIO_CMDBUF_TAIL_MASK; > + & AMDVI_MMIO_CMDBUF_TAIL_MASK > + & (s->cmdbuf_len * AMDVI_COMMAND_SIZE - 1); > amdvi_cmdbuf_run(s); > } >
