Date: Mon, 19 Jun 2017 21:48:52 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r320125 - head/sys/x86/iommu Message-ID: <201706192148.v5JLmqbi059225@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kib Date: Mon Jun 19 21:48:52 2017 New Revision: 320125 URL: https://svnweb.freebsd.org/changeset/base/320125 Log: Fix batched unload for DMAR busdma in qi mode. Do not queue dmar_map_entries with zeroed gseq to dmar_qi_invalidate_locked(). Zero gseq stops the processing in the qi task. Do not assign possibly uninitialized on-stack gseq to map entries when requeuing them on unit tlb_flush queue. Random garbage in gsec is interpreted as too high invalidation sequence number and again stop the processing in the task. Make the sequence numbers generation completely contained in dmar_qi_invalidate_locked() and dmar_qi_emit_wait_seq(). Upper code directly passes boolean requesting emiting wait command instead of trying to provide hint to avoid it by passing NULL gseq pointer. Microoptimize the requeueing to tlb_flush queue by doing it for the whole queue. Diagnosed and tested by: Brett Gutstein <bgutstein@rice.edu> Discussed with: alc Sponsored by: The FreeBSD Foundation MFC after: 1 week Modified: head/sys/x86/iommu/intel_ctx.c head/sys/x86/iommu/intel_dmar.h head/sys/x86/iommu/intel_qi.c Modified: head/sys/x86/iommu/intel_ctx.c ============================================================================== --- head/sys/x86/iommu/intel_ctx.c Mon Jun 19 21:09:50 2017 (r320124) +++ head/sys/x86/iommu/intel_ctx.c Mon Jun 19 21:48:52 2017 (r320125) @@ -703,7 +703,7 @@ dmar_domain_unload_entry(struct dmar_map_entry *entry, if (unit->qi_enabled) { DMAR_LOCK(unit); dmar_qi_invalidate_locked(entry->domain, entry->start, - entry->end - entry->start, &entry->gseq); + entry->end - entry->start, &entry->gseq, true); if (!free) entry->flags |= DMAR_MAP_ENTRY_QI_NF; TAILQ_INSERT_TAIL(&unit->tlb_flush_entries, entry, dmamap_link); @@ -715,16 +715,14 @@ dmar_domain_unload_entry(struct dmar_map_entry *entry, } } -static struct dmar_qi_genseq * -dmar_domain_unload_gseq(struct dmar_domain *domain, - struct dmar_map_entry *entry, struct dmar_qi_genseq *gseq) +static bool +dmar_domain_unload_emit_wait(struct dmar_domain *domain, + struct dmar_map_entry *entry) { - if (TAILQ_NEXT(entry, dmamap_link) != NULL) - return (NULL); - if (domain->batch_no++ % dmar_batch_coalesce != 0) - return (NULL); - return (gseq); + if (TAILQ_NEXT(entry, dmamap_link) == NULL) + return (true); + return (domain->batch_no++ % dmar_batch_coalesce == 0); } void @@ -733,7 +731,6 @@ dmar_domain_unload(struct dmar_domain *domain, { struct dmar_unit *unit; struct dmar_map_entry *entry, *entry1; - struct dmar_qi_genseq gseq; int error; unit = domain->dmar; @@ -757,17 +754,11 @@ dmar_domain_unload(struct dmar_domain *domain, KASSERT(unit->qi_enabled, ("loaded entry left")); DMAR_LOCK(unit); TAILQ_FOREACH(entry, entries, dmamap_link) { - entry->gseq.gen = 0; - entry->gseq.seq = 0; dmar_qi_invalidate_locked(domain, entry->start, entry->end - - entry->start, dmar_domain_unload_gseq(domain, entry, - &gseq)); + entry->start, &entry->gseq, + dmar_domain_unload_emit_wait(domain, entry)); } - TAILQ_FOREACH_SAFE(entry, entries, dmamap_link, entry1) { - entry->gseq = gseq; - TAILQ_REMOVE(entries, entry, dmamap_link); - TAILQ_INSERT_TAIL(&unit->tlb_flush_entries, entry, dmamap_link); - } + TAILQ_CONCAT(&unit->tlb_flush_entries, entries, dmamap_link); DMAR_UNLOCK(unit); } Modified: head/sys/x86/iommu/intel_dmar.h ============================================================================== --- head/sys/x86/iommu/intel_dmar.h Mon Jun 19 21:09:50 2017 (r320124) +++ head/sys/x86/iommu/intel_dmar.h Mon Jun 19 21:48:52 2017 (r320125) @@ -305,7 +305,7 @@ void dmar_disable_qi_intr(struct dmar_unit *unit); int dmar_init_qi(struct dmar_unit *unit); void dmar_fini_qi(struct dmar_unit *unit); void dmar_qi_invalidate_locked(struct dmar_domain *domain, dmar_gaddr_t start, - dmar_gaddr_t size, struct dmar_qi_genseq *pseq); + dmar_gaddr_t size, struct dmar_qi_genseq *psec, bool emit_wait); void dmar_qi_invalidate_ctx_glob_locked(struct dmar_unit *unit); void dmar_qi_invalidate_iotlb_glob_locked(struct dmar_unit *unit); void dmar_qi_invalidate_iec_glob(struct dmar_unit *unit); Modified: head/sys/x86/iommu/intel_qi.c ============================================================================== --- head/sys/x86/iommu/intel_qi.c Mon Jun 19 21:09:50 2017 (r320124) +++ head/sys/x86/iommu/intel_qi.c Mon Jun 19 21:48:52 2017 (r320125) @@ -171,7 +171,8 @@ dmar_qi_emit_wait_descr(struct dmar_unit *unit, uint32 } static void -dmar_qi_emit_wait_seq(struct dmar_unit *unit, struct dmar_qi_genseq *pseq) +dmar_qi_emit_wait_seq(struct dmar_unit *unit, struct dmar_qi_genseq *pseq, + bool emit_wait) { struct dmar_qi_genseq gsec; uint32_t seq; @@ -192,7 +193,10 @@ dmar_qi_emit_wait_seq(struct dmar_unit *unit, struct d seq = unit->inv_waitd_seq++; pseq->gen = unit->inv_waitd_gen; pseq->seq = seq; - dmar_qi_emit_wait_descr(unit, seq, true, true, false); + if (emit_wait) { + dmar_qi_ensure(unit, 1); + dmar_qi_emit_wait_descr(unit, seq, true, true, false); + } } static void @@ -215,7 +219,7 @@ dmar_qi_wait_for_seq(struct dmar_unit *unit, const str void dmar_qi_invalidate_locked(struct dmar_domain *domain, dmar_gaddr_t base, - dmar_gaddr_t size, struct dmar_qi_genseq *pseq) + dmar_gaddr_t size, struct dmar_qi_genseq *pseq, bool emit_wait) { struct dmar_unit *unit; dmar_gaddr_t isize; @@ -232,10 +236,7 @@ dmar_qi_invalidate_locked(struct dmar_domain *domain, DMAR_IQ_DESCR_IOTLB_DID(domain->domain), base | am); } - if (pseq != NULL) { - dmar_qi_ensure(unit, 1); - dmar_qi_emit_wait_seq(unit, pseq); - } + dmar_qi_emit_wait_seq(unit, pseq, emit_wait); dmar_qi_advance_tail(unit); } @@ -247,7 +248,7 @@ dmar_qi_invalidate_ctx_glob_locked(struct dmar_unit *u DMAR_ASSERT_LOCKED(unit); dmar_qi_ensure(unit, 2); dmar_qi_emit(unit, DMAR_IQ_DESCR_CTX_INV | DMAR_IQ_DESCR_CTX_GLOB, 0); - dmar_qi_emit_wait_seq(unit, &gseq); + dmar_qi_emit_wait_seq(unit, &gseq, true); dmar_qi_advance_tail(unit); dmar_qi_wait_for_seq(unit, &gseq, false); } @@ -261,7 +262,7 @@ dmar_qi_invalidate_iotlb_glob_locked(struct dmar_unit dmar_qi_ensure(unit, 2); dmar_qi_emit(unit, DMAR_IQ_DESCR_IOTLB_INV | DMAR_IQ_DESCR_IOTLB_GLOB | DMAR_IQ_DESCR_IOTLB_DW | DMAR_IQ_DESCR_IOTLB_DR, 0); - dmar_qi_emit_wait_seq(unit, &gseq); + dmar_qi_emit_wait_seq(unit, &gseq, true); dmar_qi_advance_tail(unit); dmar_qi_wait_for_seq(unit, &gseq, false); } @@ -274,7 +275,7 @@ dmar_qi_invalidate_iec_glob(struct dmar_unit *unit) DMAR_ASSERT_LOCKED(unit); dmar_qi_ensure(unit, 2); dmar_qi_emit(unit, DMAR_IQ_DESCR_IEC_INV, 0); - dmar_qi_emit_wait_seq(unit, &gseq); + dmar_qi_emit_wait_seq(unit, &gseq, true); dmar_qi_advance_tail(unit); dmar_qi_wait_for_seq(unit, &gseq, false); } @@ -298,7 +299,7 @@ dmar_qi_invalidate_iec(struct dmar_unit *unit, u_int s DMAR_IQ_DESCR_IEC_IM(l), 0); } dmar_qi_ensure(unit, 1); - dmar_qi_emit_wait_seq(unit, &gseq); + dmar_qi_emit_wait_seq(unit, &gseq, true); dmar_qi_advance_tail(unit); /* @@ -344,8 +345,7 @@ dmar_qi_task(void *arg, int pending __unused) entry = TAILQ_FIRST(&unit->tlb_flush_entries); if (entry == NULL) break; - if ((entry->gseq.gen == 0 && entry->gseq.seq == 0) || - !dmar_qi_seq_processed(unit, &entry->gseq)) + if (!dmar_qi_seq_processed(unit, &entry->gseq)) break; TAILQ_REMOVE(&unit->tlb_flush_entries, entry, dmamap_link); DMAR_UNLOCK(unit); @@ -432,7 +432,7 @@ dmar_fini_qi(struct dmar_unit *unit) DMAR_LOCK(unit); /* quisce */ dmar_qi_ensure(unit, 1); - dmar_qi_emit_wait_seq(unit, &gseq); + dmar_qi_emit_wait_seq(unit, &gseq, true); dmar_qi_advance_tail(unit); dmar_qi_wait_for_seq(unit, &gseq, false); /* only after the quisce, disable queue */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201706192148.v5JLmqbi059225>