Re: [PATCH v15 34/35] [RFC] kselftest/arm64: Add SCMI Telemetry UAPI compliance testcases

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Mon,  9 Nov 2026 09:12:20 +0000
Cristian Marussi <cristian.marussi@arm.com> wrote:

> From: Philip Radford <philip.radford@arm.com>
> 
> Add support to test SCMI Telemetry UAPI compliance testcases.

I'm thoroughly in favor of self tests, but rather less 1825 line patches!
I'd figure out an initial subset to propose of a few 100 lines then
build the rest up. Possibly just have a minimal set as part of first
series. 

I took a look at the code style for a few as not going to dig into whether
they are all functionally correct.
Given I'm assuming this LLM output, and those are pretty good at this
sort of test authoring I'm assuming they are probably ok.

Jonathan


> 
> Signed-off-by: Philip Radford <philip.radford@arm.com>
> Co-developed-by: Cristian Marussi <cristian.marussi@arm.com>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>

> diff --git a/tools/testing/selftests/arm64/scmi/config b/tools/testing/selftests/arm64/scmi/config
> new file mode 100644
> index 000000000000..63cd7e67ab55
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/scmi/config
> @@ -0,0 +1,2 @@
> +CONFIG_ARM_SCMI_SYSTEM_TELEMETRY=y
> +
> diff --git a/tools/testing/selftests/arm64/scmi/test_scmi_telemetry_uapi.c b/tools/testing/selftests/arm64/scmi/test_scmi_telemetry_uapi.c
> new file mode 100644
> index 000000000000..24a8170a4380
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/scmi/test_scmi_telemetry_uapi.c
...
I commented on style for a few near the end.
> +
> +TEST_F(stlm, tlm_get_uuid_list)
> +{
> +	struct scmi_tlm_batch batch = {};
> +	struct scmi_tlm_uuid *uuids;
> +	int i, j;
> +	bool nonzero;

No other reason for order? Go with reverse xmas tree.

> +
> +	if ((ksft_session.info.abi_features & SCMI_TLM_ABI_FEAT_UUID_LIST) == 0)
> +		SKIP(return, "UUID list not supported");
> +
> +	batch.num_items = 0;
> +	batch.item_sz = sizeof(*uuids);
> +
> +	ASSERT_EQ(0, ioctl(self->fd, SCMI_TLM_GET_UUID_LIST, &batch));
> +
> +	if (!batch.num_items)
> +		SKIP(return, "no UUIDs available");
> +
> +	uuids = malloc(sizeof(*uuids) * batch.num_items);
> +	ASSERT_NE(NULL, uuids);
> +	bzero(uuids, sizeof(*uuids) * batch.num_items);
> +
> +	batch.items = (__u64)uuids;
> +
> +	ASSERT_EQ(0, ioctl(self->fd, SCMI_TLM_GET_UUID_LIST, &batch));
> +	ASSERT_GT(batch.num_items, 0);
> +	ASSERT_EQ(sizeof(*uuids), batch.item_sz);
> +	ASSERT_EQ(0, batch.reserved);
> +
> +	for (i = 0; i < batch.num_items; i++) {
	for (int i = 0; ... 
> +		nonzero = false;
> +
> +		for (j = 0; j < SCMI_TLM_DE_IMPL_UUID_SZ; j++) {
		for (int j = 0; j < ...

> +			if (uuids[i].bytes[j]) {
> +				nonzero = true;
> +				break;
> +			}
> +		}
> +
> +		EXPECT_TRUE(nonzero);
> +
> +		TH_LOG("SCMI_TLM_GET_UUID_LIST - [%d] %02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x",
> +		       i, uuids[i].bytes[0], uuids[i].bytes[1],
> +		       uuids[i].bytes[2], uuids[i].bytes[3],
> +		       uuids[i].bytes[4], uuids[i].bytes[5],
> +		       uuids[i].bytes[6], uuids[i].bytes[7],
> +		       uuids[i].bytes[8], uuids[i].bytes[9],
> +		       uuids[i].bytes[10], uuids[i].bytes[11],
> +		       uuids[i].bytes[12], uuids[i].bytes[13],
> +		       uuids[i].bytes[14], uuids[i].bytes[15]);
> +	}
> +
> +	free(uuids);
> +}

> +TEST_F(stlm, tlm_get_uuid_list_bad_items_ptr)
> +{
> +	struct scmi_tlm_batch batch = {};
> +
> +	if ((ksft_session.info.abi_features & SCMI_TLM_ABI_FEAT_UUID_LIST) == 0)
> +		SKIP(return, "UUID list not supported");
> +
Where possible I'd assign structures with
	batch = (struct scmi_tlm_batch) {
		.num_items = 1,
		.items_sz = sizeof(struct scmi_tlm_uid),
		.items = 0, //Invalid pointer
	};
as that avoids having to zero explicitly and tends to end up more readable
than a wall of separate values being set in the same structure.
> +	batch.num_items = 1;
> +	batch.item_sz = sizeof(struct scmi_tlm_uuid);
> +	batch.items = 0;
> +
> +	ASSERT_EQ(-1, ioctl(self->fd, SCMI_TLM_GET_UUID_LIST, &batch));
> +	EXPECT_EQ(EFAULT, errno);
> +}