* [PATCH 1/1] hw/i386/amd_iommu: Fix IOMMU event log encoding errors
@ 2022-04-21 16:01 Wei Huang
2022-04-21 16:08 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: Wei Huang @ 2022-04-21 16:01 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, pbonzini, richard.henderson, eduardo, mst,
marcel.apfelbaum, jasowang, peterx, Suravee.Suthikulpanit
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);
}
/* log an error encountered during a page walk
*
@@ -218,7 +218,7 @@ static void amdvi_encode_event(uint64_t *evt, uint16_t devid, uint64_t addr,
static void amdvi_page_fault(AMDVIState *s, uint16_t devid,
hwaddr addr, uint16_t info)
{
- uint64_t evt[4];
+ uint64_t evt[2] = { 0 };
info |= AMDVI_EVENT_IOPF_I | AMDVI_EVENT_IOPF;
amdvi_encode_event(evt, devid, addr, info);
@@ -234,7 +234,7 @@ static void amdvi_page_fault(AMDVIState *s, uint16_t devid,
static void amdvi_log_devtab_error(AMDVIState *s, uint16_t devid,
hwaddr devtab, uint16_t info)
{
- uint64_t evt[4];
+ uint64_t evt[2] = { 0 };
info |= AMDVI_EVENT_DEV_TAB_HW_ERROR;
@@ -248,7 +248,8 @@ static void amdvi_log_devtab_error(AMDVIState *s, uint16_t devid,
*/
static void amdvi_log_command_error(AMDVIState *s, hwaddr addr)
{
- uint64_t evt[4], info = AMDVI_EVENT_COMMAND_HW_ERROR;
+ uint64_t evt[2] = { 0 };
+ uint16_t info = AMDVI_EVENT_COMMAND_HW_ERROR;
amdvi_encode_event(evt, 0, addr, info);
amdvi_log_event(s, evt);
@@ -261,7 +262,7 @@ static void amdvi_log_command_error(AMDVIState *s, hwaddr addr)
static void amdvi_log_illegalcom_error(AMDVIState *s, uint16_t info,
hwaddr addr)
{
- uint64_t evt[4];
+ uint64_t evt[2] = { 0 };
info |= AMDVI_EVENT_ILLEGAL_COMMAND_ERROR;
amdvi_encode_event(evt, 0, addr, info);
@@ -276,7 +277,7 @@ static void amdvi_log_illegalcom_error(AMDVIState *s, uint16_t info,
static void amdvi_log_illegaldevtab_error(AMDVIState *s, uint16_t devid,
hwaddr addr, uint16_t info)
{
- uint64_t evt[4];
+ uint64_t evt[2] = { 0 };
info |= AMDVI_EVENT_ILLEGAL_DEVTAB_ENTRY;
amdvi_encode_event(evt, devid, addr, info);
@@ -288,7 +289,7 @@ static void amdvi_log_illegaldevtab_error(AMDVIState *s, uint16_t devid,
static void amdvi_log_pagetab_error(AMDVIState *s, uint16_t devid,
hwaddr addr, uint16_t info)
{
- uint64_t evt[4];
+ uint64_t evt[2] = { 0 };
info |= AMDVI_EVENT_PAGE_TAB_HW_ERROR;
amdvi_encode_event(evt, devid, addr, info);
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] hw/i386/amd_iommu: Fix IOMMU event log encoding errors
2022-04-21 16:01 [PATCH 1/1] hw/i386/amd_iommu: Fix IOMMU event log encoding errors Wei Huang
@ 2022-04-21 16:08 ` Peter Maydell
2022-04-21 16:19 ` Wei Huang
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2022-04-21 16:08 UTC (permalink / raw)
To: Wei Huang
Cc: eduardo, Suravee.Suthikulpanit, mst, jasowang, richard.henderson,
qemu-devel, peterx, pbonzini
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.
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?
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] hw/i386/amd_iommu: Fix IOMMU event log encoding errors
2022-04-21 16:08 ` Peter Maydell
@ 2022-04-21 16:19 ` Wei Huang
0 siblings, 0 replies; 3+ messages in thread
From: Wei Huang @ 2022-04-21 16:19 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, pbonzini, richard.henderson, eduardo, mst,
marcel.apfelbaum, jasowang, peterx, Suravee.Suthikulpanit
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-21 17:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 16:01 [PATCH 1/1] hw/i386/amd_iommu: Fix IOMMU event log encoding errors Wei Huang
2022-04-21 16:08 ` Peter Maydell
2022-04-21 16:19 ` Wei Huang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.