All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sairaj Kodilkar <sarunkod@amd.com>
To: <qemu-devel@nongnu.org>
Cc: <mst@redhat.com>, <marcel.apfelbaum@gmail.com>,
	<pbonzini@redhat.com>, <eduardo@habkost.net>,
	<richard.henderson@linaro.org>, <alejandro.j.jimenez@oracle.com>,
	Sairaj Kodilkar <sarunkod@amd.com>,
	"Vasant Hegde" <vasant.hegde@amd.com>
Subject: [PATCH 5/7] hw/i386/amd_iommu: Fix event log generation
Date: Wed, 16 Jul 2025 13:01:43 +0530	[thread overview]
Message-ID: <20250716073145.915-6-sarunkod@amd.com> (raw)
In-Reply-To: <20250716073145.915-1-sarunkod@amd.com>

Current event logging code is broken, because of following issues

1. The code uses '|' instead of '&' to test the bit field, which causes
   vIOMMU to generate overflow interrupt for every log entry.
2. Code does not update the eventlog tail MMIO register after adding an
   entry to the buffer, because of which guest cannot process new
   entries (as head == tail means buffer is empty).
3. Compares eventlog tail (which is byte offset in the buffer) to
   eventlog length (which is number of maximum entries in the buffer).
   This causes vIOMMU to generate only fix number of event logs, after
   which it keeps on generating overflow interrupts, without
   actually resetting the log buffer.
4. Updates ComWaitInt instead of EventLogInt bitfield in Status
   register. Guest checks this field to see if there are new event log
   entries in the buffer.

Fix above issues, so that guest can process event log entries.

Fixes: d29a09ca68428 ("hw/i386: Introduce AMD IOMMU")
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
---
 hw/i386/amd_iommu.c | 20 ++++++++++++++++----
 hw/i386/amd_iommu.h |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index e0f4220b8f25..a34062153194 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -172,7 +172,7 @@ static void amdvi_writeq(AMDVIState *s, hwaddr addr, uint64_t val)
 /* OR a 64-bit register with a 64-bit value */
 static bool amdvi_test_mask(AMDVIState *s, hwaddr addr, uint64_t val)
 {
-    return amdvi_readq(s, addr) | val;
+    return amdvi_readq(s, addr) & val;
 }
 
 /* OR a 64-bit register with a 64-bit value storing result in the register */
@@ -201,16 +201,26 @@ static void amdvi_generate_msi_interrupt(AMDVIState *s)
     }
 }
 
+static uint32_t get_next_eventlog_entry(AMDVIState *s)
+{
+    uint32_t evtlog_size = s->evtlog_len * AMDVI_EVENT_LEN;
+    return (s->evtlog_tail + AMDVI_EVENT_LEN) % evtlog_size;
+}
+
 static void amdvi_log_event(AMDVIState *s, uint64_t *evt)
 {
+    uint32_t evtlog_tail_next;
+
     /* event logging not enabled */
     if (!s->evtlog_enabled || amdvi_test_mask(s, AMDVI_MMIO_STATUS,
         AMDVI_MMIO_STATUS_EVT_OVF)) {
         return;
     }
 
+    evtlog_tail_next = get_next_eventlog_entry(s);
+
     /* event log buffer full */
-    if (s->evtlog_tail >= s->evtlog_len) {
+    if (evtlog_tail_next == s->evtlog_head) {
         amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_EVT_OVF);
         /* generate interrupt */
         amdvi_generate_msi_interrupt(s);
@@ -222,8 +232,10 @@ static void amdvi_log_event(AMDVIState *s, uint64_t *evt)
         trace_amdvi_evntlog_fail(s->evtlog, s->evtlog_tail);
     }
 
-    s->evtlog_tail += AMDVI_EVENT_LEN;
-    amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
+    s->evtlog_tail = evtlog_tail_next;
+    amdvi_writeq(s, AMDVI_MMIO_EVENT_TAIL, s->evtlog_tail);
+
+    amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_EVENT_INT);
     amdvi_generate_msi_interrupt(s);
 }
 
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 62641b779ca3..3dd4e7e3e8b8 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -111,6 +111,7 @@
 #define AMDVI_MMIO_STATUS_CMDBUF_RUN  (1 << 4)
 #define AMDVI_MMIO_STATUS_EVT_RUN     (1 << 3)
 #define AMDVI_MMIO_STATUS_COMP_INT    (1 << 2)
+#define AMDVI_MMIO_STATUS_EVENT_INT   (1 << 1)
 #define AMDVI_MMIO_STATUS_EVT_OVF     (1 << 0)
 
 #define AMDVI_CMDBUF_ID_BYTE              0x07
-- 
2.34.1



  parent reply	other threads:[~2025-07-16  7:35 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-16  7:31 [PATCH 0/7] hw/i386/amd_iommu: Cleanups and fixes Sairaj Kodilkar
2025-07-16  7:31 ` [PATCH 1/7] hw/i386/amd_iommu: Fix MMIO register write tracing Sairaj Kodilkar
2025-07-16 12:31   ` Philippe Mathieu-Daudé
2025-07-16  7:31 ` [PATCH 2/7] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field Sairaj Kodilkar
2025-07-16 12:35   ` Philippe Mathieu-Daudé
2025-07-16  7:31 ` [PATCH 3/7] hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end Sairaj Kodilkar
2025-07-16  7:31 ` [PATCH 4/7] hw/i386/amd_iommu: Support MMIO writes to the status register Sairaj Kodilkar
2025-07-16 14:27   ` Ethan MILON
2025-07-17  6:41     ` Sairaj Kodilkar
2025-07-16  7:31 ` Sairaj Kodilkar [this message]
2025-07-16 14:50   ` [PATCH 5/7] hw/i386/amd_iommu: Fix event log generation Ethan MILON
2025-07-21 10:44     ` Sairaj Kodilkar
2025-07-16  7:31 ` [PATCH 6/7] hw/i386/amd_iommu: Fix handling device on buses != 0 Sairaj Kodilkar
2025-07-16 15:18   ` Ethan MILON
2025-07-17  6:47     ` Sairaj Kodilkar
2025-07-16  7:31 ` [PATCH 7/7] hw/i386/amd_iommu: Support 64 bit address for IOTLB lookup Sairaj Kodilkar
2025-07-16 12:37 ` [PATCH 0/7] hw/i386/amd_iommu: Cleanups and fixes Philippe Mathieu-Daudé
2025-07-16 12:56   ` Sairaj Kodilkar
2025-07-16 13:29     ` Michael S. Tsirkin
2025-07-17  5:47       ` Sairaj Kodilkar
2025-07-17  6:07         ` Michael S. Tsirkin
2025-07-17 13:48           ` Alejandro Jimenez
2025-07-18 13:28             ` Vasant Hegde
2025-07-18 14:30               ` Alejandro Jimenez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250716073145.915-6-sarunkod@amd.com \
    --to=sarunkod@amd.com \
    --cc=alejandro.j.jimenez@oracle.com \
    --cc=eduardo@habkost.net \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=vasant.hegde@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.