PRQ slot allocation must be thread safe to avoid posting multiple
commands in the same slot so we take a lock in vtd_pri_request_page.

This allows multiple devices to call the page request interface
of the same iommu instance.

Signed-off-by: Clement Mathieu--Drif <clement.mathieu--drif@bull.com>
---
 hw/i386/intel_iommu.c | 91 +++++++++++++++++++++++++++----------------
 1 file changed, 57 insertions(+), 34 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 9f21622a4f..350d2b7753 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3627,15 +3627,26 @@ static void vtd_handle_iectl_write(IntelIOMMUState *s)
 
 static void vtd_handle_prs_write(IntelIOMMUState *s)
 {
-    uint32_t prs = vtd_get_long_raw(s, DMAR_PRS_REG);
+    uint32_t prs;
+
+    vtd_iommu_lock(s);
+
+    prs = vtd_get_long_raw(s, DMAR_PRS_REG);
     if (!(prs & VTD_PR_STATUS_PPR) && !(prs & VTD_PR_STATUS_PRO)) {
         vtd_set_clear_mask_long(s, DMAR_PECTL_REG, VTD_PR_PECTL_IP, 0);
     }
+
+    vtd_iommu_unlock(s);
 }
 
 static void vtd_handle_pectl_write(IntelIOMMUState *s)
 {
-    uint32_t pectl = vtd_get_long_raw(s, DMAR_PECTL_REG);
+    uint32_t pectl;
+
+    vtd_iommu_lock(s);
+
+    pectl = vtd_get_long_raw(s, DMAR_PECTL_REG);
+
     if ((pectl & VTD_PR_PECTL_IP) && !(pectl & VTD_PR_PECTL_IM)) {
         /*
          * If IP field was 1 when software clears the IM field,
@@ -3644,6 +3655,8 @@ static void vtd_handle_pectl_write(IntelIOMMUState *s)
         vtd_set_clear_mask_long(s, DMAR_PECTL_REG, VTD_PR_PECTL_IP, 0);
         vtd_generate_interrupt(s, DMAR_PEADDR_REG, DMAR_PEDATA_REG);
     }
+
+    vtd_iommu_unlock(s);
 }
 
 static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size)
@@ -5378,19 +5391,18 @@ static int vtd_pri_request_page(PCIBus *bus, void *opaque, int devfn,
 {
     IntelIOMMUState *s = opaque;
     VTDAddressSpace *vtd_as;
+    uint64_t queue_addr_reg;
+    uint64_t queue_tail_offset_reg;
+    uint64_t new_queue_tail_offset;
+    uint64_t queue_head_offset_reg;
+    hwaddr queue_tail;
+    uint32_t old_pr_status;
+    uint16_t sid;
+    VTDPRDesc desc;
+    int ret = 0;
 
     vtd_as = vtd_find_add_as(s, bus, devfn, pasid);
-
-    uint64_t queue_addr_reg = vtd_get_quad(s, DMAR_PQA_REG);
-    uint64_t queue_tail_offset_reg = vtd_get_quad(s, DMAR_PQT_REG);
-    uint64_t new_queue_tail_offset = (
-                                (queue_tail_offset_reg + VTD_PQA_ENTRY_SIZE) %
-                                (vtd_prq_size(s) * VTD_PQA_ENTRY_SIZE));
-    uint64_t queue_head_offset_reg = vtd_get_quad(s, DMAR_PQH_REG);
-    hwaddr queue_tail = (queue_addr_reg & VTD_PQA_ADDR) + queue_tail_offset_reg;
-    uint32_t old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
-    uint16_t sid = PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn);
-    VTDPRDesc desc;
+    sid = PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn);
 
     if (!(s->ecap & VTD_ECAP_PRS)) {
         return -EPERM;
@@ -5414,52 +5426,63 @@ static int vtd_pri_request_page(PCIBus *bus, void *opaque, int devfn,
         return -EPERM;
     }
 
+    /* Prepare the descriptor */
+    desc.lo = VTD_PRD_TYPE | VTD_PRD_PP(true) | VTD_PRD_RID(sid) |
+              VTD_PRD_PASID(vtd_as->pasid) | VTD_PRD_PMR(priv_req);
+    desc.hi = VTD_PRD_RDR(is_read) | VTD_PRD_WRR(is_write) |
+              VTD_PRD_LPIG(lpig) | VTD_PRD_PRGI(prgi) | VTD_PRD_ADDR(addr);
+
+    desc.lo = cpu_to_le64(desc.lo);
+    desc.hi = cpu_to_le64(desc.hi);
+
+    if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) {
+        return -EINVAL;
+    }
+
+    vtd_iommu_lock(s);
+
+    queue_addr_reg = vtd_get_quad(s, DMAR_PQA_REG);
+    queue_tail_offset_reg = vtd_get_quad(s, DMAR_PQT_REG);
+    new_queue_tail_offset = ((queue_tail_offset_reg + VTD_PQA_ENTRY_SIZE) %
+                            (vtd_prq_size(s) * VTD_PQA_ENTRY_SIZE));
+    queue_head_offset_reg = vtd_get_quad(s, DMAR_PQH_REG);
+    queue_tail = (queue_addr_reg & VTD_PQA_ADDR) + queue_tail_offset_reg;
+    old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
+
     if (old_pr_status & VTD_PR_STATUS_PRO) {
         /*
          * No action is taken by hardware to report a fault
          * or generate an event
          */
-        return -ENOSPC;
+        ret = -ENOSPC;
+        goto out;
     }
 
     /* Check for overflow */
     if (new_queue_tail_offset == queue_head_offset_reg) {
         vtd_set_clear_mask_long(s, DMAR_PRS_REG, 0, VTD_PR_STATUS_PRO);
         vtd_generate_page_request_event(s, old_pr_status);
-        return -ENOSPC;
-    }
-
-    if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) {
-        return -EINVAL;
+        ret = -ENOSPC;
+        goto out;
     }
 
-    desc.lo = VTD_PRD_TYPE | VTD_PRD_PP(true) | VTD_PRD_RID(sid) |
-              VTD_PRD_PASID(vtd_as->pasid) | VTD_PRD_PMR(priv_req);
-    desc.hi = VTD_PRD_RDR(is_read) | VTD_PRD_WRR(is_write) |
-              VTD_PRD_LPIG(lpig) | VTD_PRD_PRGI(prgi) | VTD_PRD_ADDR(addr);
-
-    desc.lo = cpu_to_le64(desc.lo);
-    desc.hi = cpu_to_le64(desc.hi);
     if (dma_memory_write(&address_space_memory, queue_tail, &desc, sizeof(desc),
                          MEMTXATTRS_UNSPECIFIED)) {
         error_report_once("IO error, the PQ tail cannot be updated");
-        return -EIO;
+        ret = -EIO;
+        goto out;
     }
 
     /* increment the tail register and set the pending request bit */
     vtd_set_quad(s, DMAR_PQT_REG, new_queue_tail_offset);
-    /*
-     * read status again so that the kernel does not miss a request.
-     * in some cases, we can trigger an unecessary interrupt but this strategy
-     * drastically improves performance as we don't need to take a lock.
-     */
-    old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
     if (!(old_pr_status & VTD_PR_STATUS_PPR)) {
         vtd_set_clear_mask_long(s, DMAR_PRS_REG, 0, VTD_PR_STATUS_PPR);
         vtd_generate_page_request_event(s, old_pr_status);
     }
 
-    return 0;
+out:
+    vtd_iommu_unlock(s);
+    return ret;
 }
 
 static void vtd_init_iotlb_notifier(PCIBus *bus, void *opaque, int devfn,
-- 
2.54.0

>-----Original Message-----
>From: Clément MATHIEU--DRIF <clement.mathieu--drif@bull.com>
>Subject: [PATCH v1] intel_iommu: Support concurrent page fault handling with PRI
>
>PRQ slot allocation must be thread safe to avoid posting multiple
>commands in the same slot so we take a lock in vtd_pri_request_page.
>
>This allows multiple devices to call the page request interface
>of the same iommu instance.

Aren't these devices' emulations serialized by BQL?

>
>Signed-off-by: Clement Mathieu--Drif <clement.mathieu--drif@bull.com>
>---
> hw/i386/intel_iommu.c | 91 +++++++++++++++++++++++++++----------------
> 1 file changed, 57 insertions(+), 34 deletions(-)
>
>diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>index 9f21622a4f..350d2b7753 100644
>--- a/hw/i386/intel_iommu.c
>+++ b/hw/i386/intel_iommu.c
>@@ -3627,15 +3627,26 @@ static void vtd_handle_iectl_write(IntelIOMMUState
>*s)
>
> static void vtd_handle_prs_write(IntelIOMMUState *s)
> {
>-    uint32_t prs = vtd_get_long_raw(s, DMAR_PRS_REG);
>+    uint32_t prs;
>+
>+    vtd_iommu_lock(s);
>+
>+    prs = vtd_get_long_raw(s, DMAR_PRS_REG);
>     if (!(prs & VTD_PR_STATUS_PPR) && !(prs & VTD_PR_STATUS_PRO)) {
>         vtd_set_clear_mask_long(s, DMAR_PECTL_REG, VTD_PR_PECTL_IP, 0);
>     }
>+
>+    vtd_iommu_unlock(s);
> }
>
> static void vtd_handle_pectl_write(IntelIOMMUState *s)
> {
>-    uint32_t pectl = vtd_get_long_raw(s, DMAR_PECTL_REG);
>+    uint32_t pectl;
>+
>+    vtd_iommu_lock(s);
>+
>+    pectl = vtd_get_long_raw(s, DMAR_PECTL_REG);
>+
>     if ((pectl & VTD_PR_PECTL_IP) && !(pectl & VTD_PR_PECTL_IM)) {
>         /*
>          * If IP field was 1 when software clears the IM field,
>@@ -3644,6 +3655,8 @@ static void vtd_handle_pectl_write(IntelIOMMUState
>*s)
>         vtd_set_clear_mask_long(s, DMAR_PECTL_REG, VTD_PR_PECTL_IP, 0);
>         vtd_generate_interrupt(s, DMAR_PEADDR_REG, DMAR_PEDATA_REG);
>     }
>+
>+    vtd_iommu_unlock(s);
> }
>
> static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size)
>@@ -5378,19 +5391,18 @@ static int vtd_pri_request_page(PCIBus *bus, void
>*opaque, int devfn,
> {
>     IntelIOMMUState *s = opaque;
>     VTDAddressSpace *vtd_as;
>+    uint64_t queue_addr_reg;
>+    uint64_t queue_tail_offset_reg;
>+    uint64_t new_queue_tail_offset;
>+    uint64_t queue_head_offset_reg;
>+    hwaddr queue_tail;
>+    uint32_t old_pr_status;
>+    uint16_t sid;
>+    VTDPRDesc desc;
>+    int ret = 0;
>
>     vtd_as = vtd_find_add_as(s, bus, devfn, pasid);
>-
>-    uint64_t queue_addr_reg = vtd_get_quad(s, DMAR_PQA_REG);
>-    uint64_t queue_tail_offset_reg = vtd_get_quad(s, DMAR_PQT_REG);
>-    uint64_t new_queue_tail_offset = (
>-                                (queue_tail_offset_reg + VTD_PQA_ENTRY_SIZE) %
>-                                (vtd_prq_size(s) * VTD_PQA_ENTRY_SIZE));
>-    uint64_t queue_head_offset_reg = vtd_get_quad(s, DMAR_PQH_REG);
>-    hwaddr queue_tail = (queue_addr_reg & VTD_PQA_ADDR) +
>queue_tail_offset_reg;
>-    uint32_t old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
>-    uint16_t sid = PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn);
>-    VTDPRDesc desc;
>+    sid = PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn);
>
>     if (!(s->ecap & VTD_ECAP_PRS)) {
>         return -EPERM;
>@@ -5414,52 +5426,63 @@ static int vtd_pri_request_page(PCIBus *bus, void
>*opaque, int devfn,
>         return -EPERM;
>     }
>
>+    /* Prepare the descriptor */
>+    desc.lo = VTD_PRD_TYPE | VTD_PRD_PP(true) | VTD_PRD_RID(sid) |
>+              VTD_PRD_PASID(vtd_as->pasid) | VTD_PRD_PMR(priv_req);
>+    desc.hi = VTD_PRD_RDR(is_read) | VTD_PRD_WRR(is_write) |
>+              VTD_PRD_LPIG(lpig) | VTD_PRD_PRGI(prgi) | VTD_PRD_ADDR(addr);
>+
>+    desc.lo = cpu_to_le64(desc.lo);
>+    desc.hi = cpu_to_le64(desc.hi);
>+
>+    if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) {
>+        return -EINVAL;
>+    }
>+
>+    vtd_iommu_lock(s);
>+
>+    queue_addr_reg = vtd_get_quad(s, DMAR_PQA_REG);
>+    queue_tail_offset_reg = vtd_get_quad(s, DMAR_PQT_REG);
>+    new_queue_tail_offset = ((queue_tail_offset_reg + VTD_PQA_ENTRY_SIZE) %
>+                            (vtd_prq_size(s) * VTD_PQA_ENTRY_SIZE));
>+    queue_head_offset_reg = vtd_get_quad(s, DMAR_PQH_REG);
>+    queue_tail = (queue_addr_reg & VTD_PQA_ADDR) + queue_tail_offset_reg;
>+    old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
>+
>     if (old_pr_status & VTD_PR_STATUS_PRO) {
>         /*
>          * No action is taken by hardware to report a fault
>          * or generate an event
>          */
>-        return -ENOSPC;
>+        ret = -ENOSPC;
>+        goto out;
>     }
>
>     /* Check for overflow */
>     if (new_queue_tail_offset == queue_head_offset_reg) {
>         vtd_set_clear_mask_long(s, DMAR_PRS_REG, 0, VTD_PR_STATUS_PRO);
>         vtd_generate_page_request_event(s, old_pr_status);
>-        return -ENOSPC;
>-    }
>-
>-    if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) {
>-        return -EINVAL;
>+        ret = -ENOSPC;
>+        goto out;
>     }
>
>-    desc.lo = VTD_PRD_TYPE | VTD_PRD_PP(true) | VTD_PRD_RID(sid) |
>-              VTD_PRD_PASID(vtd_as->pasid) | VTD_PRD_PMR(priv_req);
>-    desc.hi = VTD_PRD_RDR(is_read) | VTD_PRD_WRR(is_write) |
>-              VTD_PRD_LPIG(lpig) | VTD_PRD_PRGI(prgi) | VTD_PRD_ADDR(addr);
>-
>-    desc.lo = cpu_to_le64(desc.lo);
>-    desc.hi = cpu_to_le64(desc.hi);
>     if (dma_memory_write(&address_space_memory, queue_tail, &desc,
>sizeof(desc),
>                          MEMTXATTRS_UNSPECIFIED)) {
>         error_report_once("IO error, the PQ tail cannot be updated");
>-        return -EIO;
>+        ret = -EIO;
>+        goto out;
>     }
>
>     /* increment the tail register and set the pending request bit */
>     vtd_set_quad(s, DMAR_PQT_REG, new_queue_tail_offset);
>-    /*
>-     * read status again so that the kernel does not miss a request.
>-     * in some cases, we can trigger an unecessary interrupt but this strategy
>-     * drastically improves performance as we don't need to take a lock.
>-     */
>-    old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
>     if (!(old_pr_status & VTD_PR_STATUS_PPR)) {
>         vtd_set_clear_mask_long(s, DMAR_PRS_REG, 0, VTD_PR_STATUS_PPR);
>         vtd_generate_page_request_event(s, old_pr_status);
>     }
>
>-    return 0;
>+out:
>+    vtd_iommu_unlock(s);
>+    return ret;
> }
>
> static void vtd_init_iotlb_notifier(PCIBus *bus, void *opaque, int devfn,
>--
>2.54.0

On Tue, 2026-09-08 at 07:01 +0000, Duan, Zhenzhong 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.
> 
> 
> 
> > -----Original Message-----
> > From: Clément MATHIEU--DRIF <[clement.mathieu--drif@bull.com](mailto:clement.mathieu--drif@bull.com)>
> > Subject: [PATCH v1] intel_iommu: Support concurrent page fault handling with PRI
> > 
> > PRQ slot allocation must be thread safe to avoid posting multiple
> > commands in the same slot so we take a lock in vtd_pri_request_page.
> > 
> > This allows multiple devices to call the page request interface
> > of the same iommu instance.
> 
> 
> Aren't these devices' emulations serialized by BQL?
> 

Hi,

Not really, emulated devices calling the PRI interface  
are discouraged from taking the BQL to avoid deadlocks.  
A device's DMA module is likely to have an internal lock.  
The following concurrent paths can lead to deadlocks:

- DMA lock -> ... internal logic -> BQL lock + PRI request
- PRI response -> MMIO with BQL in VT-d -> PRI notifier -> DMA lock

DMA lock and BQL taken in reverse order.

Thanks,  
cmd

> 
> 
> > Signed-off-by: Clement Mathieu--Drif <[clement.mathieu--drif@bull.com](mailto:clement.mathieu--drif@bull.com)>
> > ---
> > hw/i386/intel_iommu.c | 91 +++++++++++++++++++++++++++----------------
> > 1 file changed, 57 insertions(+), 34 deletions(-)
> > 
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 9f21622a4f..350d2b7753 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -3627,15 +3627,26 @@ static void vtd_handle_iectl_write(IntelIOMMUState
> > *s)
> > 
> > static void vtd_handle_prs_write(IntelIOMMUState *s)
> > {
> > -    uint32_t prs = vtd_get_long_raw(s, DMAR_PRS_REG);
> > +    uint32_t prs;
> > +
> > +    vtd_iommu_lock(s);
> > +
> > +    prs = vtd_get_long_raw(s, DMAR_PRS_REG);
> >     if (!(prs & VTD_PR_STATUS_PPR) && !(prs & VTD_PR_STATUS_PRO)) {
> >         vtd_set_clear_mask_long(s, DMAR_PECTL_REG, VTD_PR_PECTL_IP, 0);
> >     }
> > +
> > +    vtd_iommu_unlock(s);
> > }
> > 
> > static void vtd_handle_pectl_write(IntelIOMMUState *s)
> > {
> > -    uint32_t pectl = vtd_get_long_raw(s, DMAR_PECTL_REG);
> > +    uint32_t pectl;
> > +
> > +    vtd_iommu_lock(s);
> > +
> > +    pectl = vtd_get_long_raw(s, DMAR_PECTL_REG);
> > +
> >     if ((pectl & VTD_PR_PECTL_IP) && !(pectl & VTD_PR_PECTL_IM)) {
> >         /*
> >          * If IP field was 1 when software clears the IM field,
> > @@ -3644,6 +3655,8 @@ static void vtd_handle_pectl_write(IntelIOMMUState
> > *s)
> >         vtd_set_clear_mask_long(s, DMAR_PECTL_REG, VTD_PR_PECTL_IP, 0);
> >         vtd_generate_interrupt(s, DMAR_PEADDR_REG, DMAR_PEDATA_REG);
> >     }
> > +
> > +    vtd_iommu_unlock(s);
> > }
> > 
> > static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size)
> > @@ -5378,19 +5391,18 @@ static int vtd_pri_request_page(PCIBus *bus, void
> > *opaque, int devfn,
> > {
> >     IntelIOMMUState *s = opaque;
> >     VTDAddressSpace *vtd_as;
> > +    uint64_t queue_addr_reg;
> > +    uint64_t queue_tail_offset_reg;
> > +    uint64_t new_queue_tail_offset;
> > +    uint64_t queue_head_offset_reg;
> > +    hwaddr queue_tail;
> > +    uint32_t old_pr_status;
> > +    uint16_t sid;
> > +    VTDPRDesc desc;
> > +    int ret = 0;
> > 
> >     vtd_as = vtd_find_add_as(s, bus, devfn, pasid);
> > -
> > -    uint64_t queue_addr_reg = vtd_get_quad(s, DMAR_PQA_REG);
> > -    uint64_t queue_tail_offset_reg = vtd_get_quad(s, DMAR_PQT_REG);
> > -    uint64_t new_queue_tail_offset = (
> > -                                (queue_tail_offset_reg + VTD_PQA_ENTRY_SIZE) %
> > -                                (vtd_prq_size(s) * VTD_PQA_ENTRY_SIZE));
> > -    uint64_t queue_head_offset_reg = vtd_get_quad(s, DMAR_PQH_REG);
> > -    hwaddr queue_tail = (queue_addr_reg & VTD_PQA_ADDR) +
> > queue_tail_offset_reg;
> > -    uint32_t old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
> > -    uint16_t sid = PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn);
> > -    VTDPRDesc desc;
> > +    sid = PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn);
> > 
> >     if (!(s->ecap & VTD_ECAP_PRS)) {
> >         return -EPERM;
> > @@ -5414,52 +5426,63 @@ static int vtd_pri_request_page(PCIBus *bus, void
> > *opaque, int devfn,
> >         return -EPERM;
> >     }
> > 
> > +    /* Prepare the descriptor */
> > +    desc.lo = VTD_PRD_TYPE | VTD_PRD_PP(true) | VTD_PRD_RID(sid) |
> > +              VTD_PRD_PASID(vtd_as->pasid) | VTD_PRD_PMR(priv_req);
> > +    desc.hi = VTD_PRD_RDR(is_read) | VTD_PRD_WRR(is_write) |
> > +              VTD_PRD_LPIG(lpig) | VTD_PRD_PRGI(prgi) | VTD_PRD_ADDR(addr);
> > +
> > +    desc.lo = cpu_to_le64(desc.lo);
> > +    desc.hi = cpu_to_le64(desc.hi);
> > +
> > +    if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    vtd_iommu_lock(s);
> > +
> > +    queue_addr_reg = vtd_get_quad(s, DMAR_PQA_REG);
> > +    queue_tail_offset_reg = vtd_get_quad(s, DMAR_PQT_REG);
> > +    new_queue_tail_offset = ((queue_tail_offset_reg + VTD_PQA_ENTRY_SIZE) %
> > +                            (vtd_prq_size(s) * VTD_PQA_ENTRY_SIZE));
> > +    queue_head_offset_reg = vtd_get_quad(s, DMAR_PQH_REG);
> > +    queue_tail = (queue_addr_reg & VTD_PQA_ADDR) + queue_tail_offset_reg;
> > +    old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
> > +
> >     if (old_pr_status & VTD_PR_STATUS_PRO) {
> >         /*
> >          * No action is taken by hardware to report a fault
> >          * or generate an event
> >          */
> > -        return -ENOSPC;
> > +        ret = -ENOSPC;
> > +        goto out;
> >     }
> > 
> >     /* Check for overflow */
> >     if (new_queue_tail_offset == queue_head_offset_reg) {
> >         vtd_set_clear_mask_long(s, DMAR_PRS_REG, 0, VTD_PR_STATUS_PRO);
> >         vtd_generate_page_request_event(s, old_pr_status);
> > -        return -ENOSPC;
> > -    }
> > -
> > -    if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) {
> > -        return -EINVAL;
> > +        ret = -ENOSPC;
> > +        goto out;
> >     }
> > 
> > -    desc.lo = VTD_PRD_TYPE | VTD_PRD_PP(true) | VTD_PRD_RID(sid) |
> > -              VTD_PRD_PASID(vtd_as->pasid) | VTD_PRD_PMR(priv_req);
> > -    desc.hi = VTD_PRD_RDR(is_read) | VTD_PRD_WRR(is_write) |
> > -              VTD_PRD_LPIG(lpig) | VTD_PRD_PRGI(prgi) | VTD_PRD_ADDR(addr);
> > -
> > -    desc.lo = cpu_to_le64(desc.lo);
> > -    desc.hi = cpu_to_le64(desc.hi);
> >     if (dma_memory_write(&address_space_memory, queue_tail, &desc,
> > sizeof(desc),
> >                          MEMTXATTRS_UNSPECIFIED)) {
> >         error_report_once("IO error, the PQ tail cannot be updated");
> > -        return -EIO;
> > +        ret = -EIO;
> > +        goto out;
> >     }
> > 
> >     /* increment the tail register and set the pending request bit */
> >     vtd_set_quad(s, DMAR_PQT_REG, new_queue_tail_offset);
> > -    /*
> > -     * read status again so that the kernel does not miss a request.
> > -     * in some cases, we can trigger an unecessary interrupt but this strategy
> > -     * drastically improves performance as we don't need to take a lock.
> > -     */
> > -    old_pr_status = vtd_get_long(s, DMAR_PRS_REG);
> >     if (!(old_pr_status & VTD_PR_STATUS_PPR)) {
> >         vtd_set_clear_mask_long(s, DMAR_PRS_REG, 0, VTD_PR_STATUS_PPR);
> >         vtd_generate_page_request_event(s, old_pr_status);
> >     }
> > 
> > -    return 0;
> > +out:
> > +    vtd_iommu_unlock(s);
> > +    return ret;
> > }
> > 
> > static void vtd_init_iotlb_notifier(PCIBus *bus, void *opaque, int devfn,
> > --
> > 2.54.0
> 

>-----Original Message-----
>From: Clément MATHIEU--DRIF <clement.mathieu--drif@bull.com>
>Subject: Re: [PATCH v1] intel_iommu: Support concurrent page fault handling with
>PRI
>
>
>On Tue, 2026-09-08 at 07:01 +0000, Duan, Zhenzhong 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.
>>
>>
>>
>> > -----Original Message-----
>> > From: Clément MATHIEU--DRIF <[clement.mathieu--
>drif@bull.com](mailto:clement.mathieu--drif@bull.com)>
>> > Subject: [PATCH v1] intel_iommu: Support concurrent page fault handling with
>PRI
>> >
>> > PRQ slot allocation must be thread safe to avoid posting multiple
>> > commands in the same slot so we take a lock in vtd_pri_request_page.
>> >
>> > This allows multiple devices to call the page request interface
>> > of the same iommu instance.
>>
>>
>> Aren't these devices' emulations serialized by BQL?
>>
>
>Hi,
>
>Not really, emulated devices calling the PRI interface
>are discouraged from taking the BQL to avoid deadlocks.
>A device's DMA module is likely to have an internal lock.
>The following concurrent paths can lead to deadlocks:
>
>- DMA lock -> ... internal logic -> BQL lock + PRI request

I see BQL is taken in prepare_mmio_access(), isn't DMA lock taken
after calling prepare_mmio_access()?

>- PRI response -> MMIO with BQL in VT-d -> PRI notifier -> DMA lock
>
>DMA lock and BQL taken in reverse order.