On 2024/7/2 23:29, CLEMENT MATHIEU--DRIF wrote:

On 02/07/2024 15:33, Yi Liu wrote:
Caution: External email. Do not open attachments or click links,
unless this email comes from a known sender and you know the content
is safe.


On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote:
From: Clément Mathieu--Drif <clement.mathieu--d...@eviden.com>

Signed-off-by: Clément Mathieu--Drif <clement.mathieu--d...@eviden.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
   hw/i386/intel_iommu.c | 5 +++++
   1 file changed, 5 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 98996ededc..71cebe2fd3 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3500,6 +3500,11 @@ static bool
vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
       } else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
           /* Interrupt flag */
           vtd_generate_completion_event(s);
+    } else if (inv_desc->lo & VTD_INV_DESC_WAIT_FN) {
+        /*
+         * SW = 0, IF = 0, FN = 1
+         * Nothing to do as we process the events sequentially
+         */

This code looks a bit weird. SW field does not co-exist with IF. But
either
SW or IF can co-exist with FN flag. Is it? Have you already seen a wait
descriptor that only has FN flag set but no SW nor IF flag?
Yes, my test suite triggers that condition

I see. Spec indeed has such usage. Please add a comment for it.
Since it does not need a response, so QEMU can just bypass it. Also
please adjust the subject a bit. It's misleading. Perhaps

"intel_iommu: Bypass barrier wait descriptor"

Spec CH 7.10
a. Submit Invalidation Wait Descriptor (inv_wait_dsc) with Fence flag (FN=1) Set to Invalidation Queue. This ensures that all requests submitted to the Invalidation Queue ahead of this wait descriptor are processed and completed by remapping hardware before processing requests after the Invalidation Wait Descriptor. It is not required to specify SW flag (or IF flag) in this descriptor or for software to wait on its completion, as its function is to only act as a barrier.


       } else {
           error_report_once("%s: invalid wait desc: hi=%"PRIx64",
lo=%"PRIx64
                             " (unknown type)", __func__, inv_desc->hi,

--
Regards,
Yi Liu

--
Regards,
Yi Liu

Reply via email to