Re: [PATCH 1/1] hw/i386/amd_iommu: Fix IOMMU event log encoding errors

[Wei Huang <wei.huang2@amd.com>]

On 4/21/22 11:08, Peter Maydell wrote:
> On Thu, 21 Apr 2022 at 17:01, Wei Huang <wei.huang2@amd.com> wrote:
>>
>> Coverity issues several UNINIT warnings against AMD IOMMU device [1]. This
>> patch fixes them by initializing the variables. On top of it, this patch
>> changes the event log size to 16 bytes per IOMMU specification. Also the
>> event encoding function incorrectly defines the format of event log entry,
>> which is also fixed.
>>
>> [1] CID 1487116/1487200/1487190/1487232/1487115/1487258
>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Wei Huang <wei.huang2@amd.com>
>> ---
>>   hw/i386/amd_iommu.c | 17 +++++++++--------
>>   1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index ea8eaeb330b6..0f7f8929a687 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -208,8 +208,8 @@ static void amdvi_encode_event(uint64_t *evt, uint16_t devid, uint64_t addr,
>>                                  uint16_t info)
>>   {
>>       amdvi_setevent_bits(evt, devid, 0, 16);
>> -    amdvi_setevent_bits(evt, info, 55, 8);
>> -    amdvi_setevent_bits(evt, addr, 63, 64);
>> +    amdvi_setevent_bits(evt, info, 48, 16);
>> +    amdvi_setevent_bits(evt, addr, 64, 64);
> 
> There's a comment just above this function which also needs updating.

Will do.

> 
> Would it be better to have this function start with
>    evt[0] = 0;
>    evt[1] = 0;
> 
> rather than requiring every caller to zero-initialize the buffer?

Assuming that Coverity is smart enough to poke one function further for 
checking UNINIT, I will fix it in the next spin. I will send another rev 
later today.

> 
> thanks
> -- PMM