| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <48b192c7-4250-4e11-8dcd-b147dd8c0a28@arm.com>
Date: Fri, 14 Mar 2025 18:05:34 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: James Clark <james.clark@...aro.org>,
Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach
<mike.leach@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Leo Yan <leo.yan@....com>
Cc: linux-kernel@...r.kernel.org, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4] coresight: Add a KUnit test for
coresight_find_default_sink()
On 3/12/25 16:01, James Clark wrote:
> Add a test to confirm that default sink selection skips over an ETF
> and returns an ETR even if it's further away.
>
> This also makes it easier to add new unit tests in the future.
The test overall looks good but there are some comments below.
>
> Reviewed-by: Leo Yan <leo.yan@....com>
> Signed-off-by: James Clark <james.clark@...aro.org>
> ---
> Changes in v4:
> - Rename etm to src now that it's not CORESIGHT_DEV_SUBTYPE_SOURCE_PROC
> - Remove the now empty src_ops too
> - Fix a rebase mistake in the Makefile that removed CTCU
> - Link to v3: https://lore.kernel.org/r/20250312-james-cs-kunit-test-v3-1-dcfb69730161@linaro.org
>
> Changes in v3:
> - Use CORESIGHT_DEV_SUBTYPE_SOURCE_BUS type instead of the default
> (CORESIGHT_DEV_SUBTYPE_SOURCE_PROC) so that the test still works even
> when TRBE sinks are registered. This also removes the need for the
> fake CPU ID callback.
> - Link to v2: https://lore.kernel.org/r/20250305-james-cs-kunit-test-v2-1-83ba682b976c@linaro.org
>
> Changes in v2:
> - Let devm free everything rather than doing individual kfrees:
> "Like with managed drivers, KUnit-managed fake devices are
> automatically cleaned up when the test finishes, but can be manually
> cleaned up early with kunit_device_unregister()."
> - Link to v1: https://lore.kernel.org/r/20250225164639.522741-1-james.clark@linaro.org
> ---
> drivers/hwtracing/coresight/Kconfig | 9 +++
> drivers/hwtracing/coresight/Makefile | 1 +
> drivers/hwtracing/coresight/coresight-core.c | 1 +
> .../hwtracing/coresight/coresight-kunit-tests.c | 74 ++++++++++++++++++++++
> 4 files changed, 85 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index ecd7086a5b83..f064e3d172b3 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -259,4 +259,13 @@ config CORESIGHT_DUMMY
>
> To compile this driver as a module, choose M here: the module will be
> called coresight-dummy.
> +
> +config CORESIGHT_KUNIT_TESTS
> + tristate "Enable Coresight unit tests"
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
> + help
> + Enable Coresight unit tests. Only useful for development and not
> + intended for production.
The description sounds adequate given that more unit tests might be added later.
> +
> endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 8e62c3150aeb..4e6ea5b05b01 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -53,3 +53,4 @@ obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o
> obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o
> obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o
> coresight-ctcu-y := coresight-ctcu-core.o
> +obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index fb43ef6a3b1f..47af75ba7a00 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -959,6 +959,7 @@ coresight_find_default_sink(struct coresight_device *csdev)
> }
> return csdev->def_sink;
> }
> +EXPORT_SYMBOL_GPL(coresight_find_default_sink);
This symbol needs exporting given that the kunit test here can also be built
as a module.
>
> static int coresight_remove_sink_ref(struct device *dev, void *data)
> {
> diff --git a/drivers/hwtracing/coresight/coresight-kunit-tests.c b/drivers/hwtracing/coresight/coresight-kunit-tests.c
> new file mode 100644
> index 000000000000..c8f361767c45
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <kunit/test.h>
> +#include <kunit/device.h>
> +#include <linux/coresight.h>
> +
> +#include "coresight-priv.h"
> +
> +static struct coresight_device *coresight_test_device(struct device *dev)
> +{
> + struct coresight_device *csdev = devm_kcalloc(dev, 1,
> + sizeof(struct coresight_device),
> + GFP_KERNEL);
> + csdev->pdata = devm_kcalloc(dev, 1,
> + sizeof(struct coresight_platform_data),
> + GFP_KERNEL);
Guess both these memory allocations gets freed up here automatically
via kunit_device's device framework when the test suit exits ?
> + return csdev;
> +}
> +
> +static void test_default_sink(struct kunit *test)
> +{
> + /*
> + * Source -> ETF -> ETR -> CATU
> + * ^
> + * | default
> + */
Should some more random path combinations involving ETR and ETF along with
other coresight devices also be tested where ETR gets selected as default ?
> + struct device *dev = kunit_device_register(test, "coresight_kunit");
> + struct coresight_device *src = coresight_test_device(dev),
> + *etf = coresight_test_device(dev),
> + *etr = coresight_test_device(dev),
> + *catu = coresight_test_device(dev);
> + struct coresight_connection conn = {};
> +
> + src->type = CORESIGHT_DEV_TYPE_SOURCE;
> + /*
> + * Don't use CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, that would always return
> + * a TRBE sink if one is registered.
Probably this should be tested as well i.e when TRBE sink is also available
and it gets picked up as a default.
> + */
> + src->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_BUS;
> + etf->type = CORESIGHT_DEV_TYPE_LINKSINK;
> + etf->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
> + etr->type = CORESIGHT_DEV_TYPE_SINK;
> + etr->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;
> + catu->type = CORESIGHT_DEV_TYPE_HELPER;
> +
> + conn.src_dev = src;
> + conn.dest_dev = etf;
> + coresight_add_out_conn(dev, src->pdata, &conn);
> +
> + conn.src_dev = etf;
> + conn.dest_dev = etr;
> + coresight_add_out_conn(dev, etf->pdata, &conn);
> +
> + conn.src_dev = etr;
> + conn.dest_dev = catu;
> + coresight_add_out_conn(dev, etr->pdata, &conn);
> +
> + KUNIT_ASSERT_PTR_EQ(test, coresight_find_default_sink(src), etr);
> +}
> +
> +static struct kunit_case coresight_testcases[] = {
> + KUNIT_CASE(test_default_sink),
> + {}
> +};
> +
> +static struct kunit_suite coresight_test_suite = {
> + .name = "coresight_test_suite",
> + .test_cases = coresight_testcases,
> +};
> +
> +kunit_test_suites(&coresight_test_suite);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("James Clark <james.clark@...aro.org>");
> +MODULE_DESCRIPTION("Arm CoreSight KUnit tests");
>
> ---
> base-commit: 3eadce8308bc8d808fd9e3a9d211c84215087451
> change-id: 20250305-james-cs-kunit-test-3af1df2401e6
>
> Best regards,
Powered by blists - more mailing lists