[PATCH v1] hw/i386/amd_iommu: clean up broken event logging

[Daniil Tatianin]

- Don't create evt buffer in every function where we want to log,
  instead make amdvi_log_event construct the buffer in-place using the
  arguments it was given.

- Correctly place address & info in the event buffer. Previously both
  would get shifted to incorrect bit offsets leading to evt containing
  uninitialized event type and address.

- Reduce buffer size from 32 to 16 bytes, which is the amount of bytes
  actually needed for event storage.

- Remove amdvi_encode_event & amdvi_setevent_bits, as they are no longer
  needed.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 hw/i386/amd_iommu.c | 62 ++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 48 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index fd75cae024..44b995be91 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -164,8 +164,14 @@ static void amdvi_generate_msi_interrupt(AMDVIState *s)
     }
 }
 
-static void amdvi_log_event(AMDVIState *s, uint64_t *evt)
+static void amdvi_log_event(AMDVIState *s, uint16_t devid,
+                            hwaddr addr, uint16_t info)
 {
+    uint64_t evt[2] = {
+        devid | ((uint64_t)info << 48),
+        addr
+    };
+
     /* event logging not enabled */
     if (!s->evtlog_enabled || amdvi_test_mask(s, AMDVI_MMIO_STATUS,
         AMDVI_MMIO_STATUS_EVT_OVF)) {
@@ -190,27 +196,6 @@ static void amdvi_log_event(AMDVIState *s, uint64_t *evt)
     amdvi_generate_msi_interrupt(s);
 }
 
-static void amdvi_setevent_bits(uint64_t *buffer, uint64_t value, int start,
-                                int length)
-{
-    int index = start / 64, bitpos = start % 64;
-    uint64_t mask = MAKE_64BIT_MASK(start, length);
-    buffer[index] &= ~mask;
-    buffer[index] |= (value << bitpos) & mask;
-}
-/*
- * AMDVi event structure
- *    0:15   -> DeviceID
- *    55:63  -> event type + miscellaneous info
- *    63:127 -> related address
- */
-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);
-}
 /* log an error encountered during a page walk
  *
  * @addr: virtual address in translation request
@@ -218,11 +203,8 @@ 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];
-
     info |= AMDVI_EVENT_IOPF_I | AMDVI_EVENT_IOPF;
-    amdvi_encode_event(evt, devid, addr, info);
-    amdvi_log_event(s, evt);
+    amdvi_log_event(s, devid, addr, info);
     pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS,
             PCI_STATUS_SIG_TARGET_ABORT);
 }
@@ -232,14 +214,10 @@ static void amdvi_page_fault(AMDVIState *s, uint16_t 
devid,
  *  @info : error flags
  */
 static void amdvi_log_devtab_error(AMDVIState *s, uint16_t devid,
-                                   hwaddr devtab, uint16_t info)
+                                   hwaddr addr, uint16_t info)
 {
-    uint64_t evt[4];
-
     info |= AMDVI_EVENT_DEV_TAB_HW_ERROR;
-
-    amdvi_encode_event(evt, devid, devtab, info);
-    amdvi_log_event(s, evt);
+    amdvi_log_event(s, devid, addr, info);
     pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS,
             PCI_STATUS_SIG_TARGET_ABORT);
 }
@@ -248,10 +226,7 @@ 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;
-
-    amdvi_encode_event(evt, 0, addr, info);
-    amdvi_log_event(s, evt);
+    amdvi_log_event(s, 0, addr, AMDVI_EVENT_COMMAND_HW_ERROR);
     pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS,
             PCI_STATUS_SIG_TARGET_ABORT);
 }
@@ -261,11 +236,8 @@ 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];
-
     info |= AMDVI_EVENT_ILLEGAL_COMMAND_ERROR;
-    amdvi_encode_event(evt, 0, addr, info);
-    amdvi_log_event(s, evt);
+    amdvi_log_event(s, 0, addr, info);
 }
 /* log an error accessing device table
  *
@@ -276,11 +248,8 @@ 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];
-
     info |= AMDVI_EVENT_ILLEGAL_DEVTAB_ENTRY;
-    amdvi_encode_event(evt, devid, addr, info);
-    amdvi_log_event(s, evt);
+    amdvi_log_event(s, devid, addr, info);
 }
 /* log an error accessing a PTE entry
  * @addr : address that couldn't be accessed
@@ -288,11 +257,8 @@ 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];
-
     info |= AMDVI_EVENT_PAGE_TAB_HW_ERROR;
-    amdvi_encode_event(evt, devid, addr, info);
-    amdvi_log_event(s, evt);
+    amdvi_log_event(s, devid, addr, info);
     pci_word_test_and_set_mask(s->pci.dev.config + PCI_STATUS,
              PCI_STATUS_SIG_TARGET_ABORT);
 }
-- 
2.25.1