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

2022-04-24 Thread Jason Wang 
On Fri, Apr 22, 2022 at 1:52 PM Wei Huang  wrote:
>
> Coverity issues several UNINIT warnings against amd_iommu.c [1]. This
> patch fixes them by clearing evt before encoding. On top of it, this
> patch changes the event log size to 16 bytes per IOMMU specification,
> and fixes the event log entry format in amdvi_encode_event().
>
> [1] CID 1487116/1487200/1487190/1487232/1487115/1487258
>
> Reported-by: Peter Maydell 
> Signed-off-by: Wei Huang 
> ---

Acked-by: Jason Wang 

>  hw/i386/amd_iommu.c | 24 ++--
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index ea8eaeb330b6..725f69095b9e 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -201,15 +201,18 @@ static void amdvi_setevent_bits(uint64_t *buffer, 
> uint64_t value, int start,
>  /*
>   * AMDVi event structure
>   *0:15   -> DeviceID
> - *55:63  -> event type + miscellaneous info
> - *63:127 -> related address
> + *48:63  -> event type + miscellaneous info
> + *64:127 -> related address
>   */
>  static void amdvi_encode_event(uint64_t *evt, uint16_t devid, uint64_t addr,
> uint16_t info)
>  {
> +evt[0] = 0;
> +evt[1] = 0;
> +
>  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 +221,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];
>
>  info |= AMDVI_EVENT_IOPF_I | AMDVI_EVENT_IOPF;
>  amdvi_encode_event(evt, devid, addr, info);
> @@ -234,7 +237,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];
>
>  info |= AMDVI_EVENT_DEV_TAB_HW_ERROR;
>
> @@ -248,7 +251,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];
> +uint16_t info = AMDVI_EVENT_COMMAND_HW_ERROR;
>
>  amdvi_encode_event(evt, 0, addr, info);
>  amdvi_log_event(s, evt);
> @@ -261,7 +265,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];
>
>  info |= AMDVI_EVENT_ILLEGAL_COMMAND_ERROR;
>  amdvi_encode_event(evt, 0, addr, info);
> @@ -276,7 +280,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];
>
>  info |= AMDVI_EVENT_ILLEGAL_DEVTAB_ENTRY;
>  amdvi_encode_event(evt, devid, addr, info);
> @@ -288,7 +292,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];
>
>  info |= AMDVI_EVENT_PAGE_TAB_HW_ERROR;
>  amdvi_encode_event(evt, devid, addr, info);
> --
> 2.35.1
>

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

2022-04-22 Thread Peter Maydell 
On Fri, 22 Apr 2022 at 06:51, Wei Huang  wrote:
>
> Coverity issues several UNINIT warnings against amd_iommu.c [1]. This
> patch fixes them by clearing evt before encoding. On top of it, this
> patch changes the event log size to 16 bytes per IOMMU specification,
> and fixes the event log entry format in amdvi_encode_event().
>
> [1] CID 1487116/1487200/1487190/1487232/1487115/1487258
>
> Reported-by: Peter Maydell 
> Signed-off-by: Wei Huang 
> ---
Reviewed-by: Peter Maydell 

thanks
-- PMM

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

2022-04-21 Thread Wei Huang 
Coverity issues several UNINIT warnings against amd_iommu.c [1]. This
patch fixes them by clearing evt before encoding. On top of it, this
patch changes the event log size to 16 bytes per IOMMU specification,
and fixes the event log entry format in amdvi_encode_event().

[1] CID 1487116/1487200/1487190/1487232/1487115/1487258

Reported-by: Peter Maydell 
Signed-off-by: Wei Huang 
---
 hw/i386/amd_iommu.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index ea8eaeb330b6..725f69095b9e 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -201,15 +201,18 @@ static void amdvi_setevent_bits(uint64_t *buffer, 
uint64_t value, int start,
 /*
  * AMDVi event structure
  *0:15   -> DeviceID
- *55:63  -> event type + miscellaneous info
- *63:127 -> related address
+ *48:63  -> event type + miscellaneous info
+ *64:127 -> related address
  */
 static void amdvi_encode_event(uint64_t *evt, uint16_t devid, uint64_t addr,
uint16_t info)
 {
+evt[0] = 0;
+evt[1] = 0;
+
 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 +221,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];
 
 info |= AMDVI_EVENT_IOPF_I | AMDVI_EVENT_IOPF;
 amdvi_encode_event(evt, devid, addr, info);
@@ -234,7 +237,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];
 
 info |= AMDVI_EVENT_DEV_TAB_HW_ERROR;
 
@@ -248,7 +251,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];
+uint16_t info = AMDVI_EVENT_COMMAND_HW_ERROR;
 
 amdvi_encode_event(evt, 0, addr, info);
 amdvi_log_event(s, evt);
@@ -261,7 +265,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];
 
 info |= AMDVI_EVENT_ILLEGAL_COMMAND_ERROR;
 amdvi_encode_event(evt, 0, addr, info);
@@ -276,7 +280,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];
 
 info |= AMDVI_EVENT_ILLEGAL_DEVTAB_ENTRY;
 amdvi_encode_event(evt, devid, addr, info);
@@ -288,7 +292,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];
 
 info |= AMDVI_EVENT_PAGE_TAB_HW_ERROR;
 amdvi_encode_event(evt, devid, addr, info);
-- 
2.35.1