Honor DTO_SUBMIT_BOF only on work queues configured with block-on-fault

README.md

@@ -128,10 +128,11 @@ could not finish, so once it returns `DTO_ASYNC_DONE` every result is valid.
 
 Explicit submits are not subject to `DTO_MIN_BYTES`: the caller decides what to
 offload. `DTO_SUBMIT_BOF` makes the device wait for page faults to be
-resolved; it requires a work queue configured with block-on-fault
-(`accel-config config-wq --block-on-fault=1`), and on one without it the
-device rejects the operation. Without the flag a page fault stops the
-operation partway, on any work queue. `dto_async_crc_val` and `dto_batch_crc`
+resolved when the work queue is configured with block-on-fault
+(`accel-config config-wq --block-on-fault=1`); on a queue without it the flag
+is dropped, since the device would reject it. Without the flag in effect a
+page fault stops the operation partway (`DTO_ASYNC_FAILED`, status `0x03`), so
+callers handle that case either way. `dto_async_crc_val` and `dto_batch_crc`
 return the raw CRC32C (the SSE4.2 `crc32` instruction applied from 0, no final
 inversion); the seeded `*_crc32c` calls return standard CRC32C and chain across
 buffers. See `dto.h` for the full contract.

TESTING.md

@@ -77,7 +77,9 @@ sudo DTO_TEST_EXPECT_DSA=1 ./dto-test-functional-explicit
 ```
 
 `test_async_bof` checks the block-on-fault behavior against whichever setting
-the work queue has (it reports which), so run the suite once with
+the work queue has (it reports which): with `block_on_fault` enabled a
+`DTO_SUBMIT_BOF` copy into untouched pages completes, without it DTO drops the
+flag and the copy stops at the first untouched page. Run the suite once with
 `block_on_fault` enabled and once without to cover both. It assumes all work
 queues DTO uses share the same setting.
 

dto.c

@@ -87,6 +87,9 @@ struct dto_wq {
 	int wq_size;
 	uint32_t max_transfer_size;
 	uint32_t max_batch_size;
+	/* the work queue's block_on_fault attribute: whether descriptors may
+	 * carry IDXD_OP_FLAG_BOF (a queue without it rejects them) */
+	bool block_on_fault;
 	int wq_fd;
 	void *wq_portal;
 	bool wq_mmapped;
@@ -1015,6 +1018,12 @@ static int dsa_init_from_wq_list(char *wq_list)
 			goto fail_wq;
 		}
 
+		/* Not fatal when the attribute cannot be read: assume the
+		 * queue has no block-on-fault and never set the flag. */
+		wqs[num_wqs].block_on_fault =
+			dto_get_param_ullong(dir_fd, "block_on_fault", &rc) == 1 && !rc;
+		rc = 0;
+
 		dto_get_param_string(dir_fd, "mode", wq_mode);
 
 		if (wq_mode[0] == '\0') {
@@ -1179,6 +1188,7 @@ static int dsa_init_from_accfg(void)
 			wqs[num_wqs].wq_size = accfg_wq_get_size(wq);
 			wqs[num_wqs].max_transfer_size = accfg_wq_get_max_transfer_size(wq);
 			wqs[num_wqs].max_batch_size = accfg_wq_get_max_batch_size(wq);
+			wqs[num_wqs].block_on_fault = accfg_wq_get_block_on_fault(wq) > 0;
 
 			wqs[num_wqs].acc_wq = wq;
 			wqs[num_wqs].dsa_gencap = accfg_device_get_gen_cap(device);
@@ -1612,8 +1622,9 @@ static int init_dto(void)
 				log_level, collect_stats, use_std_lib_calls, dsa_min_size,
 				cpu_size_fraction_float, wait_names[wait_method], auto_adjust_knobs, numa_aware_names[is_numa_aware], dto_dsa_cc);
 			for (int i = 0; i < num_wqs; i++)
-				LOG_TRACE("[%d] wq_path: %s, wq_size: %d, dsa_cap: %lx\n", i,
-					wqs[i].wq_path, wqs[i].wq_size, wqs[i].dsa_gencap);
+				LOG_TRACE("[%d] wq_path: %s, wq_size: %d, dsa_cap: %lx, block_on_fault: %d\n", i,
+					wqs[i].wq_path, wqs[i].wq_size, wqs[i].dsa_gencap,
+					wqs[i].block_on_fault);
 		}
 		dto_initialized = 1;
 
@@ -2285,10 +2296,11 @@ static __always_inline uint32_t async_desc_flags(const struct dto_wq *wq,
 {
 	uint32_t f = IDXD_OP_FLAG_CRAV | IDXD_OP_FLAG_RCR;
 
-	/* Passed as given: a work queue configured without block-on-fault
-	 * fails the descriptor with DSA_COMP_INVALID_FLAGS, and without the
-	 * flag a page fault stops the operation partway (see dto.h). */
-	if (flags & DTO_SUBMIT_BOF)
+	/* BOF only when the work queue allows it: a queue configured without
+	 * block-on-fault fails any descriptor carrying the flag with
+	 * DSA_COMP_INVALID_FLAGS, so there the flag is dropped and a page
+	 * fault stops the operation partway instead (see dto.h). */
+	if ((flags & DTO_SUBMIT_BOF) && wq->block_on_fault)
 		f |= IDXD_OP_FLAG_BOF;
 	/* CC is only legal for operations with a destination; CRC Generation
 	 * would be failed by the device with DSA_COMP_INVALID_FLAGS. */
@@ -2697,9 +2709,9 @@ int dto_batch_count(const dto_batch *b)
 }
 
 /* Descriptor for the next operation, or NULL (with *index unset) when the
- * batch is full or submitted or n is out of range. BOF is set as requested;
- * CC is recorded as requested and settled at submit, once the work queue
- * (and so the device's cache-control support) is known. */
+ * batch is full or submitted or n is out of range. BOF and CC are recorded as
+ * requested and settled at submit, once the work queue (and so its
+ * block-on-fault setting and the device's cache-control support) is known. */
 static struct dsa_hw_desc *batch_add(dto_batch *b, uint32_t opcode,
 				     const void *hint, size_t n,
 				     unsigned int flags, bool has_dest,
@@ -2869,6 +2881,8 @@ int dto_batch_submit(dto_batch *b)
 
 		if (!(wq->dsa_gencap & GENCAP_CC_MEMORY))
 			d->flags &= ~IDXD_OP_FLAG_CC;
+		if (!wq->block_on_fault)
+			d->flags &= ~IDXD_OP_FLAG_BOF;
 		b->comps[i].status = 0;
 	}
 	if (b->count == 1) {

dto.h

@@ -58,19 +58,19 @@
  *
  * DTO_SUBMIT_BOF makes the device block on a page fault until the kernel has
  * resolved it, so the operation completes even if its buffers are not
- * faulted in. It is passed to the device as given, and the work queue must
- * be configured with block-on-fault ("accel-config config-wq
- * --block-on-fault=1"):
+ * faulted in. The device only accepts it on a work queue configured with
+ * block-on-fault ("accel-config config-wq --block-on-fault=1"); on a queue
+ * without it the flag is dropped, so the submission is still accepted but a
+ * page fault stops the operation partway, as if the flag had not been set:
  *
  *   WQ block-on-fault  DTO_SUBMIT_BOF  page fault ->
  *   enabled            set             the device waits; the op completes
- *   disabled           set             (always) DTO_ASYNC_FAILED, status 0x11
- *                                      (invalid flags): the device rejects it
- *   either             not set         DTO_ASYNC_FAILED, status 0x03: partial
- *                                      completion, see above
+ *   disabled           set             DTO_ASYNC_FAILED, status 0x03: partial
+ *   either             not set         completion, see above
  *
- * The same applies to each operation of a batch, which dto_batch_poll then
- * redoes on the CPU.
+ * The flag is thus "block on faults when the work queue allows it", and the
+ * caller must handle a partial completion either way. The same applies to
+ * each operation of a batch, which dto_batch_poll then redoes on the CPU.
  *
  * CRC values: dto_async_crc_val() and dto_batch_crc() return the raw CRC32C
  * of the data -- the SSE4.2 crc32 instruction applied from 0, with no final

tests/test_functional.c

@@ -874,42 +874,39 @@ static int test_async_transl_fetch(void)
 	return 0;
 }
 
-/* DTO_SUBMIT_BOF is passed to the device as given (dto.h). Learn whether the
- * work queue has block-on-fault from a BOF submit on faulted-in memory, then
- * copy into a destination whose first pages are touched and the rest not:
+/* DTO_SUBMIT_BOF is honored only on a work queue with block-on-fault; on one
+ * without it DTO drops the flag rather than let the device reject the
+ * descriptor (dto.h). Copy into a destination whose first pages are touched
+ * and the rest not:
  *   BOF set, WQ with block-on-fault:    the device waits; the copy completes
- *   BOF set, WQ without block-on-fault: rejected, status 0x11 (invalid flags)
+ *   BOF set, WQ without block-on-fault: flag dropped: stops at the first
+ *                                       untouched page, status 0x03
  *   BOF not set, any WQ:                stops at the first untouched page,
  *                                       status 0x03, the touched part done
+ * Whether the work queue has block-on-fault is learned from that first BOF
+ * copy; a BOF submit is never rejected with 0x11 (invalid flags) any more.
  * Assumes every work queue DTO uses has the same block-on-fault setting. */
 static int test_async_bof(void)
 {
 	const size_t page = 4096, n = 16 * page, warm = 4 * page;
-	uint8_t *src = malloc(n), *hot = malloc(n);
+	uint8_t *src = malloc(n), *hot = malloc(n), *cold;
 	dto_batch *b = dto_batch_create(2);
 	dto_async_op op;
-	int rc, wq_bof;
+	int rc, wq_bof = -1;
 
 	ASSERT_TRUE(src && hot && b);
 	fill_pattern(src, n);
 	clear_buf(hot, n);
 
+	/* On faulted-in memory a BOF submit completes on any work queue. */
 	rc = dto_submit_memcpy(&op, hot, src, n, DTO_SUBMIT_BOF);
 	CHECK_SUBMIT(rc);
 	if (rc != DTO_ASYNC_SUBMITTED)
 		return 0;
-	rc = finish(&op);
-	if (rc == DTO_ASYNC_DONE) {
-		wq_bof = 1;
-		ASSERT_TRUE(verify_equal(hot, src, n));
-	} else {
-		ASSERT_EQ(rc, DTO_ASYNC_FAILED);
-		ASSERT_EQ(dto_async_status(&op), 0x11);
-		wq_bof = 0;
-	}
-	printf("    (work queue block-on-fault: %s)\n", wq_bof ? "enabled" : "disabled");
+	ASSERT_EQ(finish(&op), DTO_ASYNC_DONE);
+	ASSERT_TRUE(verify_equal(hot, src, n));
 
-	for (int bof = 0; bof < 2; bof++) {
+	for (int bof = 1; bof >= 0; bof--) {
 		uint8_t *dst = mmap(NULL, n, PROT_READ | PROT_WRITE,
 				    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 
@@ -919,13 +916,15 @@ static int test_async_bof(void)
 		CHECK_SUBMIT(rc);
 		if (rc == DTO_ASYNC_SUBMITTED) {
 			rc = finish(&op);
+			if (bof) {
+				wq_bof = rc == DTO_ASYNC_DONE;
+				printf("    (work queue block-on-fault: %s)\n",
+				       wq_bof ? "enabled" : "disabled");
+			}
 			if (bof && wq_bof) {
-				ASSERT_EQ(rc, DTO_ASYNC_DONE);
 				ASSERT_TRUE(verify_equal(dst, src, n));
-			} else if (bof) {
-				ASSERT_EQ(rc, DTO_ASYNC_FAILED);
-				ASSERT_EQ(dto_async_status(&op), 0x11);
 			} else {
+				/* no BOF in effect: partial, never 0x11 */
 				ASSERT_EQ(rc, DTO_ASYNC_FAILED);
 				ASSERT_EQ(dto_async_status(&op), 0x03);
 				ASSERT_EQ(dto_async_bytes_completed(&op), warm);
@@ -935,18 +934,26 @@ static int test_async_bof(void)
 		munmap(dst, n);
 	}
 
-	/* the same per batch operation; dto_batch_poll redoes the rejected ones */
+	/* The same per batch operation: a BOF member into untouched pages
+	 * completes on a block-on-fault queue and is otherwise stopped and
+	 * redone by dto_batch_poll; the member on hot memory always completes. */
+	cold = mmap(NULL, n / 2, PROT_READ | PROT_WRITE,
+		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	ASSERT_TRUE(cold != MAP_FAILED);
 	clear_buf(hot, n);
-	ASSERT_EQ(dto_batch_add_memmove(b, hot, src, n / 2, DTO_SUBMIT_BOF), 0);
-	ASSERT_EQ(dto_batch_add_memmove(b, hot + n / 2, src + n / 2, n / 2, 0), 1);
+	ASSERT_EQ(dto_batch_add_memmove(b, cold, src, n / 2, DTO_SUBMIT_BOF), 0);
+	ASSERT_EQ(dto_batch_add_memmove(b, hot, src + n / 2, n / 2, DTO_SUBMIT_BOF), 1);
 	rc = dto_batch_submit(b);
 	CHECK_SUBMIT(rc);
 	if (rc == DTO_ASYNC_SUBMITTED) {
 		ASSERT_EQ(ASYNC_WAIT(dto_batch_poll(b)), DTO_ASYNC_DONE);
-		ASSERT_EQ(dto_batch_status(b, 0), wq_bof ? 1 : 0x11);
+		if (wq_bof >= 0)
+			ASSERT_EQ(dto_batch_status(b, 0), wq_bof ? 1 : 0x03);
 		ASSERT_EQ(dto_batch_status(b, 1), 1);
-		ASSERT_TRUE(verify_equal(hot, src, n));
+		ASSERT_TRUE(verify_equal(cold, src, n / 2));
+		ASSERT_TRUE(verify_equal(hot, src + n / 2, n / 2));
 	}
+	munmap(cold, n / 2);
 	dto_batch_destroy(b);
 	free(src);
 	free(hot);