IOTINVAL currently walks the IOATC hash table and invalidates matching entries by setting iot->perm to IOMMU_NONE. The invalidated entries remain in the hash table afterwards.
Under workloads with frequent DMA map/unmap activity, invalid entries can accumulate while only a smaller number of entries remain valid. Subsequent IOTINVAL operations continue to scan these entries. They are also included in g_hash_table_size(), causing the IOATC to reach iot_limit earlier and the entire cache to be flushed more frequently. Remove matching entries during invalidation instead of leaving them in the hash table with IOMMU_NONE. Removing an entry also frees it, so a concurrent lookup must not retain a pointer to an entry being removed. Accesses that modify the IOATC hash table must also be serialized. Add iot_lock to protect IOATC lookup, update, invalidation, and reset operations, keeping both the hash table state and cached entry lifetime valid while they are accessed. The measurements use a Linux 7.3-rc5 guest based on linux-iommu/next commit cec2dc7663e7 with v6 of the "iommu/riscv: Enable MSI remapping, IOMMU_DMA and VFIO" series applied. The QEMU behavior is not specific to this guest kernel; this configuration makes it easier to exercise through normal DMA map/unmap activity. In a high-churn NVMe workload with frequent DMA map/unmap activity, runtime drops from 168 seconds to 35 seconds, a 4.8x speedup. A short fio workload with a small working set remains within measurement noise, showing no measurable regression from the added locking. Across three runs for each version of a 300-second fio workload with a 4 GiB working set, median throughput improves from 68.6 MiB/s to 75.0 MiB/s, an increase of 9.3%. Signed-off-by: Tan Chi <[email protected]> --- hw/riscv/riscv-iommu.c | 160 ++++++++++++++++++++--------------------- hw/riscv/riscv-iommu.h | 1 + 2 files changed, 79 insertions(+), 82 deletions(-) diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c index 323a041b4a..85121f6468 100644 --- a/hw/riscv/riscv-iommu.c +++ b/hw/riscv/riscv-iommu.c @@ -1582,107 +1582,96 @@ static guint riscv_iommu_iot_hash(gconstpointer v) /* GV: 0 AV: 0 PSCV: 0 GVMA: 0 */ /* GV: 0 AV: 0 GVMA: 1 */ static -void riscv_iommu_iot_inval_all(gpointer key, gpointer value, gpointer data) +gboolean riscv_iommu_iot_inval_all(gpointer key, gpointer value, gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag; } /* GV: 0 AV: 0 PSCV: 1 GVMA: 0 */ static -void riscv_iommu_iot_inval_pscid(gpointer key, gpointer value, gpointer data) +gboolean riscv_iommu_iot_inval_pscid(gpointer key, gpointer value, + gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->pscid == arg->pscid) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->pscid == arg->pscid; } /* GV: 0 AV: 1 PSCV: 0 GVMA: 0 */ static -void riscv_iommu_iot_inval_iova(gpointer key, gpointer value, gpointer data) +gboolean riscv_iommu_iot_inval_iova(gpointer key, gpointer value, gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->iova == arg->iova) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->iova == arg->iova; } /* GV: 0 AV: 1 PSCV: 1 GVMA: 0 */ -static void riscv_iommu_iot_inval_pscid_iova(gpointer key, gpointer value, - gpointer data) +static gboolean riscv_iommu_iot_inval_pscid_iova(gpointer key, + gpointer value, + gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->pscid == arg->pscid && - iot->iova == arg->iova) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->pscid == arg->pscid && + iot->iova == arg->iova; } /* GV: 1 AV: 0 PSCV: 0 GVMA: 0 */ /* GV: 1 AV: 0 GVMA: 1 */ -static -void riscv_iommu_iot_inval_gscid(gpointer key, gpointer value, gpointer data) +static gboolean riscv_iommu_iot_inval_gscid(gpointer key, gpointer value, + gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->gscid == arg->gscid) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->gscid == arg->gscid; } /* GV: 1 AV: 0 PSCV: 1 GVMA: 0 */ -static void riscv_iommu_iot_inval_gscid_pscid(gpointer key, gpointer value, - gpointer data) +static gboolean riscv_iommu_iot_inval_gscid_pscid(gpointer key, + gpointer value, + gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->gscid == arg->gscid && - iot->pscid == arg->pscid) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->gscid == arg->gscid && + iot->pscid == arg->pscid; } /* GV: 1 AV: 1 PSCV: 0 GVMA: 0 */ /* GV: 1 AV: 1 GVMA: 1 */ -static void riscv_iommu_iot_inval_gscid_iova(gpointer key, gpointer value, - gpointer data) +static gboolean riscv_iommu_iot_inval_gscid_iova(gpointer key, + gpointer value, + gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->gscid == arg->gscid && - iot->iova == arg->iova) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->gscid == arg->gscid && + iot->iova == arg->iova; } /* GV: 1 AV: 1 PSCV: 1 GVMA: 0 */ -static void riscv_iommu_iot_inval_gscid_pscid_iova(gpointer key, gpointer value, - gpointer data) +static gboolean riscv_iommu_iot_inval_gscid_pscid_iova(gpointer key, + gpointer value, + gpointer data) { RISCVIOMMUEntry *iot = (RISCVIOMMUEntry *) value; RISCVIOMMUEntry *arg = (RISCVIOMMUEntry *) data; - if (iot->tag == arg->tag && - iot->gscid == arg->gscid && - iot->pscid == arg->pscid && - iot->iova == arg->iova) { - iot->perm = IOMMU_NONE; - } + return iot->tag == arg->tag && + iot->gscid == arg->gscid && + iot->pscid == arg->pscid && + iot->iova == arg->iova; } -/* caller should keep ref-count for iot_cache object */ +/* caller must hold s->iot_lock */ static RISCVIOMMUEntry *riscv_iommu_iot_lookup(RISCVIOMMUContext *ctx, GHashTable *iot_cache, hwaddr iova, RISCVIOMMUTransTag transtag) { @@ -1695,27 +1684,26 @@ static RISCVIOMMUEntry *riscv_iommu_iot_lookup(RISCVIOMMUContext *ctx, return g_hash_table_lookup(iot_cache, &key); } -/* caller should keep ref-count for iot_cache object */ -static void riscv_iommu_iot_update(RISCVIOMMUState *s, - GHashTable *iot_cache, RISCVIOMMUEntry *iot) +/* caller must hold s->iot_lock */ +static void riscv_iommu_iot_update(RISCVIOMMUState *s, RISCVIOMMUEntry *iot) { if (!s->iot_limit) { return; } if (g_hash_table_size(s->iot_cache) >= s->iot_limit) { - iot_cache = g_hash_table_new_full(riscv_iommu_iot_hash, + GHashTable *old_cache = s->iot_cache; + s->iot_cache = g_hash_table_new_full(riscv_iommu_iot_hash, riscv_iommu_iot_equal, g_free, NULL); - g_hash_table_unref(qatomic_xchg(&s->iot_cache, iot_cache)); + g_hash_table_unref(old_cache); } - g_hash_table_add(iot_cache, iot); + g_hash_table_add(s->iot_cache, iot); } -static void riscv_iommu_iot_inval(RISCVIOMMUState *s, GHFunc func, +static void riscv_iommu_iot_inval(RISCVIOMMUState *s, GHRFunc func, uint32_t gscid, uint32_t pscid, hwaddr iova, RISCVIOMMUTransTag transtag) { - GHashTable *iot_cache; RISCVIOMMUEntry key = { .tag = transtag, .gscid = gscid, @@ -1723,9 +1711,9 @@ static void riscv_iommu_iot_inval(RISCVIOMMUState *s, GHFunc func, .iova = PPN_DOWN(iova), }; - iot_cache = g_hash_table_ref(s->iot_cache); - g_hash_table_foreach(iot_cache, func, &key); - g_hash_table_unref(iot_cache); + qemu_mutex_lock(&s->iot_lock); + g_hash_table_foreach_remove(s->iot_cache, func, &key); + qemu_mutex_unlock(&s->iot_lock); } static RISCVIOMMUTransTag riscv_iommu_get_transtag(RISCVIOMMUContext *ctx) @@ -1750,12 +1738,10 @@ static int riscv_iommu_translate(RISCVIOMMUState *s, RISCVIOMMUContext *ctx, IOMMUAccessFlags perm; bool enable_pid; bool enable_pri; - GHashTable *iot_cache; int fault; riscv_iommu_hpm_incr_ctr(s, ctx, RISCV_IOMMU_HPMEVENT_URQ); - iot_cache = g_hash_table_ref(s->iot_cache); enable_pri = (iotlb->perm == IOMMU_NONE) && (ctx->tc & RISCV_IOMMU_DC_TC_EN_PRI); enable_pid = (ctx->tc & RISCV_IOMMU_DC_TC_PDTV); @@ -1771,15 +1757,18 @@ static int riscv_iommu_translate(RISCVIOMMUState *s, RISCVIOMMUContext *ctx, } } - iot = riscv_iommu_iot_lookup(ctx, iot_cache, iotlb->iova, transtag); + qemu_mutex_lock(&s->iot_lock); + iot = riscv_iommu_iot_lookup(ctx, s->iot_cache, iotlb->iova, transtag); perm = iot ? iot->perm : IOMMU_NONE; if (perm != IOMMU_NONE) { iotlb->translated_addr = PPN_PHYS(iot->phys); iotlb->addr_mask = ~TARGET_PAGE_MASK; iotlb->perm = perm; fault = 0; + qemu_mutex_unlock(&s->iot_lock); goto done; } + qemu_mutex_unlock(&s->iot_lock); riscv_iommu_hpm_incr_ctr(s, ctx, RISCV_IOMMU_HPMEVENT_TLB_MISS); @@ -1805,12 +1794,12 @@ static int riscv_iommu_translate(RISCVIOMMUState *s, RISCVIOMMUContext *ctx, iot->pscid = get_field(ctx->ta, RISCV_IOMMU_DC_TA_PSCID); iot->perm = iotlb->perm; iot->tag = transtag; - riscv_iommu_iot_update(s, iot_cache, iot); + qemu_mutex_lock(&s->iot_lock); + riscv_iommu_iot_update(s, iot); + qemu_mutex_unlock(&s->iot_lock); } done: - g_hash_table_unref(iot_cache); - if (enable_pri && fault) { struct riscv_iommu_pq_record pr = {0}; if (enable_pid) { @@ -1969,7 +1958,8 @@ static void riscv_iommu_process_cq_tail(RISCVIOMMUState *s) dma_addr_t addr; uint32_t tail, head, ctrl; uint64_t cmd_opcode; - GHFunc func; + GHRFunc iot_func; + GHFunc ctx_func; ctrl = riscv_iommu_reg_get32(s, RISCV_IOMMU_REG_CQCSR); tail = riscv_iommu_reg_get32(s, RISCV_IOMMU_REG_CQT) & s->cq_mask; @@ -2032,18 +2022,18 @@ static void riscv_iommu_process_cq_tail(RISCVIOMMUState *s) goto cmd_ill; } - func = riscv_iommu_iot_inval_all; + iot_func = riscv_iommu_iot_inval_all; if (gv) { - func = (av) ? riscv_iommu_iot_inval_gscid_iova : + iot_func = (av) ? riscv_iommu_iot_inval_gscid_iova : riscv_iommu_iot_inval_gscid; } riscv_iommu_iot_inval( - s, func, gscid, pscid, iova, RISCV_IOMMU_TRANS_TAG_VG); + s, iot_func, gscid, pscid, iova, RISCV_IOMMU_TRANS_TAG_VG); riscv_iommu_iot_inval( - s, func, gscid, pscid, iova, RISCV_IOMMU_TRANS_TAG_VN); + s, iot_func, gscid, pscid, iova, RISCV_IOMMU_TRANS_TAG_VN); break; } @@ -2063,24 +2053,24 @@ static void riscv_iommu_process_cq_tail(RISCVIOMMUState *s) if (gv) { transtag = RISCV_IOMMU_TRANS_TAG_VN; if (pscv) { - func = (av) ? riscv_iommu_iot_inval_gscid_pscid_iova : + iot_func = (av) ? riscv_iommu_iot_inval_gscid_pscid_iova : riscv_iommu_iot_inval_gscid_pscid; } else { - func = (av) ? riscv_iommu_iot_inval_gscid_iova : + iot_func = (av) ? riscv_iommu_iot_inval_gscid_iova : riscv_iommu_iot_inval_gscid; } } else { transtag = RISCV_IOMMU_TRANS_TAG_SS; if (pscv) { - func = (av) ? riscv_iommu_iot_inval_pscid_iova : + iot_func = (av) ? riscv_iommu_iot_inval_pscid_iova : riscv_iommu_iot_inval_pscid; } else { - func = (av) ? riscv_iommu_iot_inval_iova : + iot_func = (av) ? riscv_iommu_iot_inval_iova : riscv_iommu_iot_inval_all; } } - riscv_iommu_iot_inval(s, func, gscid, pscid, iova, transtag); + riscv_iommu_iot_inval(s, iot_func, gscid, pscid, iova, transtag); break; } @@ -2088,12 +2078,12 @@ static void riscv_iommu_process_cq_tail(RISCVIOMMUState *s) RISCV_IOMMU_CMD_IODIR_OPCODE): if (!(cmd.dword0 & RISCV_IOMMU_CMD_IODIR_DV)) { /* invalidate all device context cache mappings */ - func = riscv_iommu_ctx_inval_all; + ctx_func = riscv_iommu_ctx_inval_all; } else { /* invalidate all device context matching DID */ - func = riscv_iommu_ctx_inval_devid; + ctx_func = riscv_iommu_ctx_inval_devid; } - riscv_iommu_ctx_inval(s, func, + riscv_iommu_ctx_inval(s, ctx_func, get_field(cmd.dword0, RISCV_IOMMU_CMD_IODIR_DID), 0); break; @@ -2103,9 +2093,9 @@ static void riscv_iommu_process_cq_tail(RISCVIOMMUState *s) /* illegal command arguments IODIR_PDT & DV == 0 */ goto cmd_ill; } else { - func = riscv_iommu_ctx_inval_devid_procid; + ctx_func = riscv_iommu_ctx_inval_devid_procid; } - riscv_iommu_ctx_inval(s, func, + riscv_iommu_ctx_inval(s, ctx_func, get_field(cmd.dword0, RISCV_IOMMU_CMD_IODIR_DID), get_field(cmd.dword0, RISCV_IOMMU_CMD_IODIR_PID)); break; @@ -2671,6 +2661,7 @@ static void riscv_iommu_instance_init(Object *obj) riscv_iommu_ctx_equal, g_free, NULL); + qemu_mutex_init(&s->iot_lock); s->iot_cache = g_hash_table_new_full(riscv_iommu_iot_hash, riscv_iommu_iot_equal, g_free, NULL); @@ -2690,6 +2681,8 @@ static void riscv_iommu_instance_finalize(Object *obj) g_hash_table_unref(s->ctx_cache); g_hash_table_unref(s->iot_cache); + + qemu_mutex_destroy(&s->iot_lock); } static void riscv_iommu_realize(DeviceState *dev, Error **errp) @@ -2850,7 +2843,10 @@ void riscv_iommu_reset(RISCVIOMMUState *s) riscv_iommu_reg_set32(s, RISCV_IOMMU_REG_IPSR, 0); g_hash_table_remove_all(s->ctx_cache); + + qemu_mutex_lock(&s->iot_lock); g_hash_table_remove_all(s->iot_cache); + qemu_mutex_unlock(&s->iot_lock); } static const Property riscv_iommu_properties[] = { diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h index da70e8bfa8..16d257a106 100644 --- a/hw/riscv/riscv-iommu.h +++ b/hw/riscv/riscv-iommu.h @@ -73,6 +73,7 @@ struct RISCVIOMMUState { GHashTable *iot_cache; /* IO Translated Address Cache */ unsigned iot_limit; /* IO Translation Cache size limit */ + QemuMutex iot_lock; /* MMIO Hardware Interface */ MemoryRegion regs_mr; -- 2.53.0
