This patch series adds unit tests for the clk fixed rate basic type and
the clk registration functions that use struct clk_parent_data. To get
there, we add support for loading device tree overlays onto the live DTB
along with probing platform drivers to bind to device nodes in the
overlays. With this series, we're able to exercise some of the code in
the common clk framework that uses devicetree lookups to find parents
and the fixed rate clk code that scans device tree directly and creates
clks. Please review.
I Cced everyone to all the patches so they get the full context. I'm
hoping I can take the whole pile through the clk tree as they almost all
depend on each other.
Changes from v1 (https://lore.kernel.org/r/20230302013822.1808711-1-sboyd@kernel.org):
* Don't depend on UML, use unittest data approach to attach nodes
* Introduce overlay loading API for KUnit
* Move platform_device KUnit code to drivers/base/test
* Use #define macros for constants shared between unit tests and
overlays
* Settle on "test" as a vendor prefix
* Make KUnit wrappers have "_kunit" postfix
Stephen Boyd (11):
of: Load KUnit DTB from of_core_init()
of: Add test managed wrappers for of_overlay_apply()/of_node_put()
dt-bindings: vendor-prefixes: Add "test" vendor for KUnit and friends
dt-bindings: test: Add KUnit empty node binding
of: Add a KUnit test for overlays and test managed APIs
platform: Add test managed platform_device/driver APIs
dt-bindings: kunit: Add fixed rate clk consumer test
clk: Add test managed clk provider/consumer APIs
clk: Add KUnit tests for clk fixed rate basic type
dt-bindings: clk: Add KUnit clk_parent_data test
clk: Add KUnit tests for clks registered with struct clk_parent_data
.../clock/test,clk-kunit-parent-data.yaml | 47 ++
.../kunit/test,clk-kunit-fixed-rate.yaml | 35 ++
.../bindings/test/test,kunit-empty.yaml | 30 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/base/test/Makefile | 3 +
drivers/base/test/platform_kunit-test.c | 108 +++++
drivers/base/test/platform_kunit.c | 186 +++++++
drivers/clk/.kunitconfig | 3 +
drivers/clk/Kconfig | 7 +
drivers/clk/Makefile | 9 +-
drivers/clk/clk-fixed-rate_test.c | 299 ++++++++++++
drivers/clk/clk-fixed-rate_test.h | 8 +
drivers/clk/clk_kunit.c | 219 +++++++++
drivers/clk/clk_parent_data_test.h | 10 +
drivers/clk/clk_test.c | 459 +++++++++++++++++-
drivers/clk/kunit_clk_fixed_rate_test.dtso | 19 +
drivers/clk/kunit_clk_parent_data_test.dtso | 28 ++
drivers/of/.kunitconfig | 5 +
drivers/of/Kconfig | 23 +
drivers/of/Makefile | 7 +
drivers/of/base.c | 182 +++++++
drivers/of/kunit.dtso | 10 +
drivers/of/kunit_overlay_test.dtso | 9 +
drivers/of/of_kunit.c | 123 +++++
drivers/of/of_private.h | 6 +
drivers/of/of_test.c | 43 ++
drivers/of/overlay_test.c | 107 ++++
drivers/of/unittest.c | 101 +---
include/kunit/clk.h | 28 ++
include/kunit/of.h | 90 ++++
include/kunit/platform_device.h | 15 +
31 files changed, 2119 insertions(+), 102 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/test,clk-kunit-parent-data.yaml
create mode 100644 Documentation/devicetree/bindings/kunit/test,clk-kunit-fixed-rate.yaml
create mode 100644 Documentation/devicetree/bindings/test/test,kunit-empty.yaml
create mode 100644 drivers/base/test/platform_kunit-test.c
create mode 100644 drivers/base/test/platform_kunit.c
create mode 100644 drivers/clk/clk-fixed-rate_test.c
create mode 100644 drivers/clk/clk-fixed-rate_test.h
create mode 100644 drivers/clk/clk_kunit.c
create mode 100644 drivers/clk/clk_parent_data_test.h
create mode 100644 drivers/clk/kunit_clk_fixed_rate_test.dtso
create mode 100644 drivers/clk/kunit_clk_parent_data_test.dtso
create mode 100644 drivers/of/.kunitconfig
create mode 100644 drivers/of/kunit.dtso
create mode 100644 drivers/of/kunit_overlay_test.dtso
create mode 100644 drivers/of/of_kunit.c
create mode 100644 drivers/of/of_test.c
create mode 100644 drivers/of/overlay_test.c
create mode 100644 include/kunit/clk.h
create mode 100644 include/kunit/of.h
create mode 100644 include/kunit/platform_device.h
base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
--
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/https://git.kernel.org/pub/scm/linux/kernel/git/sboyd/spmi.git
Hi all,
I don't know if this is expected result, so I am filing the bug report.
Reports like this from tools/testing/selftests/net/tls:
# RUN tls.13_sm4_ccm.sendfile ...
# tls.c:323:sendfile:Expected ret (-1) == 0 (0)
# sendfile: Test terminated by assertion
# FAIL tls.13_sm4_ccm.sendfile
not ok 251 tls.13_sm4_ccm.sendfile
# RUN tls.13_sm4_ccm.send_then_sendfile ...
# tls.c:323:send_then_sendfile:Expected ret (-1) == 0 (0)
# send_then_sendfile: Test terminated by assertion
# FAIL tls.13_sm4_ccm.send_then_sendfile
not ok 252 tls.13_sm4_ccm.send_then_sendfile
# RUN tls.13_sm4_ccm.multi_chunk_sendfile ...
# tls.c:323:multi_chunk_sendfile:Expected ret (-1) == 0 (0)
# multi_chunk_sendfile: Test terminated by assertion
# FAIL tls.13_sm4_ccm.multi_chunk_sendfile
not ok 253 tls.13_sm4_ccm.multi_chunk_sendfile
Apparently, all are connected with sm4 hash ccm.
(Please find the complete report attached in tls-6.3-rc3-1.log)
The rest of the failed tests is as follows from this command:
[marvin@pc-mtodorov linux_torvalds]$ grep -v '^#' ../kselftest-6.3-rc3-1.log | grep "not ok"
not ok 2 selftests: alsa: pcm-test # TIMEOUT 45 seconds
not ok 1 selftests: drivers/net/bonding: bond-arp-interval-causes-panic.sh # exit=1
not ok 2 selftests: drivers/net/bonding: bond-break-lacpdu-tx.sh # exit=1
not ok 1 selftests: filesystems/binderfs: binderfs_test # exit=1
not ok 1 selftests: ftrace: ftracetest # exit=1
not ok 1 selftests: gpio: gpio-mockup.sh # exit=1
not ok 1 selftests: intel_pstate: run.sh # TIMEOUT 45 seconds
not ok 1 selftests: iommu: iommufd # exit=1
not ok 26 selftests: kvm: vmx_preemption_timer_test # exit=254
not ok 1 selftests: landlock: fs_test # exit=1
not ok 1 selftests: mincore: mincore_selftest # exit=1
not ok 2 selftests: mqueue: mq_perf_tests # TIMEOUT 45 seconds
not ok 1 selftests: nci: nci_dev # exit=1
not ok 6 selftests: net: tls # exit=1
not ok 12 selftests: net: run_netsocktests # exit=1
not ok 28 selftests: net: udpgro_bench.sh # exit=255
not ok 29 selftests: net: udpgro.sh # exit=255
not ok 36 selftests: net: fcnal-test.sh # TIMEOUT 1500 seconds
not ok 37 selftests: net: l2tp.sh # exit=2
not ok 45 selftests: net: icmp_redirect.sh # exit=1
not ok 49 selftests: net: txtimestamp.sh # exit=1
not ok 54 selftests: net: vrf_route_leaking.sh # exit=1
not ok 58 selftests: net: udpgro_fwd.sh # exit=1
not ok 59 selftests: net: udpgro_frglist.sh # exit=255
not ok 60 selftests: net: veth.sh # exit=1
not ok 67 selftests: net: srv6_end_dt46_l3vpn_test.sh # exit=1
not ok 68 selftests: net: srv6_end_dt4_l3vpn_test.sh # exit=1
not ok 82 selftests: net: rps_default_mask.sh # exit=1
not ok 85 selftests: net: test_ingress_egress_chaining.sh # exit=1
not ok 1 selftests: net/hsr: hsr_ping.sh # TIMEOUT 45 seconds
not ok 3 selftests: net/mptcp: mptcp_join.sh # exit=1
not ok 3 selftests: netfilter: nft_nat.sh # exit=1
not ok 5 selftests: netfilter: conntrack_icmp_related.sh # exit=1
not ok 8 selftests: netfilter: nft_concat_range.sh # exit=1
not ok 14 selftests: netfilter: conntrack_tcp_unreplied.sh # exit=1
not ok 15 selftests: netfilter: conntrack_vrf.sh # exit=1
not ok 15 selftests: proc: read # exit=134
not ok 1 selftests: pstore: pstore_tests # exit=1
not ok 3 selftests: ptrace: vmaccess # exit=1
not ok 1 selftests: rlimits: rlimits-per-userns # exit=1
not ok 1 selftests: sgx: test_sgx # exit=1
not ok 2 selftests: splice: short_splice_read.sh # exit=3
not ok 1 selftests: tdx: tdx_guest_test # exit=1
not ok 3 selftests: mm: split_huge_page_test # exit=1
not ok 5 selftests: mm: mdwe_test # exit=1
[marvin@pc-mtodorov linux_torvalds]$
The environment is AlmaLinux 8.7 running 6.3-rc3 vanilla kernel with
MGLRU, KMEMLEAK and CONFIG_DEBUG_KOBJECT=y enabled.
Hw := LENOVO_MT_10TX_BU_Lenovo_FM_V530S-07ICB
In case you are interested to debug this, I am available for additional
diagnostics.
As 45 bug reports might overwhelm me due to the overhead of bug submission,
I will probably submit a bug or two at a time.
Best regards,
Mirsad
--
Mirsad Goran Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia
This is the basic functionality for iommufd to support
iommufd_device_replace() and IOMMU_HWPT_ALLOC for physical devices.
iommufd_device_replace() allows changing the HWPT associated with the
device to a new IOAS or HWPT. Replace does this in way that failure leaves
things unchanged, and utilizes the iommu iommu_group_replace_domain() API
to allow the iommu driver to perform an optional non-disruptive change.
IOMMU_HWPT_ALLOC allows HWPTs to be explicitly allocated by the user and
used by attach or replace. At this point it isn't very useful since the
HWPT is the same as the automatically managed HWPT from the IOAS. However
a following series will allow userspace to customize the created HWPT.
The implementation is complicated because we have to introduce some
per-iommu_group memory in iommufd and redo how we think about multi-device
groups to be more explicit. This solves all the locking problems in the
prior attempts.
This series is infrastructure work for the following series which:
- Add replace for attach
- Expose replace through VFIO APIs
- Implement driver parameters for HWPT creation (nesting)
Once review of this is complete I will keep it on a side branch and
accumulate the following series when they are ready so we can have a
stable base and make more incremental progress. When we have all the parts
together to get a full implementation it can go to Linus.
I have this on github:
https://github.com/jgunthorpe/linux/commits/iommufd_hwpt
Jason Gunthorpe (12):
iommufd: Move isolated msi enforcement to iommufd_device_bind()
iommufd: Add iommufd_group
iommufd: Replace the hwpt->devices list with iommufd_group
iommufd: Use the iommufd_group to avoid duplicate reserved groups and
msi setup
iommufd: Make sw_msi_start a group global
iommufd: Move putting a hwpt to a helper function
iommufd: Add enforced_cache_coherency to iommufd_hw_pagetable_alloc()
iommufd: Add iommufd_device_replace()
iommufd: Make destroy_rwsem use a lock class per object type
iommufd: Add IOMMU_HWPT_ALLOC
iommufd/selftest: Return the real idev id from selftest mock_domain
iommufd/selftest: Add a selftest for IOMMU_HWPT_ALLOC
Nicolin Chen (2):
iommu: Introduce a new iommu_group_replace_domain() API
iommufd/selftest: Test iommufd_device_replace()
drivers/iommu/iommu-priv.h | 10 +
drivers/iommu/iommu.c | 30 ++
drivers/iommu/iommufd/device.c | 482 +++++++++++++-----
drivers/iommu/iommufd/hw_pagetable.c | 96 +++-
drivers/iommu/iommufd/io_pagetable.c | 5 +-
drivers/iommu/iommufd/iommufd_private.h | 44 +-
drivers/iommu/iommufd/iommufd_test.h | 7 +
drivers/iommu/iommufd/main.c | 17 +-
drivers/iommu/iommufd/selftest.c | 40 ++
include/linux/iommufd.h | 1 +
include/uapi/linux/iommufd.h | 26 +
tools/testing/selftests/iommu/iommufd.c | 64 ++-
.../selftests/iommu/iommufd_fail_nth.c | 57 ++-
tools/testing/selftests/iommu/iommufd_utils.h | 59 ++-
14 files changed, 758 insertions(+), 180 deletions(-)
create mode 100644 drivers/iommu/iommu-priv.h
base-commit: ac395958f9156733246b5bc3a481c6d38c321a7a
--
2.39.1
On Sat, 11 Mar 2023 at 07:21, Stephen Boyd <sboyd(a)kernel.org> wrote:
>
> Quoting David Gow (2023-03-02 23:15:35)
> > On Thu, 2 Mar 2023 at 09:38, Stephen Boyd <sboyd(a)kernel.org> wrote:
> > >
> > > Unit tests are more ergonomic and simpler to understand if they don't
> > > have to hoist a bunch of code into the test harness init and exit
> > > functions. Add some test managed wrappers for the clk APIs so that clk
> > > unit tests can write more code in the actual test and less code in the
> > > harness.
> > >
> > > Only add APIs that are used for now. More wrappers can be added in the
> > > future as necessary.
> > >
> > > Cc: Brendan Higgins <brendan.higgins(a)linux.dev>
> > > Cc: David Gow <davidgow(a)google.com>
> > > Signed-off-by: Stephen Boyd <sboyd(a)kernel.org>
> > > ---
> >
> > Looks good, modulo bikeshedding below.
>
> Cool!
>
> > >
> > > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > > index e3ca0d058a25..7efce649b0d3 100644
> > > --- a/drivers/clk/Makefile
> > > +++ b/drivers/clk/Makefile
> > > @@ -17,6 +17,11 @@ ifeq ($(CONFIG_OF), y)
> > > obj-$(CONFIG_COMMON_CLK) += clk-conf.o
> > > endif
> > >
> > > +# KUnit specific helpers
> > > +ifeq ($(CONFIG_COMMON_CLK), y)
> > > +obj-$(CONFIG_KUNIT) += clk-kunit.o
> >
> > Do we want to compile these in whenever KUnit is enabled, or only when
> > we're building clk tests specifically? I suspect this would be served
> > better by being under a CLK_KUNIT config option, which all of the
> > tests then depend on. (Whether that's the existing
> > CONFIG_CLK_KUNIT_TEST, and all of the clk tests live under the same
> > config option, or a separate parent option would be up to you).
>
> I was thinking of building it in with whatever mode CONFIG_KUNIT is
> built as. If this is a module because CONFIG_KUNIT=m, then unit tests
> would depend on that, and this would be a module as well. modprobe would
> know that some unit test module depends on symbols provided by
> clk-kunit.ko and thus load clk-kunit.ko first.
>
Personally, I'd rather have this behind CONFIG_CLK_KUNIT_TEST if
possible, if only to avoid needlessly building these if someone just
wants to test some other subsystem (but needs CONFIG_COMMON_CLK
enabled anyway). I doubt it'd be a problem in practice in this case,
but we definitely want to keep build (and hence iteration) times down
as much as possible, so it's probably good practice to keep all tests
behind at least some sort of "test this subsystem" option.
> >
> > Equally, this could be a bit interesting if CONFIG_KUNIT=m. Given
> > CONFIG_COMMON_CLK=y, this would end up as a clk-kunit module, no?
>
> Yes, that is the intent.
>
> >
> > > +endif
> > > +
> > > # hardware specific clock types
> > > # please keep this section sorted lexicographically by file path name
> > > obj-$(CONFIG_COMMON_CLK_APPLE_NCO) += clk-apple-nco.o
> > > diff --git a/drivers/clk/clk-kunit.c b/drivers/clk/clk-kunit.c
> > > new file mode 100644
> > > index 000000000000..78d85b3a7a4a
> > > --- /dev/null
> > > +++ b/drivers/clk/clk-kunit.c
> > > @@ -0,0 +1,204 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * KUnit helpers for clk tests
> > > + */
> > > +#include <linux/clk.h>
> > > +#include <linux/clk-provider.h>
> > > +#include <linux/err.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/slab.h>
> > > +
> > > +#include <kunit/resource.h>
> > > +
> > > +#include "clk-kunit.h"
> > > +
> > > +static void kunit_clk_disable_unprepare(struct kunit_resource *res)
> >
> > We need to decide on the naming scheme of these, and in particular if
> > they should be kunit_clk or clk_kunit (or something else).
> >
> > I'd lean to clk_kunit, if only to match DRM's KUnit helpers being
> > drm_kunit_helper better, and so that these are more tightly bound to
> > the subsystem being tested.
> > (i.e., so I don't have to scroll through every subsystem's helpers
> > when autocompleting kunit_).
>
> Ok, got it. I was trying to match kunit_kzalloc() style. It makes it
> easy to slap the 'kunit_' prefix on existing auto-completed function
> names like kzalloc() or clk_prepare_enable().
Yeah: my rule of thumb at the moment is to keep the kunit_ prefix for
things which are generic across the whole kernel (and tend to be
implemented in lib/kunit), and to use suffixes or infixes (whichever
works best) for things which are subsystem-specific.
> I wasn't aware of drm_kunit_helper. That's a mouthful! We don't call it
> slab_kunit_helper_kzalloc(). Maybe to satisfy all conditions it should
> be:
>
> clk_prepare_enable_kunit()
>
> so that kunit_ autocomplete doesn't have a big scroll list, and clk
> subsystem autocompletes, and we know it is kunit specific.
Sounds good to me.
Cheers,
-- David