Skip site navigation (1)Skip section navigation (2)
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>