Re: [PATCH v3 5/5] cxl/memdev: Don't overwrite the error from an earlier partition poison query

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Failure to advance the offset in Get Poison List loop causes redundant mailbox queries and missing poison records.
--

--- Patch [5]: [PATCH v3 5/5] cxl/memdev: Don't overwrite the error from an earlier partition poison query ---
commit d12b3a1ef61c4d80195127d0b11aa93c505ec62e
Author: Richard Cheng <icheng@nvidia.com>

cxl/memdev: Don't overwrite the error from an earlier partition poison query

This commit fixes an issue in cxl_get_poison_by_memdev() where errors from
querying poison lists for earlier partitions were overwritten by subsequent
partition successes. It adds a check to break the loop early on any error
that is not explicitly tolerated as a RAM -EFAULT.

> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 33a3d2e7b13af..8718964b9c5e8 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -231,6 +231,8 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
>  		 */
>  		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
>  			rc = 0;
> +		if (rc)
> +			break;

[Severity: High]
This isn't a bug introduced by this patch, but I notice an issue with the
Get Poison List loop inside cxl_mem_get_poison(), which is called just above
this block.

When the device returns CXL_POISON_FLAG_MORE, the do-while loop in
drivers/cxl/core/mbox.c:cxl_mem_get_poison() does not advance pi.offset:

	pi.offset = cpu_to_le64(offset);
	pi.length = cpu_to_le64(len / CXL_POISON_LEN_MULT);

	do {
		struct cxl_mbox_cmd mbox_cmd = (struct cxl_mbox_cmd){
			.opcode = CXL_MBOX_OP_GET_POISON,
			.size_in = sizeof(pi),
			.payload_in = &pi,
			[ ... ]
		};

		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
		if (rc)
			break;

		[ ... ]

	} while (po->flags & CXL_POISON_FLAG_MORE);

Does this cause redundant mailbox queries using the exact same offset?

If the device has more poison records than can fit in a single response,
would this result in fetching the same records repeatedly and missing the
subsequent ones?

>  	}
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708074228.43654-1-icheng@nvidia.com?part=5