Re: [PATCH 4/4] cxl/mbox: Add KUnit tests for cxl_internal_send_cmd()

[Alison Schofield <alison.schofield@intel.com>]

On Thu, Oct 15, 2026 at 05:37:26PM +0800, Li Ming wrote:
> 
> 在 2026/10/14 12:17, Alison Schofield 写道:
> > cxl_internal_send_cmd() validates mailbox payload sizes, translates device
> > return codes to errnos, and checks returned output sizes. Existing cxl_test
> > coverage does not exercise many of its error paths.
> > 
> > Add a KUnit suite that replaces mbox_send() with a stub transport, allowing
> > tests to control transport errors, device return codes, and output sizes.
> > 
> > The suite covers command forwarding, payload size limits, transport errors,
> > defined and unknown device return codes, background commands, and fixed
> > versus variable output sizes.
> > 
> > The unknown return code cases provide regression coverage for "cxl/mbox:
> > Treat unknown device return codes as errors". Without that change, the
> > tests fail and KASAN reports a global out-of-bounds read.
> > 
> > The suite uses the existing CONFIG_CXL_KUNIT_TEST option, allowing it to
> > run without additional configuration.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >   Documentation/driver-api/cxl/testing.rst |   3 +
> >   drivers/cxl/core/Makefile                |   1 +
> >   drivers/cxl/core/tests/Makefile          |   5 +
> >   drivers/cxl/core/tests/mbox_kunit.c      | 249 +++++++++++++++++++++++
> >   4 files changed, 258 insertions(+)
> >   create mode 100644 drivers/cxl/core/tests/Makefile
> >   create mode 100644 drivers/cxl/core/tests/mbox_kunit.c
> > 

snip


> > +static int cxl_mbox_test_init(struct kunit *test)
> > +{
> > +	struct mock_mbox *mock;
> > +
> > +	mock = kunit_kzalloc(test, sizeof(*mock), GFP_KERNEL);
> > +	KUNIT_ASSERT_NOT_NULL(test, mock);
> > +
> > +	mock->cxl_mbox.payload_size = TEST_PAYLOAD_SIZE;
> > +	mock->cxl_mbox.mbox_send = mock_mbox_send;

	mock->return_code = CXL_MBOX_CMD_RC_SUCCESS;


> > +	test->priv = mock;
> 
> How about adding a line "mock->return_code = CXL_MBOX_CMD_RC_SUCCESS" here
> to explicitly initialize mock->return_code? It makes easier for readers to
> see the default return_code is SUCCESS.
> 

Thanks and agree. Done like above in v2.


snip more below...
> > +
> > +static void send_unknown_return_code(struct kunit *test)
> > +{
> > +	static const u16 codes[] = { ARRAY_SIZE(cxl_mbox_cmd_rctable), 0x100,
> > +				     0xffff };
> > +	struct mock_mbox *mock = test->priv;
> > +
> > +	/* Codes the driver does not know are still device errors */
> > +	for (int i = 0; i < ARRAY_SIZE(codes); i++) {
> > +		struct cxl_mbox_cmd cmd = {};
> > +		int rc;
> > +
> > +		mock->return_code = codes[i];
> > +		rc = cxl_internal_send_cmd(&mock->cxl_mbox, &cmd);
> > +		KUNIT_EXPECT_EQ_MSG(test, rc, -ENXIO, "return_code=%#x",
> > +				    codes[i]);
> > +		KUNIT_EXPECT_STREQ_MSG(test, cxl_mbox_cmd_rc2str(&cmd),
> > +				       "unknown return code", "return_code=%#x",
> > +				       codes[i]);
> > +	}
> > +}
> Since we already have cxl_mbox_cmd_rctable here, should we also cover the
> existing return_codes that we know?


The known return codes are covered by send_device_error(). That iterates
thru the rc table and checks the expected errno for each entry. SUCCESS
and BACKGROUND are covered by their own tests.

The unknown return code test exercises the new out-of-range handling, so
I think we have the coverage we need here.

-- Alison