Re: who's maintaining amd_iommu.c these days?

[Peter Xu]

On Thu, Mar 31, 2022 at 05:01:52PM +0100, Peter Maydell wrote:
> Coverity points out some problems with hw/i386/amd_iommu.c's event
> logging code -- specifically, CID 1487115 1487116 1487190 1487200
> 1487232 1487258 are all the same basic problem, which is that various
> functions declare a local "uint64_t evt[4]", populate only some
> bits of it and then write it to guest memory, so we end up using
> uninitialized host data and leaking it to the guest. I was going to
> write a fix for this, but in looking at the code I noticed that
> it has more extensive problems:
> 
> (1) these functions allocate an array of 4 64-bit values,
> but we only copy 2 to the guest, because AMDVI_EVENT_LEN is 16.
> Looking at the spec, I think that the length is right and it's
> really 4 32-bit values (or 2 64-bit values, if you like).
> 
> (2) There are host-endianness bugs, because we assemble the
> event as a set of host-endianness values but then write them
> to guest memory as a bag-of-bytes with dma_memory_write()
> 
> (3) amdvi_encode_event() is throwing away most of its
> "addr" argument, because it calls
>   amdvi_setevent_bits(evt, addr, 63, 64) apparently intending
> that to write 64 bits starting at 63 bits into the packet, but
> the amdvi_setevent_bits() function only ever updates one
> uint64_t in the array, so it will in fact write bit 63 and
> nothing else.
> 
> (4) The claimed bit layout of the event structure doesn't
> match up with the one in the spec document I found. This
> could be because I found a document for some other bit
> of hardware, of course.
> 
> Anyway, adding all these up, the event logging probably
> needs a bit of a restructuring, and that should ideally be
> done by somebody who (a) knows the hardware we're emulating
> here and (b) is in a position to test things. Any volunteers?

Copying some AMD developers (from where I saw the last patches from)...

-- 
Peter Xu