A few minor comments, In any case: Reviewed-by: Clement Mathieu--Drif <[email protected]>
On Wed, 2026-09-09 at 15:28 +0800, Zhenzhong Duan 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. > > > Propagate guest's PRQ response to host by writing to fault_fd. > Create a new VTDPRQEntry to cache cookie for each fault group, > this cookie is used to mark the fault group on host side. > > If there is either PRQ overflow or full error condition, propagate > a success response to host for LPIG request with the PRQ requests > dropped. For other error conditions, propagate response of invalid > request. > > Signed-off-by: Zhenzhong Duan > <[[email protected]](mailto:[email protected])> > Reviewed-by: Clement Mathieu--Drif > <[[email protected]](mailto:[email protected])> > Tested-by: Xudong Hao <[[email protected]](mailto:[email protected])> > --- > hw/i386/intel_iommu_accel.h | 12 +++++ > include/hw/i386/intel_iommu.h | 6 +++ > hw/i386/intel_iommu.c | 4 ++ > hw/i386/intel_iommu_accel.c | 99 ++++++++++++++++++++++++++++++++--- > hw/i386/trace-events | 1 + > 5 files changed, 115 insertions(+), 7 deletions(-) > > diff --git a/hw/i386/intel_iommu_accel.h b/hw/i386/intel_iommu_accel.h > index e0319749a5..0cc22b2cbb 100644 > --- a/hw/i386/intel_iommu_accel.h > +++ b/hw/i386/intel_iommu_accel.h > @@ -20,6 +20,7 @@ typedef struct VTDAccelPASIDCacheEntry { > uint32_t fs_hwpt_id; > uint32_t fault_id; > int fault_fd; > + QLIST_HEAD(, VTDPRQEntry) vtd_prq_list; > QLIST_ENTRY(VTDAccelPASIDCacheEntry) next; > } VTDAccelPASIDCacheEntry; > > @@ -31,6 +32,9 @@ void vtd_flush_host_piotlb_all_locked(IntelIOMMUState *s, > uint16_t domain_id, > uint64_t npages, bool ih); > void vtd_accel_pasid_cache_sync(IntelIOMMUState *s, VTDPASIDCacheInfo > *pc_info); > void vtd_accel_pasid_cache_reset(IntelIOMMUState *s); > +bool vtd_accel_propagate_page_group_response(IntelIOMMUState *s, > + uint16_t rid, uint32_t pasid, > + IOMMUPRIResponse *response); > void vtd_iommu_ops_update_accel(PCIIOMMUOps *ops); > #else > static inline bool vtd_check_hiod_accel(IntelIOMMUState *s, > @@ -69,6 +73,14 @@ static inline void > vtd_accel_pasid_cache_reset(IntelIOMMUState *s) > { > } > > +static inline > +bool vtd_accel_propagate_page_group_response(IntelIOMMUState *s, > + uint16_t rid, uint32_t pasid, > + IOMMUPRIResponse *response) > +{ > + return false; > +} > + > static inline void vtd_iommu_ops_update_accel(PCIIOMMUOps *ops) > { > } > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h > index 1842ba5840..5d44eac0ed 100644 > --- a/include/hw/i386/intel_iommu.h > +++ b/include/hw/i386/intel_iommu.h > @@ -100,6 +100,12 @@ typedef struct VTDPASIDCacheEntry { > bool valid; > } VTDPASIDCacheEntry; > > +typedef struct VTDPRQEntry { > + uint32_t grpid; > + uint32_t cookie; > + QLIST_ENTRY(VTDPRQEntry) next; > +} VTDPRQEntry; > + > struct VTDAddressSpace { > PCIBus *bus; > uint8_t devfn; > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 022923a058..4dfa2dd9a6 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -3380,6 +3380,10 @@ static bool > vtd_process_page_group_response_desc(IntelIOMMUState *s, > response.response_code = IOMMU_PRI_RESP_FAILURE; > } > > + if (vtd_accel_propagate_page_group_response(s, rid, pasid, &response)) { > > + return true; > + } > + > if (vtd_dev_as->pri_notifier) { > vtd_dev_as->pri_notifier->notify(vtd_dev_as->pri_notifier, > &response); > } > diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c > index c0bd249c4a..903c74bad1 100644 > --- a/hw/i386/intel_iommu_accel.c > +++ b/hw/i386/intel_iommu_accel.c > @@ -78,6 +78,62 @@ VTDHostIOMMUDevice *vtd_find_hiod_iommufd(VTDAddressSpace > *as) > return NULL; > } > > +static void vtd_write_fs_faultq(VTDAccelPASIDCacheEntry *vtd_pce, > + uint32_t cookie, uint32_t code) > +{ > + struct iommu_hwpt_page_response resp = {cookie, code}; I know this will never cause a problem, but we could be a bit more cautious = { .cookie = cookie, .code = code } > + uint32_t id = vtd_pce->fault_id; > + int fd = vtd_pce->fault_fd; > + ssize_t bytes; > + > + bytes = write(fd, &resp, sizeof(resp)); > + trace_vtd_write_fs_faultq(id, fd, cookie, code, bytes); > + if (bytes < 0) { > + error_report_once("FAULTQ(id %u): write failed " > + "[cookie 0x%x code 0x%x] (%m)", id, cookie, code); > > + } > +} > + > +static void vtd_prq_handle_pasid_response(VTDAccelPASIDCacheEntry *vtd_pce, > + IOMMUPRIResponse *response) > +{ > + VTDPRQEntry *prqe, *tmp; > + > + QLIST_FOREACH_SAFE(prqe, &vtd_pce->vtd_prq_list, next, tmp) { > + if (prqe->grpid != response->prgi) { > + continue; > + } > + > + vtd_write_fs_faultq(vtd_pce, prqe->cookie, response->response_code); > > + > + QLIST_REMOVE(prqe, next); > + g_free(prqe); > + break; > + } > +} > + > +bool vtd_accel_propagate_page_group_response(IntelIOMMUState *s, > + uint16_t rid, uint32_t pasid, > + IOMMUPRIResponse *response) > +{ > + VTDAddressSpace *vtd_as = vtd_get_as_by_sid(s, rid); > + VTDAccelPASIDCacheEntry *vtd_pce; > + VTDHostIOMMUDevice *vtd_hiod = vtd_find_hiod_iommufd(vtd_as); > + > + if (!vtd_hiod) { > + return false; > + } > + > + QLIST_FOREACH(vtd_pce, &vtd_hiod->pasid_cache_list, next) { > + if (vtd_pce->pasid == pasid) { > + vtd_prq_handle_pasid_response(vtd_pce, response); > + return true; > + } > + } > + > + return false; > +} > + > static void vtd_propagate_recoverable_fault(VTDAccelPASIDCacheEntry > *vtd_pce, > struct iommu_hwpt_pgfault > *fault, > unsigned cnt) > @@ -88,14 +144,42 @@ static void > vtd_propagate_recoverable_fault(VTDAccelPASIDCacheEntry *vtd_pce, > > for (; cnt--; fault++) { > bool last_page = fault->flags & IOMMU_PGFAULT_FLAGS_LAST_PAGE; > + int ret; > + > + ret = vtd_pri_request_page(vtd_hiod->bus, vtd_hiod->iommu_state, > + vtd_hiod->devfn, pasid, > + fault->perm & IOMMU_PGFAULT_PERM_PRIV, > + fault->perm & IOMMU_PGFAULT_PERM_EXEC, > + fault->addr, last_page, fault->grpid, > + fault->perm & IOMMU_PGFAULT_PERM_READ, > + fault->perm & IOMMU_PGFAULT_PERM_WRITE); > + if (!last_page) { > + continue; > + } > + > + if (!ret) { > + VTDPRQEntry *prqe = g_malloc0(sizeof(*prqe)); > + > + prqe->grpid = fault->grpid; > + prqe->cookie = fault->cookie; > + QLIST_INSERT_HEAD(&vtd_pce->vtd_prq_list, prqe, next); > + continue; > + } > > - vtd_pri_request_page(vtd_hiod->bus, vtd_hiod->iommu_state, > - vtd_hiod->devfn, pasid, > - fault->perm & IOMMU_PGFAULT_PERM_PRIV, > - fault->perm & IOMMU_PGFAULT_PERM_EXEC, > - fault->addr, last_page, fault->grpid, > - fault->perm & IOMMU_PGFAULT_PERM_READ, > - fault->perm & IOMMU_PGFAULT_PERM_WRITE); > + /* > + * vt-d spec 7.4.1: > + * > + * Hardware generates page group response with code of success for > + * PRQ overflow and PRQ full, with code of invalid request for > + * other error conditions. > + */ > + if (ret == -ENOSPC) { > + vtd_write_fs_faultq(vtd_pce, fault->cookie, > + IOMMUFD_PAGE_RESP_SUCCESS); > + } else { > + vtd_write_fs_faultq(vtd_pce, fault->cookie, > + IOMMUFD_PAGE_RESP_INVALID); > + } > } > } > > @@ -426,6 +510,7 @@ static void vtd_accel_fill_pc(VTDHostIOMMUDevice > *vtd_hiod, uint32_t pasid, > vtd_pce->pasid = pasid; > vtd_pce->pasid_entry = *pe; > vtd_pce->fault_fd = -1; > + QLIST_INIT(&vtd_pce->vtd_prq_list); > QLIST_INSERT_HEAD(&vtd_hiod->pasid_cache_list, vtd_pce, next); > > if (!vtd_device_attach_iommufd(vtd_pce, &local_err)) { > diff --git a/hw/i386/trace-events b/hw/i386/trace-events > index fcd3f33f0a..2b2baeb408 100644 > --- a/hw/i386/trace-events > +++ b/hw/i386/trace-events > @@ -78,6 +78,7 @@ vtd_device_attach_hwpt(uint32_t dev_id, uint32_t pasid, > uint32_t hwpt_id, int re > vtd_device_detach_hwpt(uint32_t dev_id, uint32_t pasid, int ret) "dev_id %d > pasid %d ret: %d" > vtd_device_reattach_def_hwpt(uint32_t dev_id, uint32_t pasid, uint32_t > hwpt_id, int ret) "dev_id %d pasid %d hwpt_id %d, ret: %d" > vtd_read_fs_faultq(uint32_t fault_id, uint32_t fault_fd, ssize_t bytes) > "fault_id %d fault_fd %d ret: %zd" > +vtd_write_fs_faultq(uint32_t fault_id, uint32_t fault_fd, uint32_t cookie, > uint32_t code, ssize_t bytes) "fault_id %d fault_fd %d cookie %d code %d ret: > %zd" Yes, sorry for the late answer, I not against renaming ssize_t bytes to ssize_t ret > > # amd_iommu.c > amdvi_evntlog_fail(uint64_t addr, uint32_t head) "error: fail to write at > addr 0x%"PRIx64" + offset 0x%"PRIx32 > -- > 2.52.0 >
