Re: [PATCH] kunit: Fix NULL-dereference in kunit_init_suite() if suite->log is NULL
On Mon, 18 Dec 2023 at 23:17, Richard Fitzgerald wrote: > > suite->log must be checked for NULL before passing it to > string_stream_clear(). This was done in kunit_init_test() but was missing > from kunit_init_suite(). > > Signed-off-by: Richard Fitzgerald > Fixes: 6d696c4695c5 ("kunit: add ability to run tests after boot using > debugfs") > --- Acked-by: David Gow Cheers, -- David > lib/kunit/test.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index e803d998e855..ea7f0913e55a 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -658,7 +658,9 @@ static void kunit_init_suite(struct kunit_suite *suite) > kunit_debugfs_create_suite(suite); > suite->status_comment[0] = '\0'; > suite->suite_init_err = 0; > - string_stream_clear(suite->log); > + > + if (suite->log) > + string_stream_clear(suite->log); > } > > bool kunit_enabled(void) > -- > 2.30.2 > smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v2 1/2] kunit: Allow passing function pointer to kunit_activate_static_stub()
On Thu, 21 Dec 2023 at 18:39, Richard Fitzgerald wrote: > > Swap the arguments to typecheck_fn() in kunit_activate_static_stub() > so that real_fn_addr can be either the function itself or a pointer > to that function. > > This is useful to simplify redirecting static functions in a module. > Having to pass the actual function meant that it must be exported > from the module. Either making the 'static' and EXPORT_SYMBOL*() > conditional (which makes the code messy), or change it to always > exported (which increases the export namespace and prevents the > compiler inlining a trivial stub function in non-test builds). > > With the original definition of kunit_activate_static_stub() the > address of real_fn_addr was passed to typecheck_fn() as the type to > be passed. This meant that if real_fn_addr was a pointer-to-function > it would resolve to a ** instead of a *, giving an error like this: > >error: initialization of ‘int (**)(int)’ from incompatible pointer >type ‘int (*)(int)’ [-Werror=incompatible-pointer-types] >kunit_activate_static_stub(test, add_one_fn_ptr, subtract_one); > | ^~~~ >./include/linux/typecheck.h:21:25: note: in definition of macro >‘typecheck_fn’ >21 | ({ typeof(type) __tmp = function; \ > > Swapping the arguments to typecheck_fn makes it take the type of a > pointer to the replacement function. Either a function or a pointer > to function can be assigned to that. For example: > > static int some_function(int x) > { > /* whatever */ > } > > int (* some_function_ptr)(int) = some_function; > > static int replacement(int x) > { > /* whatever */ > } > > Then: > kunit_activate_static_stub(test, some_function, replacement); > yields: > typecheck_fn(typeof(&replacement), some_function); > > and: > kunit_activate_static_stub(test, some_function_ptr, replacement); > yields: > typecheck_fn(typeof(&replacement), some_function_ptr); > > The two typecheck_fn() then resolve to: > > int (*__tmp)(int) = some_function; > and > int (*__tmp)(int) = some_function_ptr; > > Both of these are valid. In the first case the compiler inserts > an implicit '&' to take the address of the supplied function, and > in the second case the RHS is already a pointer to the same type. > > Signed-off-by: Richard Fitzgerald > Reviewed-by: Rae Moar > --- This makes sense to me. I was a little worried at first that this might actually be adding a layer of indirection (which, given we're keying on the pointer, would've needed to be done carefully), but this is an obvious fix to me. Reviewed-by: David Gow Cheers, -- David > No changes since V1. > --- > include/kunit/static_stub.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h > index 85315c80b303..bf940322dfc0 100644 > --- a/include/kunit/static_stub.h > +++ b/include/kunit/static_stub.h > @@ -93,7 +93,7 @@ void __kunit_activate_static_stub(struct kunit *test, > * The redirection can be disabled again with kunit_deactivate_static_stub(). > */ > #define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do > { \ > - typecheck_fn(typeof(&real_fn_addr), replacement_addr); > \ > + typecheck_fn(typeof(&replacement_addr), real_fn_addr); > \ > __kunit_activate_static_stub(test, real_fn_addr, replacement_addr); > \ > } while (0) > > -- > 2.30.2 > smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v2 2/2] kunit: Add example of kunit_activate_static_stub() with pointer-to-function
On Thu, 21 Dec 2023 at 18:39, Richard Fitzgerald wrote: > > Adds a variant of example_static_stub_test() that shows use of a > pointer-to-function with kunit_activate_static_stub(). > > A const pointer to the add_one() function is declared. This > pointer-to-function is passed to kunit_activate_static_stub() and > kunit_deactivate_static_stub() instead of passing add_one directly. > > Signed-off-by: Richard Fitzgerald > --- Reviewed-by: David Gow Thanks, -- David smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] kunit: Protect string comparisons against NULL
On Wed, 20 Dec 2023 at 23:52, Richard Fitzgerald wrote: > > Add NULL checks to KUNIT_BINARY_STR_ASSERTION() so that it will fail > cleanly if either pointer is NULL, instead of causing a NULL pointer > dereference in the strcmp(). > > A test failure could be that a string is unexpectedly NULL. This could > be trapped by KUNIT_ASSERT_NOT_NULL() but that would terminate the test > at that point. It's preferable that the KUNIT_EXPECT_STR*() macros can > handle NULL pointers as a failure. > > Signed-off-by: Richard Fitzgerald > --- I think this is the right thing to do. There's possibly an argument that this should succeed if both are NULL, but I prefer it this way. Reviewed-by: David Gow Cheers, -- David smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH 1/6] kunit: add parameter generation macro using description from array
On Wed, 20 Dec 2023 at 23:20, wrote: > > From: Benjamin Berg > > The existing KUNIT_ARRAY_PARAM macro requires a separate function to > get the description. However, in a lot of cases the description can > just be copied directly from the array. Add a second macro that > avoids having to write a static function just for a single strscpy. > > Signed-off-by: Benjamin Berg > --- I'm generally pretty happy with this, though note the checkpatch warning below. There was some discussion at plumbers about expanding the parameterised test APIs, so we may need to adjust the implementation of this down the line, but I don't think that'll happen for a while, so don't worry. With the warnings fixed, this is: Reviewed-by: David Gow I'm okay with this going in via the wireless tree if that's easier; certainly there are some conflicts with the later patches in this series and the kunit one. Cheers, -- David > Documentation/dev-tools/kunit/usage.rst | 12 > include/kunit/test.h| 19 +++ > 2 files changed, 23 insertions(+), 8 deletions(-) > > diff --git a/Documentation/dev-tools/kunit/usage.rst > b/Documentation/dev-tools/kunit/usage.rst > index c27e1646ecd9..b959e5befcbe 100644 > --- a/Documentation/dev-tools/kunit/usage.rst > +++ b/Documentation/dev-tools/kunit/usage.rst > @@ -566,13 +566,9 @@ By reusing the same ``cases`` array from above, we can > write the test as a > }, > }; > > - // Need a helper function to generate a name for each test case. > - static void case_to_desc(const struct sha1_test_case *t, char *desc) > - { > - strcpy(desc, t->str); > - } > - // Creates `sha1_gen_params()` to iterate over `cases`. > - KUNIT_ARRAY_PARAM(sha1, cases, case_to_desc); > + // Creates `sha1_gen_params()` to iterate over `cases` while using > + // the struct member `str` for the case description. > + KUNIT_ARRAY_PARAM_DESC(sha1, cases, str); > > // Looks no different from a normal test. > static void sha1_test(struct kunit *test) > @@ -588,7 +584,7 @@ By reusing the same ``cases`` array from above, we can > write the test as a > } > > // Instead of KUNIT_CASE, we use KUNIT_CASE_PARAM and pass in the > - // function declared by KUNIT_ARRAY_PARAM. > + // function declared by KUNIT_ARRAY_PARAM or KUNIT_ARRAY_PARAM_DESC. > static struct kunit_case sha1_test_cases[] = { > KUNIT_CASE_PARAM(sha1_test, sha1_gen_params), > {} > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 20ed9f9275c9..2dfa851e1f88 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -1514,6 +1514,25 @@ do { > \ > return NULL; > \ > } > > +/** > + * KUNIT_ARRAY_PARAM_DESC() - Define test parameter generator from an array. > + * @name: prefix for the test parameter generator function. > + * @array: array of test parameters. > + * @desc_member: structure member from array element to use as description > + * > + * Define function @name_gen_params which uses @array to generate parameters. > + */ > +#define KUNIT_ARRAY_PARAM_DESC(name, array, desc_member) > \ > + static const void *name##_gen_params(const void *prev, char *desc) > \ > + { > \ > + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + > 1 : (array); \ checkpatch is complaining here: ERROR: need consistent spacing around '*' (ctx:WxV) #71: FILE: include/kunit/test.h:1528: + typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \ > + if (__next - (array) < ARRAY_SIZE((array))) { > \ > + strscpy(desc, __next->desc_member, > KUNIT_PARAM_DESC_SIZE); \ > + return __next; > \ > + } > \ > + return NULL; > \ > + } > + > // TODO(dlaty...@google.com): consider eventually migrating users to > explicitly > // include resource.h themselves if they need it. > #include > -- > 2.43.0 > > -- > You received this message because you are subscribed to the Google Groups > "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to kunit-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/kunit-dev/2023122015
Re: [PATCH 2/6] kunit: add a convenience allocation wrapper for SKBs
On Wed, 20 Dec 2023 at 23:20, wrote: > > From: Benjamin Berg > > Add a simple convenience helper to allocate and zero fill an SKB for the > use by a kunit test. Also provide a way to free it again in case that > may be desirable. > > This simply mirrors the kunit_kmalloc API. > > Signed-off-by: Benjamin Berg > --- I'm happy with this as-is, but do think there's a discussion to be had about where subsystem-specific KUnit helpers should live. I think, because this is just a header (and it mirrors the normal linux/skbuff.h), that having it in include/kunit works well. If it needed a source file, I'm not 100% sure whether it should be in net/core/ or lib/kunit. Regardless, this looks good to me, modulo the small nitpick below. Reviewed-by: David Gow > include/kunit/skbuff.h | 56 ++ > 1 file changed, 56 insertions(+) > create mode 100644 include/kunit/skbuff.h > > diff --git a/include/kunit/skbuff.h b/include/kunit/skbuff.h > new file mode 100644 > index ..2144d01e556f > --- /dev/null > +++ b/include/kunit/skbuff.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Base unit test (KUnit) API. This probably needs a better description which mentions skbuff, and that it's for resource management. > + * > + * Copyright (C) 2023 Intel Corporation > + */ > + > +#ifndef _KUNIT_SKBUFF_H > +#define _KUNIT_SKBUFF_H > + > +#include > +#include > + > +static void kunit_action_kfree_skb(void *p) > +{ > + kfree_skb((struct sk_buff *)p); > +} > + > +/** > + * kunit_zalloc_skb() - Allocate and initialize a resource managed skb. > + * @test: The test case to which the skb belongs > + * @len: size to allocate > + * > + * Allocate a new struct sk_buff with GFP_KERNEL, zero fill the give length > + * and add it as a resource to the kunit test for automatic cleanup. > + * > + * Returns: newly allocated SKB, or %NULL on error > + */ > +static inline struct sk_buff *kunit_zalloc_skb(struct kunit *test, int len, > + gfp_t gfp) > +{ > + struct sk_buff *res = alloc_skb(len, GFP_KERNEL); > + > + if (!res || skb_pad(res, len)) > + return NULL; > + > + if (kunit_add_action_or_reset(test, kunit_action_kfree_skb, res)) > + return NULL; > + > + return res; > +} > + > +/** > + * kunit_kfree_skb() - Like kfree_skb except for allocations managed by > KUnit. > + * @test: The test case to which the resource belongs. > + * @skb: The SKB to free. > + */ > +static inline void kunit_kfree_skb(struct kunit *test, struct sk_buff *skb) > +{ > + if (!skb) > + return; > + > + kunit_release_action(test, kunit_action_kfree_skb, (void *)skb); > +} > + > +#endif /* _KUNIT_SKBUFF_H */ > -- > 2.43.0 > > -- > You received this message because you are subscribed to the Google Groups > "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to kunit-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/kunit-dev/20231220151952.415232-3-benjamin%40sipsolutions.net. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH 0/6] Add some more cfg80211 and mac80211 kunit tests
On Fri, 22 Dec 2023 at 05:47, Shuah Khan wrote: > > On 12/21/23 13:40, Johannes Berg wrote: > > On Thu, 2023-12-21 at 13:06 -0700, Shuah Khan wrote: > >> On 12/21/23 12:39, Johannes Berg wrote: > > This patchset adds a couple of helpers for kunit as well as tests for > cfg80211 and mac80211 that use them. > >>> > >>> I can take this through the wireless tree, but then I'd like to have > >>> ACKs from kunit folks for the kunit patches: > >>> > >> > >> We have run into conflicts in the past with the kunit tree. I take the > >> kunit patches through linux-kselftest tree. I do want to make sure there > >> are no conflicts. I don't mind taking these through my tree. > > > > OK, fair enough. > > > > If you can still put it into 6.8, then I think you can also take the > > wireless tests, assuming they pass (I haven't run them in the posted > > version). I don't think we'll have conflicts there, we don't have much > > work in wireless that's likely to land for 6.8. > > > > Sounds good. > > David, will you be able to look at these patches and let me know if > I can apply for Linux 6.8-rc1. The two initial KUnit patches look fine, modulo a couple of minor docs issues and checkpatch warnings. They apply cleanly, and I doubt there's much chance of there being a merge conflict for 6.8 -- there are no other changes to the parameterised test macros, and the skb stuff is in its own file. The remaining patches don't apply on top of the kunit branch as-is. I haven't had a chance to review them properly yet; the initial glance I had didn't show any serious issues (though I think checkpatch suggested some things to 'check'). So (once those small issues are finished), I'm okay with the first two patches going in via either tree. The remaining ones are probably best done via the wireless tree, as they seem to depend on some existing patches there, so maybe it makes sense to push everything via wireless. Cheers, -- David smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH 0/6] Add some more cfg80211 and mac80211 kunit tests
Hi, Thanks for taking a look! On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: > The two initial KUnit patches look fine, modulo a couple of minor docs > issues and checkpatch warnings. I can run checkpatch (even if I can't always take it seriously), but do you want to comment more specifically wrt. the docs? > They apply cleanly, and I doubt > there's much chance of there being a merge conflict for 6.8 -- there > are no other changes to the parameterised test macros, and the skb > stuff is in its own file. Right. > The remaining patches don't apply on top of the kunit branch as-is. Oh, OK. That makes some sense though, we've had a number of changes in the stack this cycle before. I somehow thought the tests were likely standalone, but apparently not. > I > haven't had a chance to review them properly yet; the initial glance I > had didn't show any serious issues (though I think checkpatch > suggested some things to 'check'). I can check. > So (once those small issues are finished), I'm okay with the first two > patches going in via either tree. The remaining ones are probably best > done via the wireless tree, as they seem to depend on some existing > patches there, so maybe it makes sense to push everything via > wireless. If not through wireless I doubt we'll get it synchronized for 6.8, though of course it's also not needed for 6.8 to have the extra unit tests :) I'll let Shuah decide. Thanks! johannes
Re: [PATCH v2] selftests: uevent: use shared makefile library
On Thu, Dec 21, 2023 at 02:44:52PM -0700, Shuah Khan wrote: > On 12/21/23 13:49, Antonio Terceiro wrote: > > This makes the uevent selftests build not write to the source tree > > unconditionally, as that breaks out of tree builds when the source tree > > is read-only. It also avoids leaving a git repository in a dirty state > > after a build. > > > > Why can't you do that using make O= directive. That's what I meant by out of tree builds. When using O=, the uevent selftests build still writes to the source directory. Maybe my wording in the commit message is not clear enough, I will try to improve it. > > v2: drop spurious extra SPDX-License-Identifier > > > > Signed-off-by: Antonio Terceiro > > --- > > tools/testing/selftests/uevent/Makefile | 15 +++ > > 1 file changed, 3 insertions(+), 12 deletions(-) > > > > diff --git a/tools/testing/selftests/uevent/Makefile > > b/tools/testing/selftests/uevent/Makefile > > index f7baa9aa2932..872969f42694 100644 > > --- a/tools/testing/selftests/uevent/Makefile > > +++ b/tools/testing/selftests/uevent/Makefile > > @@ -1,17 +1,8 @@ > > # SPDX-License-Identifier: GPL-2.0 > > all: > > -include ../lib.mk > > - > > -.PHONY: all clean > > - > > -BINARIES := uevent_filtering > > -CFLAGS += -Wl,-no-as-needed -Wall > > +CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES) > > -uevent_filtering: uevent_filtering.c ../kselftest.h ../kselftest_harness.h > > - $(CC) $(CFLAGS) $< -o $@ > > +TEST_GEN_PROGS = uevent_filtering > > -TEST_PROGS += $(BINARIES) > > -EXTRA_CLEAN := $(BINARIES) > > - > > -all: $(BINARIES) > > +include ../lib.mk > > This change doesn't get the intended result of not writing to > source tree. Binaries will still be written to the source > tree unless O= is specified. It does in my tests. Maybe I am missing something. mainline without the patch: 8<8<8<- $ make -s defconfig O=/tmp/output && make -s kselftest-all TARGETS=uevent O=/tmp/output make[4]: Entrando no diretório '/home/terceiro/src/linaro/linux/tools/testing/selftests/uevent' make[4]: Nada a ser feito para 'all'. make[4]: Saindo do diretório '/home/terceiro/src/linaro/linux/tools/testing/selftests/uevent' $ git status --ignored On branch master Your branch is up to date with 'origin/master'. Untracked files: (use "git add ..." to include in what will be committed) tools/testing/selftests/uevent/uevent_filtering nothing added to commit but untracked files present (use "git add" to track) $ git clean -dxf Removing tools/testing/selftests/uevent/uevent_filtering 8<8<8<- mainline with the patch: 8<8<8<- $ git branch -m kselftest-uvent kselftest-uvent-o $ rm -rf /tmp/output/ $ make -s defconfig O=/tmp/output && make -s kselftest-all TARGETS=uevent O=/tmp/output make[4]: Entrando no diretório '/home/terceiro/src/linaro/linux/tools/testing/selftests/uevent' gcc -Wl,-no-as-needed -Wall -isystem /tmp/output/usr/include uevent_filtering.c -o /tmp/output/kselftest/uevent/uevent_filtering make[4]: Saindo do diretório '/home/terceiro/src/linaro/linux/tools/testing/selftests/uevent' $ git status --ignored On branch kselftest-uvent-o nothing to commit, working tree clean $ git clean -dxf $ 8<8<8<- signature.asc Description: PGP signature
Re: [PATCH v7 9/9] iommu/vt-d: Add iotlb flush for nested domain
> On Dec 22, 2023, at 15:12, Tian, Kevin wrote: > > >> >> From: Liu, Yi L >> Sent: Friday, December 22, 2023 3:02 PM >> >> On Dec 22, 2023, at 14:47, Tian, Kevin wrote: >>> >>> From: Yang, Weijiang Sent: Friday, December 22, 2023 11:56 AM > + > +xa_for_each(&domain->iommu_array, i, info) { > +nested_flush_pasid_iotlb(info->iommu, domain, addr, npages, 0); > + > +if (domain->has_iotlb_device) > +continue; Shouldn't this be if (!domain->has_iotlb_device)? >>> >>> yes that is wrong. >>> >>> actually it's weird to put domain check in a loop of domain->iommu_array. >>> >>> that check along with devtlb flush should be done out of that loop. >> >> Maybe adding a bool, set it out of the loop, check the bool in the loop. > > the point is that dev iotlb doesn't rely on info->iommu: > >nested_flush_dev_iotlb(domain, addr, mask, &fault); > > then why do it in the loop of info->iommu? yes. It should have another device loop instead.
Re: [PATCH net-next v2] net: ctnetlink: support filtering by zone
On Mon, Nov 27, 2023 at 11:49:16AM +, Felix Huettner wrote: > conntrack zones are heavily used by tools like openvswitch to run > multiple virtual "routers" on a single machine. In this context each > conntrack zone matches to a single router, thereby preventing > overlapping IPs from becoming issues. > In these systems it is common to operate on all conntrack entries of a > given zone, e.g. to delete them when a router is deleted. Previously this > required these tools to dump the full conntrack table and filter out the > relevant entries in userspace potentially causing performance issues. > > To do this we reuse the existing CTA_ZONE attribute. This was previous > parsed but not used during dump and flush requests. Now if CTA_ZONE is > set we filter these operations based on the provided zone. > However this means that users that previously passed CTA_ZONE will > experience a difference in functionality. > > Alternatively CTA_FILTER could have been used for the same > functionality. However it is not yet supported during flush requests and > is only available when using AF_INET or AF_INET6. For the record, this is applied to nf-next.
Re: [PATCH] selftests/livepatch: fix and refactor new dmesg message code
On Wed, 2023-12-20 at 10:11 -0500, Joe Lawrence wrote: > The livepatching kselftests rely on comparing expected vs. observed > dmesg output. After each test, new dmesg entries are determined by > the > 'comm' utility comparing a saved, pre-test copy of dmesg to post-test > dmesg output. > > Alexander reports that the 'comm --nocheck-order -13' invocation used > by > the tests can be confused when dmesg entry timestamps vary in > magnitude > (ie, "[ 98.820331]" vs. "[ 100.031067]"), in which case, > additional > messages are reported as new. The unexpected entries then spoil the > test results. > > Instead of relying on 'comm' or 'diff' to determine new testing dmesg > entries, refactor the code: > > - pre-test : log a unique canary dmesg entry > - test : run tests, log messages > - post-test : filter dmesg starting from pre-test message > > Reported-by: Alexander Gordeev > Closes: > https://lore.kernel.org/live-patching/zyaimypyhxva9...@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com/ > Signed-off-by: Joe Lawrence I liked the solution. As I don't speak awk I had to do some manual testing to understand that the syntax you used "prints everything after the $last_dmesg_msg message on dmesg". Either way, it's better then using 'comm' utility. I tested on my x86_64 VM, and the tests passed as expected. LGTM, so Tested-by: Marcos Paulo de Souza Reviewed-by: Marcos Paulo de Souza > --- > .../testing/selftests/livepatch/functions.sh | 37 + > -- > 1 file changed, 17 insertions(+), 20 deletions(-) > > diff --git a/tools/testing/selftests/livepatch/functions.sh > b/tools/testing/selftests/livepatch/functions.sh > index c8416c54b463..b1fd7362c2fe 100644 > --- a/tools/testing/selftests/livepatch/functions.sh > +++ b/tools/testing/selftests/livepatch/functions.sh > @@ -42,17 +42,6 @@ function die() { > exit 1 > } > > -# save existing dmesg so we can detect new content > -function save_dmesg() { > - SAVED_DMESG=$(mktemp --tmpdir -t klp-dmesg-XX) > - dmesg > "$SAVED_DMESG" > -} > - > -# cleanup temporary dmesg file from save_dmesg() > -function cleanup_dmesg_file() { > - rm -f "$SAVED_DMESG" > -} > - > function push_config() { > DYNAMIC_DEBUG=$(grep '^kernel/livepatch' > /sys/kernel/debug/dynamic_debug/control | \ > awk -F'[: ]' '{print "file " $1 " line " $2 > " " $4}') > @@ -99,7 +88,6 @@ function set_ftrace_enabled() { > > function cleanup() { > pop_config > - cleanup_dmesg_file > } > > # setup_config - save the current config and set a script exit trap > that > @@ -280,7 +268,15 @@ function set_pre_patch_ret { > function start_test { > local test="$1" > > - save_dmesg > + # Dump something unique into the dmesg log, then stash the > entry > + # in LAST_DMESG. The check_result() function will use it to > + # find new kernel messages since the test started. > + local last_dmesg_msg="livepatch kselftest timestamp: $(date > --rfc-3339=ns)" > + log "$last_dmesg_msg" > + loop_until 'dmesg | grep -q "$last_dmesg_msg"' || > + die "buffer busy? can't find canary dmesg message: > $last_dmesg_msg" > + LAST_DMESG=$(dmesg | grep "$last_dmesg_msg") > + > echo -n "TEST: $test ... " > log "= TEST: $test =" > } > @@ -291,23 +287,24 @@ function check_result { > local expect="$*" > local result > > - # Note: when comparing dmesg output, the kernel log > timestamps > - # help differentiate repeated testing runs. Remove them > with a > - # post-comparison sed filter. > - > - result=$(dmesg | comm --nocheck-order -13 "$SAVED_DMESG" - | > \ > + # Test results include any new dmesg entry since LAST_DMESG, > then: > + # - include lines matching keywords > + # - exclude lines matching keywords > + # - filter out dmesg timestamp prefixes > + result=$(dmesg | awk -v last_dmesg="$LAST_DMESG" 'p; $0 == > last_dmesg { p=1 }' | \ > grep -e 'livepatch:' -e 'test_klp' | \ > grep -v '\(tainting\|taints\) kernel' | \ > sed 's/^\[[ 0-9.]*\] //') > > if [[ "$expect" == "$result" ]] ; then > echo "ok" > + elif [[ "$result" == "" ]] ; then > + echo -e "not ok\n\nbuffer overrun? can't find canary > dmesg entry: $LAST_DMESG\n" > + die "livepatch kselftest(s) failed" > else > echo -e "not ok\n\n$(diff -upr --label expected -- > label result <(echo "$expect") <(echo "$result"))\n" > die "livepatch kselftest(s) failed" > fi > - > - cleanup_dmesg_file > } > > # check_sysfs_rights(modname, rel_path, expected_rights) - check > sysfs
[PATCH net-next 0/4] mptcp: add CurrEstab MIB counter
This MIB counter is similar to the one of TCP -- CurrEstab -- available in /proc/net/snmp. This is useful to quickly list the number of MPTCP connections without having to iterate over all of them. Patch 1 prepares its support by adding new helper functions: - MPTCP_DEC_STATS(): similar to MPTCP_INC_STATS(), but this time to decrement a counter. - mptcp_set_state(): similar to tcp_set_state(), to change the state of an MPTCP socket, and to inc/decrement the new counter when needed. Patch 2 uses mptcp_set_state() instead of directly calling inet_sk_state_store() to change the state of MPTCP sockets. Patch 3 and 4 validate the new feature in MPTCP "join" and "diag" selftests. Signed-off-by: Matthieu Baerts --- Geliang Tang (4): mptcp: add CurrEstab MIB counter support mptcp: use mptcp_set_state selftests: mptcp: join: check CURRESTAB counters selftests: mptcp: diag: check CURRESTAB counters net/mptcp/mib.c | 1 + net/mptcp/mib.h | 8 net/mptcp/pm_netlink.c | 5 +++ net/mptcp/protocol.c| 56 - net/mptcp/protocol.h| 1 + net/mptcp/subflow.c | 2 +- tools/testing/selftests/net/mptcp/diag.sh | 17 +++- tools/testing/selftests/net/mptcp/mptcp_join.sh | 46 +--- 8 files changed, 110 insertions(+), 26 deletions(-) --- base-commit: 56794e5358542b7c652f202946e53bfd2373b5e0 change-id: 20231221-upstream-net-next-20231221-mptcp-currestab-5a2867b4020b Best regards, -- Matthieu Baerts
[PATCH net-next 1/4] mptcp: add CurrEstab MIB counter support
From: Geliang Tang Add a new MIB counter named MPTCP_MIB_CURRESTAB to count current established MPTCP connections, similar to TCP_MIB_CURRESTAB. This is useful to quickly list the number of MPTCP connections without having to iterate over all of them. This patch adds a new helper function mptcp_set_state(): if the state switches from or to ESTABLISHED state, this newly added counter is incremented. This helper is going to be used in the following patch. Similar to MPTCP_INC_STATS(), a new helper called MPTCP_DEC_STATS() is also needed to decrement a MIB counter. Signed-off-by: Geliang Tang Acked-by: Paolo Abeni Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts --- net/mptcp/mib.c | 1 + net/mptcp/mib.h | 8 net/mptcp/protocol.c | 18 ++ net/mptcp/protocol.h | 1 + 4 files changed, 28 insertions(+) diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c index a0990c365a2e..c30405e76833 100644 --- a/net/mptcp/mib.c +++ b/net/mptcp/mib.c @@ -66,6 +66,7 @@ static const struct snmp_mib mptcp_snmp_list[] = { SNMP_MIB_ITEM("RcvWndShared", MPTCP_MIB_RCVWNDSHARED), SNMP_MIB_ITEM("RcvWndConflictUpdate", MPTCP_MIB_RCVWNDCONFLICTUPDATE), SNMP_MIB_ITEM("RcvWndConflict", MPTCP_MIB_RCVWNDCONFLICT), + SNMP_MIB_ITEM("MPCurrEstab", MPTCP_MIB_CURRESTAB), SNMP_MIB_SENTINEL }; diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h index cae71d947252..dd7fd1f246b5 100644 --- a/net/mptcp/mib.h +++ b/net/mptcp/mib.h @@ -65,6 +65,7 @@ enum linux_mptcp_mib_field { * conflict with another subflow while updating msk rcv wnd */ MPTCP_MIB_RCVWNDCONFLICT, /* Conflict with while updating msk rcv wnd */ + MPTCP_MIB_CURRESTAB,/* Current established MPTCP connections */ __MPTCP_MIB_MAX }; @@ -95,4 +96,11 @@ static inline void __MPTCP_INC_STATS(struct net *net, __SNMP_INC_STATS(net->mib.mptcp_statistics, field); } +static inline void MPTCP_DEC_STATS(struct net *net, + enum linux_mptcp_mib_field field) +{ + if (likely(net->mib.mptcp_statistics)) + SNMP_DEC_STATS(net->mib.mptcp_statistics, field); +} + bool mptcp_mib_alloc(struct net *net); diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 5cd5c3f535a8..b555bd0b425b 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -2885,6 +2885,24 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how) release_sock(ssk); } +void mptcp_set_state(struct sock *sk, int state) +{ + int oldstate = sk->sk_state; + + switch (state) { + case TCP_ESTABLISHED: + if (oldstate != TCP_ESTABLISHED) + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_CURRESTAB); + break; + + default: + if (oldstate == TCP_ESTABLISHED) + MPTCP_DEC_STATS(sock_net(sk), MPTCP_MIB_CURRESTAB); + } + + inet_sk_state_store(sk, state); +} + static const unsigned char new_state[16] = { /* current state: new state: action: */ [0 /* (Invalid) */] = TCP_CLOSE, diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 1240268f9e9e..3517f2d24a22 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -641,6 +641,7 @@ bool __mptcp_close(struct sock *sk, long timeout); void mptcp_cancel_work(struct sock *sk); void __mptcp_unaccepted_force_close(struct sock *sk); void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk); +void mptcp_set_state(struct sock *sk, int state); bool mptcp_addresses_equal(const struct mptcp_addr_info *a, const struct mptcp_addr_info *b, bool use_port); -- 2.43.0
[PATCH net-next 2/4] mptcp: use mptcp_set_state
From: Geliang Tang This patch replaces all the 'inet_sk_state_store()' calls under net/mptcp with the new helper mptcp_set_state(). Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/460 Signed-off-by: Geliang Tang Acked-by: Paolo Abeni Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts --- net/mptcp/pm_netlink.c | 5 + net/mptcp/protocol.c | 38 +++--- net/mptcp/subflow.c| 2 +- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c index bf4d96f6f99a..661c226dad18 100644 --- a/net/mptcp/pm_netlink.c +++ b/net/mptcp/pm_netlink.c @@ -1048,6 +1048,11 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk, if (err) return err; + /* We don't use mptcp_set_state() here because it needs to be called +* under the msk socket lock. For the moment, that will not bring +* anything more than only calling inet_sk_state_store(), because the +* old status is known (TCP_CLOSE). +*/ inet_sk_state_store(newsk, TCP_LISTEN); lock_sock(ssk); err = __inet_listen_sk(ssk, backlog); diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index b555bd0b425b..b43762e64dc5 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -443,11 +443,11 @@ static void mptcp_check_data_fin_ack(struct sock *sk) switch (sk->sk_state) { case TCP_FIN_WAIT1: - inet_sk_state_store(sk, TCP_FIN_WAIT2); + mptcp_set_state(sk, TCP_FIN_WAIT2); break; case TCP_CLOSING: case TCP_LAST_ACK: - inet_sk_state_store(sk, TCP_CLOSE); + mptcp_set_state(sk, TCP_CLOSE); break; } @@ -608,13 +608,13 @@ static bool mptcp_check_data_fin(struct sock *sk) switch (sk->sk_state) { case TCP_ESTABLISHED: - inet_sk_state_store(sk, TCP_CLOSE_WAIT); + mptcp_set_state(sk, TCP_CLOSE_WAIT); break; case TCP_FIN_WAIT1: - inet_sk_state_store(sk, TCP_CLOSING); + mptcp_set_state(sk, TCP_CLOSING); break; case TCP_FIN_WAIT2: - inet_sk_state_store(sk, TCP_CLOSE); + mptcp_set_state(sk, TCP_CLOSE); break; default: /* Other states not expected */ @@ -789,7 +789,7 @@ static bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk) */ ssk_state = inet_sk_state_load(ssk); if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD)) - inet_sk_state_store(sk, ssk_state); + mptcp_set_state(sk, ssk_state); WRITE_ONCE(sk->sk_err, -err); /* This barrier is coupled with smp_rmb() in mptcp_poll() */ @@ -2477,7 +2477,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk, inet_sk_state_load(msk->first) == TCP_CLOSE) { if (sk->sk_state != TCP_ESTABLISHED || msk->in_accept_queue || sock_flag(sk, SOCK_DEAD)) { - inet_sk_state_store(sk, TCP_CLOSE); + mptcp_set_state(sk, TCP_CLOSE); mptcp_close_wake_up(sk); } else { mptcp_start_tout_timer(sk); @@ -2572,7 +2572,7 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk) WRITE_ONCE(sk->sk_err, ECONNRESET); } - inet_sk_state_store(sk, TCP_CLOSE); + mptcp_set_state(sk, TCP_CLOSE); WRITE_ONCE(sk->sk_shutdown, SHUTDOWN_MASK); smp_mb__before_atomic(); /* SHUTDOWN must be visible first */ set_bit(MPTCP_WORK_CLOSE_SUBFLOW, &msk->flags); @@ -2707,7 +2707,7 @@ static void mptcp_do_fastclose(struct sock *sk) struct mptcp_subflow_context *subflow, *tmp; struct mptcp_sock *msk = mptcp_sk(sk); - inet_sk_state_store(sk, TCP_CLOSE); + mptcp_set_state(sk, TCP_CLOSE); mptcp_for_each_subflow_safe(msk, subflow, tmp) __mptcp_close_ssk(sk, mptcp_subflow_tcp_sock(subflow), subflow, MPTCP_CF_FASTCLOSE); @@ -2925,7 +2925,7 @@ static int mptcp_close_state(struct sock *sk) int next = (int)new_state[sk->sk_state]; int ns = next & TCP_STATE_MASK; - inet_sk_state_store(sk, ns); + mptcp_set_state(sk, ns); return next & TCP_ACTION_FIN; } @@ -3036,7 +3036,7 @@ bool __mptcp_close(struct sock *sk, long timeout) if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) { mptcp_check_listen_stop(sk); - inet_sk_state_store(sk, TCP_CLOSE); + mptcp_set_state(sk, TC
[PATCH net-next 3/4] selftests: mptcp: join: check CURRESTAB counters
From: Geliang Tang This patch adds a new helper chk_cestab_nr() to check the current established connections counter MIB_CURRESTAB. Set the newly added variables cestab_ns1 and cestab_ns2 to indicate how many connections are expected in ns1 or ns2. Invoke check_cestab() to check the counter during the connection in do_transfer() and invoke chk_cestab_nr() to re-check it when the connection closed. These checks are embedded in add_tests(). Signed-off-by: Geliang Tang Acked-by: Paolo Abeni Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 46 ++--- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 87590a43b50d..3a5b63026191 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -56,6 +56,8 @@ unset FAILING_LINKS unset test_linkfail unset addr_nr_ns1 unset addr_nr_ns2 +unset cestab_ns1 +unset cestab_ns2 unset sflags unset fastclose unset fullmesh @@ -976,6 +978,34 @@ pm_nl_set_endpoint() fi } +chk_cestab_nr() +{ + local ns=$1 + local cestab=$2 + local count + + print_check "cestab $cestab" + count=$(mptcp_lib_get_counter ${ns} "MPTcpExtMPCurrEstab") + if [ -z "$count" ]; then + print_skip + elif [ "$count" != "$cestab" ]; then + fail_test "got $count current establish[s] expected $cestab" + else + print_ok + fi +} + +# $1 namespace 1, $2 namespace 2 +check_cestab() +{ + if [ -n "${cestab_ns1}" ]; then + chk_cestab_nr ${1} ${cestab_ns1} + fi + if [ -n "${cestab_ns2}" ]; then + chk_cestab_nr ${2} ${cestab_ns2} + fi +} + do_transfer() { local listener_ns="$1" @@ -1089,6 +1119,7 @@ do_transfer() local cpid=$! pm_nl_set_endpoint $listener_ns $connector_ns $connect_addr + check_cestab $listener_ns $connector_ns wait $cpid local retc=$? @@ -2477,47 +2508,52 @@ add_tests() if reset "add single subflow"; then pm_nl_set_limits $ns1 0 1 pm_nl_set_limits $ns2 0 1 - addr_nr_ns2=1 speed=slow \ + addr_nr_ns2=1 speed=slow cestab_ns2=1 \ run_tests $ns1 $ns2 10.0.1.1 chk_join_nr 1 1 1 + chk_cestab_nr $ns2 0 fi # add signal address if reset "add signal address"; then pm_nl_set_limits $ns1 0 1 pm_nl_set_limits $ns2 1 1 - addr_nr_ns1=1 speed=slow \ + addr_nr_ns1=1 speed=slow cestab_ns1=1 \ run_tests $ns1 $ns2 10.0.1.1 chk_join_nr 1 1 1 chk_add_nr 1 1 + chk_cestab_nr $ns1 0 fi # add multiple subflows if reset "add multiple subflows"; then pm_nl_set_limits $ns1 0 2 pm_nl_set_limits $ns2 0 2 - addr_nr_ns2=2 speed=slow \ + addr_nr_ns2=2 speed=slow cestab_ns2=1 \ run_tests $ns1 $ns2 10.0.1.1 chk_join_nr 2 2 2 + chk_cestab_nr $ns2 0 fi # add multiple subflows IPv6 if reset "add multiple subflows IPv6"; then pm_nl_set_limits $ns1 0 2 pm_nl_set_limits $ns2 0 2 - addr_nr_ns2=2 speed=slow \ + addr_nr_ns2=2 speed=slow cestab_ns2=1 \ run_tests $ns1 $ns2 dead:beef:1::1 chk_join_nr 2 2 2 + chk_cestab_nr $ns2 0 fi # add multiple addresses IPv6 if reset "add multiple addresses IPv6"; then pm_nl_set_limits $ns1 0 2 pm_nl_set_limits $ns2 2 2 - addr_nr_ns1=2 speed=slow \ + addr_nr_ns1=2 speed=slow cestab_ns1=1 \ run_tests $ns1 $ns2 dead:beef:1::1 chk_join_nr 2 2 2 chk_add_nr 2 2 + chk_cestab_nr $ns1 0 fi } -- 2.43.0
[PATCH net-next 4/4] selftests: mptcp: diag: check CURRESTAB counters
From: Geliang Tang This patch adds a new helper chk_msk_cestab() to check the current established connections counter MIB_CURRESTAB in diag.sh. Invoke it to check the counter during the connection after every chk_msk_inuse(). Signed-off-by: Geliang Tang Reviewed-by: Matthieu Baerts Signed-off-by: Matthieu Baerts --- tools/testing/selftests/net/mptcp/diag.sh | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh index 95b498efacd1..04fcb8a077c9 100755 --- a/tools/testing/selftests/net/mptcp/diag.sh +++ b/tools/testing/selftests/net/mptcp/diag.sh @@ -56,7 +56,7 @@ __chk_nr() local command="$1" local expected=$2 local msg="$3" - local skip="${4:-SKIP}" + local skip="${4-SKIP}" local nr nr=$(eval $command) @@ -182,6 +182,15 @@ chk_msk_inuse() __chk_nr get_msk_inuse $expected "$msg" 0 } +# $1: cestab nr +chk_msk_cestab() +{ + local cestab=$1 + + __chk_nr "mptcp_lib_get_counter ${ns} MPTcpExtMPCurrEstab" \ +"${cestab}" "chk ${cestab} cestab" "" +} + wait_connected() { local listener_ns="${1}" @@ -219,9 +228,11 @@ chk_msk_nr 2 "after MPC handshake " chk_msk_remote_key_nr 2 "chk remote_key" chk_msk_fallback_nr 0 "chk no fallback" chk_msk_inuse 2 "chk 2 msk in use" +chk_msk_cestab 2 flush_pids chk_msk_inuse 0 "chk 0 msk in use after flush" +chk_msk_cestab 0 echo "a" | \ timeout ${timeout_test} \ @@ -237,9 +248,11 @@ echo "b" | \ wait_connected $ns 10001 chk_msk_fallback_nr 1 "check fallback" chk_msk_inuse 1 "chk 1 msk in use" +chk_msk_cestab 1 flush_pids chk_msk_inuse 0 "chk 0 msk in use after flush" +chk_msk_cestab 0 NR_CLIENTS=100 for I in `seq 1 $NR_CLIENTS`; do @@ -261,9 +274,11 @@ done wait_msk_nr $((NR_CLIENTS*2)) "many msk socket present" chk_msk_inuse $((NR_CLIENTS*2)) "chk many msk in use" +chk_msk_cestab $((NR_CLIENTS*2)) flush_pids chk_msk_inuse 0 "chk 0 msk in use after flush" +chk_msk_cestab 0 mptcp_lib_result_print_all_tap exit $ret -- 2.43.0
Re: [PATCH 0/6] Add some more cfg80211 and mac80211 kunit tests
On Fri, 22 Dec 2023 at 18:09, Johannes Berg wrote: > > Hi, > > Thanks for taking a look! > > On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: > > The two initial KUnit patches look fine, modulo a couple of minor docs > > issues and checkpatch warnings. > > I can run checkpatch (even if I can't always take it seriously), but do > you want to comment more specifically wrt. the docs? > Sorry, the 'docs' issue was just the initial comment on the include/linux/skbuff.h file in patch 2, which could have been more specific to skbuff and resource management. The actual kerneldoc comments seem fine to me. > > They apply cleanly, and I doubt > > there's much chance of there being a merge conflict for 6.8 -- there > > are no other changes to the parameterised test macros, and the skb > > stuff is in its own file. > > Right. > > > The remaining patches don't apply on top of the kunit branch as-is. > > Oh, OK. That makes some sense though, we've had a number of changes in > the stack this cycle before. I somehow thought the tests were likely > standalone, but apparently not. > I managed to get this to apply locally. The only real changes are to net/mac80211/ieee80211_i.h so it may be possible to port this across to the kselftest/kunit branch if you want, but it doesn't apply cleanly as-is. Also, there are a couple of cfg80211 failures: --- KTAP version 1 1..1 KTAP version 1 # Subtest: cfg80211-inform-bss # module: cfg80211_tests 1..2 platform regulatory.0: Direct firmware load for regulatory.db failed with error -2 cfg80211: failed to load regulatory.db ok 1 test_inform_bss_ssid_only KTAP version 1 # Subtest: test_inform_bss_ml_sta # test_inform_bss_ml_sta: EXPECTATION FAILED at net/wireless/tests/scan.c:592 Expected ies->len == 6 + 2 + sizeof(rnr) + 2 + 155 + mle_basic_common_info.var_len + 5, but ies->len == 185 (0xb9) 6 + 2 + sizeof(rnr) + 2 + 155 + mle_basic_common_info.var_len + 5 == 203 (0xcb) not ok 1 no_mld_id # test_inform_bss_ml_sta: EXPECTATION FAILED at net/wireless/tests/scan.c:588 Expected ies->len == 6 + 2 + sizeof(rnr) + 2 + 160 + 2 + 165 + mle_basic_common_info.var_len + 5, but ies->len == 357 (0x165) 6 + 2 + sizeof(rnr) + 2 + 160 + 2 + 165 + mle_basic_common_info.var_len + 5 == 376 (0x178) not ok 2 mld_id_eq_1 # test_inform_bss_ml_sta: pass:0 fail:2 skip:0 total:2 not ok 2 test_inform_bss_ml_sta # cfg80211-inform-bss: pass:1 fail:1 skip:0 total:2 # Totals: pass:1 fail:2 skip:0 total:3 not ok 1 cfg80211-inform-bss --- If the failures are because of the missing 'regulatory.db' file, would it make more sense to have that SKIP the tests instead? (And, if you actually want to check that it loads correctly, have that be its own, separate test?) > > I > > haven't had a chance to review them properly yet; the initial glance I > > had didn't show any serious issues (though I think checkpatch > > suggested some things to 'check'). > > I can check. Yeah, it mostly looks like really minor style 'suggestions' around indenting and putting blank lines in, along with a couple of "you're reusing a value in a macro, double check this" ones.. I'll paste them below (but warning, they're a bit verbose). CHECK: Please use a blank line after function/struct/union/enum declarations #1142: FILE: net/wireless/tests/scan.c:225: +}; +KUNIT_ARRAY_PARAM_DESC(gen_new_ie, gen_new_ie_cases, desc) CHECK: Please use a blank line after function/struct/union/enum declarations #1330: FILE: net/wireless/tests/scan.c:413: +}; +KUNIT_ARRAY_PARAM_DESC(inform_bss_ml_sta, inform_bss_ml_sta_cases, desc) CHECK: Alignment should match open parenthesis #1489: FILE: net/wireless/tests/scan.c:572: + KUNIT_EXPECT_EQ(test, link_bss->beacon_interval, + le16_to_cpu(sta_prof.beacon_int)); CHECK: Alignment should match open parenthesis #1491: FILE: net/wireless/tests/scan.c:574: + KUNIT_EXPECT_EQ(test, link_bss->capability, + le16_to_cpu(sta_prof.capabilities)); CHECK: Macro argument reuse '_freq' - possible side-effects? #1620: FILE: net/wireless/tests/util.h:10: +#define CHAN2G(_freq) { \ + .band = NL80211_BAND_2GHZ, \ + .center_freq = (_freq), \ + .hw_value = (_freq), \ +} CHECK: Macro argument reuse 'test' - possible side-effects? #1653: FILE: net/wireless/tests/util.h:43: +#define T_WIPHY(test, ctx) ({ \ + struct wiphy *__wiphy = \ + kunit_alloc_resource(test, t_wiphy_init,\ +t_wiphy_exit, \ +GFP_KERNEL, &(ctx)); \ + \ + KUNIT_ASSERT_NOT_NULL(test, __wiphy); \ + __wiphy;\ +
Re: [PATCH 1/6] kunit: add parameter generation macro using description from array
On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: > On Wed, 20 Dec 2023 at 23:20, wrote: > > > > From: Benjamin Berg > > > > The existing KUNIT_ARRAY_PARAM macro requires a separate function > > to > > get the description. However, in a lot of cases the description can > > just be copied directly from the array. Add a second macro that > > avoids having to write a static function just for a single strscpy. > > > > Signed-off-by: Benjamin Berg > > --- > > I'm generally pretty happy with this, though note the checkpatch > warning below. > > There was some discussion at plumbers about expanding the > parameterised test APIs, so we may need to adjust the implementation > of this down the line, but I don't think that'll happen for a while, > so don't worry. > > With the warnings fixed, this is: I think the checkpatch warning is a false positive. It seems to confuse the * as a multiplication due to typeof() looking like a function call rather than a variable declaration. Benjamin > Reviewed-by: David Gow > > I'm okay with this going in via the wireless tree if that's easier; > certainly there are some conflicts with the later patches in this > series and the kunit one. > > Cheers, > -- David > > > Documentation/dev-tools/kunit/usage.rst | 12 > > include/kunit/test.h | 19 +++ > > 2 files changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/Documentation/dev-tools/kunit/usage.rst > > b/Documentation/dev-tools/kunit/usage.rst > > index c27e1646ecd9..b959e5befcbe 100644 > > --- a/Documentation/dev-tools/kunit/usage.rst > > +++ b/Documentation/dev-tools/kunit/usage.rst > > @@ -566,13 +566,9 @@ By reusing the same ``cases`` array from > > above, we can write the test as a > > }, > > }; > > > > - // Need a helper function to generate a name for each test > > case. > > - static void case_to_desc(const struct sha1_test_case *t, > > char *desc) > > - { > > - strcpy(desc, t->str); > > - } > > - // Creates `sha1_gen_params()` to iterate over `cases`. > > - KUNIT_ARRAY_PARAM(sha1, cases, case_to_desc); > > + // Creates `sha1_gen_params()` to iterate over `cases` > > while using > > + // the struct member `str` for the case description. > > + KUNIT_ARRAY_PARAM_DESC(sha1, cases, str); > > > > // Looks no different from a normal test. > > static void sha1_test(struct kunit *test) > > @@ -588,7 +584,7 @@ By reusing the same ``cases`` array from above, > > we can write the test as a > > } > > > > // Instead of KUNIT_CASE, we use KUNIT_CASE_PARAM and pass > > in the > > - // function declared by KUNIT_ARRAY_PARAM. > > + // function declared by KUNIT_ARRAY_PARAM or > > KUNIT_ARRAY_PARAM_DESC. > > static struct kunit_case sha1_test_cases[] = { > > KUNIT_CASE_PARAM(sha1_test, sha1_gen_params), > > {} > > diff --git a/include/kunit/test.h b/include/kunit/test.h > > index 20ed9f9275c9..2dfa851e1f88 100644 > > --- a/include/kunit/test.h > > +++ b/include/kunit/test.h > > @@ -1514,6 +1514,25 @@ do > > { > > \ > > return > > NULL; > > \ > > } > > > > +/** > > + * KUNIT_ARRAY_PARAM_DESC() - Define test parameter generator from > > an array. > > + * @name: prefix for the test parameter generator function. > > + * @array: array of test parameters. > > + * @desc_member: structure member from array element to use as > > description > > + * > > + * Define function @name_gen_params which uses @array to generate > > parameters. > > + */ > > +#define KUNIT_ARRAY_PARAM_DESC(name, array, > > desc_member) \ > > + static const void *name##_gen_params(const void *prev, char > > *desc) \ > > + > > { > > \ > > + typeof((array)[0]) *__next = prev ? > > ((typeof(__next)) prev) + 1 : (array); \ > > checkpatch is complaining here: > ERROR: need consistent spacing around '*' (ctx:WxV) > #71: FILE: include/kunit/test.h:1528: > > + typeof((array)[0]) *__next = prev ? ((typeof(__next)) > prev) + 1 : (array); \ > > > + if (__next - (array) < ARRAY_SIZE((array))) > > { \ > > + strscpy(desc, __next->desc_member, > > KUNIT_PARAM_DESC_SIZE); \ > > + return > > __next; \ > > + > > } > > \ > > + return > > NULL; > > \ > >
Re: [PATCH 0/6] Add some more cfg80211 and mac80211 kunit tests
On 12/22/23 03:09, Johannes Berg wrote: Hi, Thanks for taking a look! On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: The two initial KUnit patches look fine, modulo a couple of minor docs issues and checkpatch warnings. I can run checkpatch (even if I can't always take it seriously), but do you want to comment more specifically wrt. the docs? They apply cleanly, and I doubt there's much chance of there being a merge conflict for 6.8 -- there are no other changes to the parameterised test macros, and the skb stuff is in its own file. Right. The remaining patches don't apply on top of the kunit branch as-is. Oh, OK. That makes some sense though, we've had a number of changes in the stack this cycle before. I somehow thought the tests were likely standalone, but apparently not. I haven't had a chance to review them properly yet; the initial glance I had didn't show any serious issues (though I think checkpatch suggested some things to 'check'). I can check. So (once those small issues are finished), I'm okay with the first two patches going in via either tree. The remaining ones are probably best done via the wireless tree, as they seem to depend on some existing patches there, so maybe it makes sense to push everything via wireless. If not through wireless I doubt we'll get it synchronized for 6.8, though of course it's also not needed for 6.8 to have the extra unit tests :) I'll let Shuah decide. Thank you David for the reviews. johannes, Please take these through wireless - makes it easier for all of us. Acked-by: Shuah Khan thanks, -- Shuah
Re: [PATCH v3] tracing/selftests: Add ownership modification tests for eventfs
On 12/21/23 19:16, Steven Rostedt wrote: Shuah, This patch has no dependencies. You can take it through your tree for the next merge window if you want. If not, I can take it. Tried to apply this and seeing a couple of issues: -- missing SPDX -- this file has executable permission set unlike the existing .tc files in the same directory ERROR: do not set execute permissions for source files #72: FILE: tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #72: new file mode 100755 WARNING: Missing or malformed SPDX-License-Identifier tag in line 2 #78: FILE: tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc:2: +# description: Test file and directory owership changes for eventfs total: 1 errors, 2 warnings, 113 lines checked thanks, -- Shuah
[PATCH RFC v2 00/22] KVM: arm64: Implement support for SME in non-protected guests
This series implements support for SME use in non-protected KVM guests. Much of this is very similar to SVE, the main additional challenge that SME presents is that it introduces two new controls which change the registers seen by guests: - PSTATE.ZA enables the ZA matrix register and, if SME2 is supported, the ZT0 LUT register. - PSTATE.SM enables streaming mode, a new floating point mode which uses the SVE register set with a separately configured vector length. In streaming mode implementation of the FFR register is optional. It is also permitted to build systems which support SME without SVE, in this case when not in streaming mode no SVE registers or instructions are available. Further, there is no requirement that there be any overlap in the set of vector lengths supported by SVE and SME in a system, this is expected to be a common situation in practical systems. Since there is a new vector length to configure we introduce a new feature parallel to the existing SVE one with a new pseudo register for the streaming mode vector length. Due to the overlap with SVE caused by streaming mode rather than finalising SME as a separate feature we use the existing SVE finalisation to also finalise SME, a new define KVM_ARM_VCPU_VEC is provided to help make user code clearer. Finalising SVE and SME separately would introduce complication with register access since finalising SVE makes the SVE regsiters writeable by userspace and doing multiple finalisations results in an error being reported. Dealing with a state where the SVE registers are writeable due to one of SVE or SME being finalised but may have their VL changed by the other being finalised seems like needless complexity with minimal practical utility, it seems clearer to just express directly that only one finalisation can be done in the ABI. We represent the streaming mode registers to userspace by always using the existing SVE registers to access the floating point state, using the larger of the SME and (if enabled for the guest) SVE vector lengths. There are a large number of subfeatures for SME, most of which only offer additional instructions but some of which (SME2 and FA64) add architectural state. The expectation is that these will be configured via the ID registers but since the mechanism for doing this is still unclear the current code enables SME2 and FA64 for the guest if the host supports them regardless of what the ID registers say. Since we do not yet have support for SVE in protected guests and SME is very reliant on SVE this series does not implement support for SME in protected guests. This will be added separately once SVE support is merged into mainline (or along with merging that), there is code for protected guests using SVE in the Android tree. The new KVM_ARM_VCPU_VEC feature and ZA and ZT0 registers have not been added to the get-reg-list selftest, the idea of supporting additional features there without restructuring the program to generate all possible feature combinations has been rejected. I will post a separate series which does that restructuring. I am seeing some test failures currently which I've not got to the bottom of, at this point I'm reasonably sure these are preexisting issues in the kernel which are more apparent in a guest. To: Marc Zyngier To: Oliver Upton To: James Morse To: Suzuki K Poulose To: Catalin Marinas To: Will Deacon Cc: Cc: Cc: To: Paolo Bonzini To: Jonathan Corbet Cc: Cc: To: Shuah Khan Cc: Signed-off-by: Mark Brown Changes in v2: - Rebase onto v6.7-rc3. - Configure subfeatures based on host system only. - Complete nVHE support. - There was some snafu with sending v1 out, it didn't make it to the lists but in case it hit people's inboxes I'm sending as v2. --- Mark Brown (22): KVM: arm64: Document why we trap SVE access from the host arm64/fpsimd: Make SVE<->FPSIMD rewriting available to KVM KVM: arm64: Move SVE state access macros after feature test macros KVM: arm64: Store vector lengths in an array KVM: arm64: Document the KVM ABI for SME KVM: arm64: Make FFR restore optional in __sve_restore_state() KVM: arm64: Define guest flags for SME KVM: arm64: Rename SVE finalization constants to be more general KVM: arm64: Basic SME system register descriptions KVM: arm64: Add support for TPIDR2_EL0 KVM: arm64: Make SMPRI_EL1 RES0 for SME guests KVM: arm64: Make SVCR a normal system register KVM: arm64: Context switch SME state for guest KVM: arm64: Manage and handle SME traps KVM: arm64: Implement SME vector length configuration KVM: arm64: Rename sve_state_reg_region KVM: arm64: Support userspace access to streaming mode SVE registers KVM: arm64: Expose ZA to userspace KVM: arm64: Provide userspace access to ZT0 KVM: arm64: Support SME version configuration via ID registers KVM: arm64: Provide userspace ABI for enabling SME
[PATCH RFC v2 01/22] KVM: arm64: Document why we trap SVE access from the host
When we exit from a SVE guest we leave the SVE configuration in EL2 as it was for the guest, only switching back to the host configuration on next use by the host. This is perhaps a little surprising, add comments explaining what is going on both in the trap handler and when we configure the traps. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_emulate.h | 1 + arch/arm64/kvm/hyp/nvhe/hyp-main.c | 1 + 2 files changed, 2 insertions(+) diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 78a550537b67..14d6ff2e2a39 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -598,6 +598,7 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) } else { val = CPTR_NVHE_EL2_RES1; + /* Leave traps enabled, we will restore EL2 lazily */ if (vcpu_has_sve(vcpu) && (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)) val |= CPTR_EL2_TZ; diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index 2385fd03ed87..84deed83e580 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -420,6 +420,7 @@ void handle_trap(struct kvm_cpu_context *host_ctxt) handle_host_smc(host_ctxt); break; case ESR_ELx_EC_SVE: + /* Handle lazy restore of the host VL */ if (has_hvhe()) sysreg_clear_set(cpacr_el1, 0, (CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN)); -- 2.30.2
[PATCH RFC v2 02/22] arm64/fpsimd: Make SVE<->FPSIMD rewriting available to KVM
We have routines to rewrite between FPSIMD and SVE data formats, make these visible to the KVM host code so that we can use them to present the FP register state in SVE format for guests which have SME without SVE. Signed-off-by: Mark Brown --- arch/arm64/include/asm/fpsimd.h | 5 + arch/arm64/kernel/fpsimd.c | 23 +++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h index 50e5f25d3024..7092e7f944ae 100644 --- a/arch/arm64/include/asm/fpsimd.h +++ b/arch/arm64/include/asm/fpsimd.h @@ -157,6 +157,11 @@ extern void cpu_enable_fa64(const struct arm64_cpu_capabilities *__unused); extern u64 read_smcr_features(void); +void __fpsimd_to_sve(void *sst, struct user_fpsimd_state const *fst, +unsigned int vq); +void __sve_to_fpsimd(struct user_fpsimd_state *fst, void const *sst, +unsigned int vq); + /* * Helpers to translate bit indices in sve_vq_map to VQ values (and * vice versa). This allows find_next_bit() to be used to find the diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index 1559c706d32d..e6a4dd68f62a 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -647,8 +647,8 @@ static __uint128_t arm64_cpu_to_le128(__uint128_t x) #define arm64_le128_to_cpu(x) arm64_cpu_to_le128(x) -static void __fpsimd_to_sve(void *sst, struct user_fpsimd_state const *fst, - unsigned int vq) +void __fpsimd_to_sve(void *sst, struct user_fpsimd_state const *fst, +unsigned int vq) { unsigned int i; __uint128_t *p; @@ -659,6 +659,18 @@ static void __fpsimd_to_sve(void *sst, struct user_fpsimd_state const *fst, } } +void __sve_to_fpsimd(struct user_fpsimd_state *fst, const void *sst, +unsigned int vq) +{ + unsigned int i; + __uint128_t const *p; + + for (i = 0; i < SVE_NUM_ZREGS; ++i) { + p = (__uint128_t const *)ZREG(sst, vq, i); + fst->vregs[i] = arm64_le128_to_cpu(*p); + } +} + /* * Transfer the FPSIMD state in task->thread.uw.fpsimd_state to * task->thread.sve_state. @@ -700,18 +712,13 @@ static void sve_to_fpsimd(struct task_struct *task) unsigned int vq, vl; void const *sst = task->thread.sve_state; struct user_fpsimd_state *fst = &task->thread.uw.fpsimd_state; - unsigned int i; - __uint128_t const *p; if (!system_supports_sve() && !system_supports_sme()) return; vl = thread_get_cur_vl(&task->thread); vq = sve_vq_from_vl(vl); - for (i = 0; i < SVE_NUM_ZREGS; ++i) { - p = (__uint128_t const *)ZREG(sst, vq, i); - fst->vregs[i] = arm64_le128_to_cpu(*p); - } + __sve_to_fpsimd(fst, sst, vq); } #ifdef CONFIG_ARM64_SVE -- 2.30.2
[PATCH RFC v2 03/22] KVM: arm64: Move SVE state access macros after feature test macros
In preparation for SME support move the macros used to access SVE state after the feature test macros, we will need to test for SME subfeatures to determine the size of the SME state. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 40 +++ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 824f29f04916..9180713a2f9b 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -772,26 +772,6 @@ struct kvm_vcpu_arch { #define IN_WFI __vcpu_single_flag(sflags, BIT(7)) -/* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */ -#define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) + \ -sve_ffr_offset((vcpu)->arch.sve_max_vl)) - -#define vcpu_sve_max_vq(vcpu) sve_vq_from_vl((vcpu)->arch.sve_max_vl) - -#define vcpu_sve_state_size(vcpu) ({ \ - size_t __size_ret; \ - unsigned int __vcpu_vq; \ - \ - if (WARN_ON(!sve_vl_valid((vcpu)->arch.sve_max_vl))) { \ - __size_ret = 0; \ - } else {\ - __vcpu_vq = vcpu_sve_max_vq(vcpu); \ - __size_ret = SVE_SIG_REGS_SIZE(__vcpu_vq); \ - } \ - \ - __size_ret; \ -}) - #define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \ KVM_GUESTDBG_USE_SW_BP | \ KVM_GUESTDBG_USE_HW | \ @@ -820,6 +800,26 @@ struct kvm_vcpu_arch { #define vcpu_gp_regs(v)(&(v)->arch.ctxt.regs) +/* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */ +#define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) + \ +sve_ffr_offset((vcpu)->arch.sve_max_vl)) + +#define vcpu_sve_max_vq(vcpu) sve_vq_from_vl((vcpu)->arch.sve_max_vl) + +#define vcpu_sve_state_size(vcpu) ({ \ + size_t __size_ret; \ + unsigned int __vcpu_vq; \ + \ + if (WARN_ON(!sve_vl_valid((vcpu)->arch.sve_max_vl))) { \ + __size_ret = 0; \ + } else {\ + __vcpu_vq = vcpu_sve_max_vq(vcpu); \ + __size_ret = SVE_SIG_REGS_SIZE(__vcpu_vq); \ + } \ + \ + __size_ret; \ +}) + /* * Only use __vcpu_sys_reg/ctxt_sys_reg if you know you want the * memory backed version of a register, and not the one most recently -- 2.30.2
[PATCH RFC v2 04/22] KVM: arm64: Store vector lengths in an array
SME introduces a second vector length enumerated and configured in the same manner as for SVE. In a similar manner to the host kernel refactor to store an array of vector lengths in order to facilitate sharing code between the two. We do not fully handle vcpu_sve_pffr() since we have not yet introduced support for streaming mode, this will be updated as part of implementing streaming mode. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 12 +++- arch/arm64/kvm/fpsimd.c| 2 +- arch/arm64/kvm/guest.c | 6 +++--- arch/arm64/kvm/hyp/nvhe/hyp-main.c | 5 - arch/arm64/kvm/reset.c | 16 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 9180713a2f9b..3b557ffb8e7b 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -74,7 +74,7 @@ static inline enum kvm_mode kvm_get_mode(void) { return KVM_MODE_NONE; }; DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use); -extern unsigned int __ro_after_init kvm_sve_max_vl; +extern unsigned int __ro_after_init kvm_vec_max_vl[ARM64_VEC_MAX]; int __init kvm_arm_init_sve(void); u32 __attribute_const__ kvm_target_cpu(void); @@ -515,7 +515,7 @@ struct kvm_vcpu_arch { */ void *sve_state; enum fp_type fp_type; - unsigned int sve_max_vl; + unsigned int max_vl[ARM64_VEC_MAX]; u64 svcr; /* Stage 2 paging state used by the hardware on next switch */ @@ -802,15 +802,17 @@ struct kvm_vcpu_arch { /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */ #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) + \ -sve_ffr_offset((vcpu)->arch.sve_max_vl)) +sve_ffr_offset((vcpu)->arch.max_vl[ARM64_VEC_SVE])) -#define vcpu_sve_max_vq(vcpu) sve_vq_from_vl((vcpu)->arch.sve_max_vl) +#define vcpu_vec_max_vq(type, vcpu) sve_vq_from_vl((vcpu)->arch.max_vl[type]) + +#define vcpu_sve_max_vq(vcpu) vcpu_vec_max_vq(ARM64_VEC_SVE, vcpu) #define vcpu_sve_state_size(vcpu) ({ \ size_t __size_ret; \ unsigned int __vcpu_vq; \ \ - if (WARN_ON(!sve_vl_valid((vcpu)->arch.sve_max_vl))) { \ + if (WARN_ON(!sve_vl_valid((vcpu)->arch.max_vl[ARM64_VEC_SVE]))) { \ __size_ret = 0; \ } else {\ __vcpu_vq = vcpu_sve_max_vq(vcpu); \ diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c index 8c1d0d4853df..a402a072786a 100644 --- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c @@ -150,7 +150,7 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) */ fp_state.st = &vcpu->arch.ctxt.fp_regs; fp_state.sve_state = vcpu->arch.sve_state; - fp_state.sve_vl = vcpu->arch.sve_max_vl; + fp_state.sve_vl = vcpu->arch.max_vl[ARM64_VEC_SVE]; fp_state.sme_state = NULL; fp_state.svcr = &vcpu->arch.svcr; fp_state.fp_type = &vcpu->arch.fp_type; diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index aaf1d4939739..3ae08f7c0b80 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -317,7 +317,7 @@ static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (!vcpu_has_sve(vcpu)) return -ENOENT; - if (WARN_ON(!sve_vl_valid(vcpu->arch.sve_max_vl))) + if (WARN_ON(!sve_vl_valid(vcpu->arch.max_vl[ARM64_VEC_SVE]))) return -EINVAL; memset(vqs, 0, sizeof(vqs)); @@ -355,7 +355,7 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (vq_present(vqs, vq)) max_vq = vq; - if (max_vq > sve_vq_from_vl(kvm_sve_max_vl)) + if (max_vq > sve_vq_from_vl(kvm_vec_max_vl[ARM64_VEC_SVE])) return -EINVAL; /* @@ -374,7 +374,7 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return -EINVAL; /* vcpu->arch.sve_state will be alloc'd by kvm_vcpu_finalize_sve() */ - vcpu->arch.sve_max_vl = sve_vl_from_vq(max_vq); + vcpu->arch.max_vl[ARM64_VEC_SVE] = sve_vl_from_vq(max_vq); return 0; } diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index 84deed83e580..56808df6a078 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -26,11 +26,14 @@ void __kvm_hyp_host_forward_smc(struct kvm_cpu_context *host_ctxt); static void flush_hyp_vcpu(struct
[PATCH RFC v2 05/22] KVM: arm64: Document the KVM ABI for SME
SME, the Scalable Matrix Extension, is an arm64 extension which adds support for matrix operations, with core concepts patterned after SVE. SVE introduced some complication in the ABI since it adds new vector floating point registers with runtime configurable size, the size being controlled by a prameter called the vector length (VL). To provide control of this to VMMs we offer two phase configuration of SVE, SVE must first be enabled for the vCPU with KVM_ARM_VCPU_INIT(KVM_ARM_VCPU_SVE), after which vector length may then be configured but the configurably sized floating point registers are inaccessible until finalized with a call to KVM_ARM_VCPU_FINALIZE(KVM_ARM_VCPU_SVE) after which the configurably sized registers can be accessed. SME introduces an additional independent configurable vector length which as well as controlling the size of the new ZA register also provides an alternative view of the configurably sized SVE registers (known as streaming mode) with the guest able to switch between the two modes as it pleases. There is also a fixed sized register ZT0 introduced in SME2. As well as streaming mode the guest may enable and disable ZA and (where SME2 is available) ZT0 dynamically independently of streaming mode. These modes are controlled via the system register SVCR. We handle the configuration of the vector length for SME in a similar manner to SVE, requiring initialization and finalization of the feature with a pseudo register controlling the available SME vector lengths as for SVE. Further, if the guest has both SVE and SME then finalizing one prevents further configuration of the vector length for the other. Where both SVE and SME are configured for the guest we always present the SVE registers to userspace as having the larger of the configured maximum SVE and SME vector lengths, discarding extra data at load time and zero padding on read as required if the active vector length is lower. Note that this means that enabling or disabling streaming mode while the guest is stopped will not zero Zn or Pn as it will when the guest is running, but it does allow SVCR, Zn and Pn to be read and written in any order. Userspace access to ZA and (if configured) ZT0 is always available, they will be zeroed when the guest runs if disabled in SVCR and the value read will be zero if the guest stops with them disabled. This mirrors the behaviour of the architecture, enabling access causes ZA and ZT0 to be zeroed, while allowing access to SVCR, ZA and ZT0 to be performed in any order. If SME is enabled for a guest without SVE then the FPSIMD Vn registers must be accessed via the low 128 bits of the SVE Zn registers as is the case when SVE is enabled. This is not ideal but allows access to SVCR and the registers in any order without duplication or ambiguity about which values should take effect. This may be an issue for VMMs that are unaware of SME on systems that implement it without SVE if they let SME be enabled, the lack of access to Vn may surprise them, but it seems like an unusual implementation choice. For SME unware VMMs on systems with both SVE and SME support the SVE registers may be larger than expected, this should be less disruptive than on a system without SVE as they will simply ignore the high bits of the registers. Signed-off-by: Mark Brown --- Documentation/virt/kvm/api.rst | 104 + 1 file changed, 73 insertions(+), 31 deletions(-) diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 7025b3751027..b64541fa3e2a 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -374,7 +374,7 @@ Errors: instructions from device memory (arm64) ENOSYS data abort outside memslots with no syndrome info and KVM_CAP_ARM_NISV_TO_USER not enabled (arm64) - EPERM SVE feature set but not finalized (arm64) + EPERM SVE or SME feature set but not finalized (arm64) ===== This ioctl is used to run a guest virtual cpu. While there are no @@ -2585,12 +2585,13 @@ Specifically: 0x6020 0010 00d5 FPCR32 fp_regs.fpcr === = = === -.. [1] These encodings are not accepted for SVE-enabled vcpus. See +.. [1] These encodings are not accepted for SVE or SME enabled vcpus. See KVM_ARM_VCPU_INIT. The equivalent register content can be accessed via bits [127:0] of the corresponding SVE Zn registers instead for vcpus that have SVE - enabled (see below). + or SME enabled (see below). Note carefully that this is true even + when only SVE is supported. arm64 CCSIDR registers are demultiplexed by CSSELR value:: @@ -2621,24 +2622,31 @@ arm64 SVE registers have the following bit patterns:: 0x6050 0015 060 FFR bits[256*slice + 255 : 256*slice] 0x6060 0015 f
[PATCH RFC v2 06/22] KVM: arm64: Make FFR restore optional in __sve_restore_state()
Since FFR is an optional feature of SME's streaming SVE mode in order to support load of guest register state when SME is implemented we need to provide callers of __sve_restore_state() access to the flag that the sve_load macro has indicating if FFR should be loaded. Do so, simply a matter of removing the hard coding in the asm. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_hyp.h| 2 +- arch/arm64/kvm/hyp/fpsimd.S | 1 - arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h index 145ce73fc16c..7ac38ed90062 100644 --- a/arch/arm64/include/asm/kvm_hyp.h +++ b/arch/arm64/include/asm/kvm_hyp.h @@ -111,7 +111,7 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu); void __fpsimd_save_state(struct user_fpsimd_state *fp_regs); void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs); -void __sve_restore_state(void *sve_pffr, u32 *fpsr); +void __sve_restore_state(void *sve_pffr, u32 *fpsr, bool restore_ffr); u64 __guest_enter(struct kvm_vcpu *vcpu); diff --git a/arch/arm64/kvm/hyp/fpsimd.S b/arch/arm64/kvm/hyp/fpsimd.S index 61e6f3ba7b7d..8940954b5420 100644 --- a/arch/arm64/kvm/hyp/fpsimd.S +++ b/arch/arm64/kvm/hyp/fpsimd.S @@ -21,7 +21,6 @@ SYM_FUNC_START(__fpsimd_restore_state) SYM_FUNC_END(__fpsimd_restore_state) SYM_FUNC_START(__sve_restore_state) - mov x2, #1 sve_load 0, x1, x2, 3 ret SYM_FUNC_END(__sve_restore_state) diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index f99d8af0b9af..9601212bd3ce 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -286,7 +286,7 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu) { sve_cond_update_zcr_vq(vcpu_sve_max_vq(vcpu) - 1, SYS_ZCR_EL2); __sve_restore_state(vcpu_sve_pffr(vcpu), - &vcpu->arch.ctxt.fp_regs.fpsr); + &vcpu->arch.ctxt.fp_regs.fpsr, true); write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR); } -- 2.30.2
[PATCH RFC v2 07/22] KVM: arm64: Define guest flags for SME
Introduce flags for the individually selectable features in SME which add architectural state: - Base SME which adds the system registers and ZA matrix. - SME 2 which adds ZT0. - FA64 which enables access to the full floating point feature set in streaming mode, including adding FFR in streaming mode. along with helper functions for checking them. These will be used by further patches actually implementing support for SME in guests. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 13 + arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 10 ++ 2 files changed, 23 insertions(+) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 3b557ffb8e7b..461068c99b61 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -710,6 +710,12 @@ struct kvm_vcpu_arch { #define GUEST_HAS_PTRAUTH __vcpu_single_flag(cflags, BIT(2)) /* KVM_ARM_VCPU_INIT completed */ #define VCPU_INITIALIZED __vcpu_single_flag(cflags, BIT(3)) +/* SME1 exposed to guest */ +#define GUEST_HAS_SME __vcpu_single_flag(cflags, BIT(4)) +/* SME2 exposed to guest */ +#define GUEST_HAS_SME2 __vcpu_single_flag(cflags, BIT(5)) +/* FA64 exposed to guest */ +#define GUEST_HAS_FA64 __vcpu_single_flag(cflags, BIT(6)) /* Exception pending */ #define PENDING_EXCEPTION __vcpu_single_flag(iflags, BIT(0)) @@ -780,6 +786,13 @@ struct kvm_vcpu_arch { #define vcpu_has_sve(vcpu) (system_supports_sve() && \ vcpu_get_flag(vcpu, GUEST_HAS_SVE)) +#define vcpu_has_sme(vcpu) (system_supports_sme() && \ + vcpu_get_flag(vcpu, GUEST_HAS_SME)) +#define vcpu_has_sme2(vcpu) (system_supports_sme2() && \ +vcpu_get_flag(vcpu, GUEST_HAS_SME2)) +#define vcpu_has_fa64(vcpu) (system_supports_fa64() && \ +vcpu_get_flag(vcpu, GUEST_HAS_FA64)) + #ifdef CONFIG_ARM64_PTR_AUTH #define vcpu_has_ptrauth(vcpu) \ ((cpus_have_final_cap(ARM64_HAS_ADDRESS_AUTH) ||\ diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h index bb6b571ec627..fb84834cd2a0 100644 --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h @@ -21,6 +21,16 @@ static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt) ctxt_sys_reg(ctxt, MDSCR_EL1) = read_sysreg(mdscr_el1); } +static inline bool ctxt_has_sme(struct kvm_cpu_context *ctxt) +{ + struct kvm_vcpu *vcpu = ctxt->__hyp_running_vcpu; + + if (!vcpu) + vcpu = container_of(ctxt, struct kvm_vcpu, arch.ctxt); + + return vcpu_has_sme(vcpu); +} + static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt) { ctxt_sys_reg(ctxt, TPIDR_EL0) = read_sysreg(tpidr_el0); -- 2.30.2
[PATCH RFC v2 08/22] KVM: arm64: Rename SVE finalization constants to be more general
Due to the overlap between SVE and SME vector length configuration created by streaming mode SVE we will finalize both at once. Rename the existing finalization to use _VEC (vector) for the naming to avoid confusion. Since this includes the userspace API we create an alias KVM_ARM_VCPU_VEC for the existing KVM_ARM_VCPU_SVE capability, existing code which does not enable SME will be unaffected and any SME only code will not need to use SVE constants. No functional change. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 8 +--- arch/arm64/include/uapi/asm/kvm.h | 6 ++ arch/arm64/kvm/guest.c| 10 +- arch/arm64/kvm/reset.c| 16 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 461068c99b61..920f8a1ff901 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -704,8 +704,8 @@ struct kvm_vcpu_arch { /* SVE exposed to guest */ #define GUEST_HAS_SVE __vcpu_single_flag(cflags, BIT(0)) -/* SVE config completed */ -#define VCPU_SVE_FINALIZED __vcpu_single_flag(cflags, BIT(1)) +/* SVE/SME config completed */ +#define VCPU_VEC_FINALIZED __vcpu_single_flag(cflags, BIT(1)) /* PTRAUTH exposed to guest */ #define GUEST_HAS_PTRAUTH __vcpu_single_flag(cflags, BIT(2)) /* KVM_ARM_VCPU_INIT completed */ @@ -793,6 +793,8 @@ struct kvm_vcpu_arch { #define vcpu_has_fa64(vcpu) (system_supports_fa64() && \ vcpu_get_flag(vcpu, GUEST_HAS_FA64)) +#define vcpu_has_vec(vcpu) (vcpu_has_sve(vcpu) || vcpu_has_sme(vcpu)) + #ifdef CONFIG_ARM64_PTR_AUTH #define vcpu_has_ptrauth(vcpu) \ ((cpus_have_final_cap(ARM64_HAS_ADDRESS_AUTH) ||\ @@ -1179,7 +1181,7 @@ static inline bool kvm_vm_is_protected(struct kvm *kvm) int kvm_arm_vcpu_finalize(struct kvm_vcpu *vcpu, int feature); bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu); -#define kvm_arm_vcpu_sve_finalized(vcpu) vcpu_get_flag(vcpu, VCPU_SVE_FINALIZED) +#define kvm_arm_vcpu_vec_finalized(vcpu) vcpu_get_flag(vcpu, VCPU_VEC_FINALIZED) #define kvm_has_mte(kvm) \ (system_supports_mte() && \ diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 89d2fc872d9f..3048890fac68 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -111,6 +111,12 @@ struct kvm_regs { #define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */ #define KVM_ARM_VCPU_HAS_EL2 7 /* Support nested virtualization */ +/* + * An alias for _SVE since we finalize VL configuration for both SVE and SME + * simultaneously. + */ +#define KVM_ARM_VCPU_VEC KVM_ARM_VCPU_SVE + struct kvm_vcpu_init { __u32 target; __u32 features[7]; diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 3ae08f7c0b80..6e116fd8a917 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -341,7 +341,7 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (!vcpu_has_sve(vcpu)) return -ENOENT; - if (kvm_arm_vcpu_sve_finalized(vcpu)) + if (kvm_arm_vcpu_vec_finalized(vcpu)) return -EPERM; /* too late! */ if (WARN_ON(vcpu->arch.sve_state)) @@ -496,7 +496,7 @@ static int get_sve_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (ret) return ret; - if (!kvm_arm_vcpu_sve_finalized(vcpu)) + if (!kvm_arm_vcpu_vec_finalized(vcpu)) return -EPERM; if (copy_to_user(uptr, vcpu->arch.sve_state + region.koffset, @@ -522,7 +522,7 @@ static int set_sve_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (ret) return ret; - if (!kvm_arm_vcpu_sve_finalized(vcpu)) + if (!kvm_arm_vcpu_vec_finalized(vcpu)) return -EPERM; if (copy_from_user(vcpu->arch.sve_state + region.koffset, uptr, @@ -656,7 +656,7 @@ static unsigned long num_sve_regs(const struct kvm_vcpu *vcpu) return 0; /* Policed by KVM_GET_REG_LIST: */ - WARN_ON(!kvm_arm_vcpu_sve_finalized(vcpu)); + WARN_ON(!kvm_arm_vcpu_vec_finalized(vcpu)); return slices * (SVE_NUM_PREGS + SVE_NUM_ZREGS + 1 /* FFR */) + 1; /* KVM_REG_ARM64_SVE_VLS */ @@ -674,7 +674,7 @@ static int copy_sve_reg_indices(const struct kvm_vcpu *vcpu, return 0; /* Policed by KVM_GET_REG_LIST: */ - WARN_ON(!kvm_arm_vcpu_sve_finalized(vcpu)); + WARN_ON(!kvm_arm_vcpu_vec_finalized(vcpu)); /* * Enumerate this first, so that userspace can save/restore in diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset
[PATCH RFC v2 09/22] KVM: arm64: Basic SME system register descriptions
Set up the basic system register descriptions for the more straightforward SME registers. All the registers are available from SME 1. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/sys_regs.c | 23 +++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 920f8a1ff901..4be5dda9734d 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -333,6 +333,7 @@ enum vcpu_sysreg { ACTLR_EL1, /* Auxiliary Control Register */ CPACR_EL1, /* Coprocessor Access Control */ ZCR_EL1,/* SVE Control */ + SMCR_EL1, /* SME Control */ TTBR0_EL1, /* Translation Table Base Register 0 */ TTBR1_EL1, /* Translation Table Base Register 1 */ TCR_EL1,/* Translation Control Register */ diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 4735e1b37fb3..e6339ca1d8dc 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1404,7 +1404,8 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu, if (!kvm_has_mte(vcpu->kvm)) val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_MTE); - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_SME); + if (!vcpu_has_sme(vcpu)) + val &= ~ID_AA64PFR1_EL1_SME_MASK; break; case SYS_ID_AA64ISAR1_EL1: if (!vcpu_has_ptrauth(vcpu)) @@ -1470,6 +1471,9 @@ static unsigned int id_visibility(const struct kvm_vcpu *vcpu, if (!vcpu_has_sve(vcpu)) return REG_RAZ; break; + case SYS_ID_AA64SMFR0_EL1: + if (!vcpu_has_sme(vcpu)) + return REG_RAZ; } return 0; @@ -1521,6 +1525,16 @@ static unsigned int sve_visibility(const struct kvm_vcpu *vcpu, return REG_HIDDEN; } +/* Visibility overrides for SME-specific control registers */ +static unsigned int sme_visibility(const struct kvm_vcpu *vcpu, + const struct sys_reg_desc *rd) +{ + if (vcpu_has_sme(vcpu)) + return 0; + + return REG_HIDDEN; +} + static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd) { @@ -2142,7 +2156,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_UNALLOCATED(4,2), ID_UNALLOCATED(4,3), ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0), - ID_HIDDEN(ID_AA64SMFR0_EL1), + ID_WRITABLE(ID_AA64SMFR0_EL1, ~ID_AA64SMFR0_EL1_RES0), ID_UNALLOCATED(4,6), ID_UNALLOCATED(4,7), @@ -2211,7 +2225,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility }, { SYS_DESC(SYS_TRFCR_EL1), undef_access }, { SYS_DESC(SYS_SMPRI_EL1), undef_access }, - { SYS_DESC(SYS_SMCR_EL1), undef_access }, + { SYS_DESC(SYS_SMCR_EL1), NULL, reset_val, SMCR_EL1, 0, .visibility = sme_visibility }, { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 }, @@ -2306,7 +2320,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_CLIDR_EL1), access_clidr, reset_clidr, CLIDR_EL1, .set_user = set_clidr }, { SYS_DESC(SYS_CCSIDR2_EL1), undef_access }, - { SYS_DESC(SYS_SMIDR_EL1), undef_access }, + { SYS_DESC(SYS_SMIDR_EL1), .access = access_id_reg, + .get_user = get_id_reg, .visibility = sme_visibility }, { SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 }, { SYS_DESC(SYS_CTR_EL0), access_ctr }, { SYS_DESC(SYS_SVCR), undef_access }, -- 2.30.2
[PATCH RFC v2 10/22] KVM: arm64: Add support for TPIDR2_EL0
SME adds a new user register TPIDR2_EL0, implement support for mananging this for guests. We provide system register access to it, disable traps and context switch it along with the other TPIDRs for guests with SME. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/kvm/hyp/include/hyp/switch.h| 5 - arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 6 ++ arch/arm64/kvm/sys_regs.c | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 4be5dda9734d..36bf9d7e92e1 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -347,6 +347,7 @@ enum vcpu_sysreg { CONTEXTIDR_EL1, /* Context ID Register */ TPIDR_EL0, /* Thread ID, User R/W */ TPIDRRO_EL0,/* Thread ID, User R/O */ + TPIDR2_EL0, /* Thread ID 2, User R/W */ TPIDR_EL1, /* Thread ID, Privileged */ AMAIR_EL1, /* Aux Memory Attribute Indirection Register */ CNTKCTL_EL1,/* Timer Control Register (EL1) */ @@ -885,6 +886,7 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val) case CONTEXTIDR_EL1:*val = read_sysreg_s(SYS_CONTEXTIDR_EL12);break; case TPIDR_EL0: *val = read_sysreg_s(SYS_TPIDR_EL0);break; case TPIDRRO_EL0: *val = read_sysreg_s(SYS_TPIDRRO_EL0); break; + case TPIDR2_EL0:*val = read_sysreg_s(SYS_TPIDR2_EL0); break; case TPIDR_EL1: *val = read_sysreg_s(SYS_TPIDR_EL1);break; case AMAIR_EL1: *val = read_sysreg_s(SYS_AMAIR_EL12); break; case CNTKCTL_EL1: *val = read_sysreg_s(SYS_CNTKCTL_EL12); break; @@ -928,6 +930,7 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg) case VBAR_EL1: write_sysreg_s(val, SYS_VBAR_EL12); break; case CONTEXTIDR_EL1:write_sysreg_s(val, SYS_CONTEXTIDR_EL12);break; case TPIDR_EL0: write_sysreg_s(val, SYS_TPIDR_EL0); break; + case TPIDR2_EL0:write_sysreg_s(val, SYS_TPIDR2_EL0);break; case TPIDRRO_EL0: write_sysreg_s(val, SYS_TPIDRRO_EL0); break; case TPIDR_EL1: write_sysreg_s(val, SYS_TPIDR_EL1); break; case AMAIR_EL1: write_sysreg_s(val, SYS_AMAIR_EL12);break; diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 9601212bd3ce..72982e752972 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -93,7 +93,10 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) ctxt_sys_reg(hctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2); if (cpus_have_final_cap(ARM64_SME)) { - tmp = HFGxTR_EL2_nSMPRI_EL1_MASK | HFGxTR_EL2_nTPIDR2_EL0_MASK; + tmp = HFGxTR_EL2_nSMPRI_EL1_MASK; + + if (!vcpu_has_sme(vcpu)) + tmp |= HFGxTR_EL2_nTPIDR2_EL0_MASK; r_clr |= tmp; w_clr |= tmp; diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h index fb84834cd2a0..5436b33d50b7 100644 --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h @@ -35,6 +35,9 @@ static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt) { ctxt_sys_reg(ctxt, TPIDR_EL0) = read_sysreg(tpidr_el0); ctxt_sys_reg(ctxt, TPIDRRO_EL0) = read_sysreg(tpidrro_el0); + + if (ctxt_has_sme(ctxt)) + ctxt_sys_reg(ctxt, TPIDR2_EL0) = read_sysreg_s(SYS_TPIDR2_EL0); } static inline bool ctxt_has_mte(struct kvm_cpu_context *ctxt) @@ -105,6 +108,9 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt) { write_sysreg(ctxt_sys_reg(ctxt, TPIDR_EL0), tpidr_el0); write_sysreg(ctxt_sys_reg(ctxt, TPIDRRO_EL0), tpidrro_el0); + + if (ctxt_has_sme(ctxt)) + write_sysreg_s(ctxt_sys_reg(ctxt, TPIDR2_EL0), SYS_TPIDR2_EL0); } static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index e6339ca1d8dc..a33ad12dc3ab 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2369,7 +2369,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_TPIDR_EL0), NULL, reset_unknown, TPIDR_EL0 }, { SYS_DESC(SYS_TPIDRRO_EL0), NULL, reset_unknown, TPIDRRO_EL0 }, - { SYS_DESC(SYS_TPIDR2_EL0), undef_access }, + { SYS_DESC(SYS_TPIDR2_EL0), NULL, reset_unknown, TPIDR2_EL0, 0, .visibility = sme_visibility }, { SYS_DESC(SYS_SCXTNUM_EL0), undef_access }, -- 2.30.2
[PATCH RFC v2 11/22] KVM: arm64: Make SMPRI_EL1 RES0 for SME guests
SME priorities are entirely implementation defined and Linux currently has no support for SME priorities so we do not expose them to SME capable guests, reporting SMIDR_EL1.SMPS as 0. This means that on a host which supports SME priorities we need to trap writes to SMPRI_EL1 and make the whole register RES0. For a guest that does not support SME at all the register should instead be undefined. If the host does not support SME priorities we could disable the trap but since there is no reason to access SMPRI_EL1 on a system that does not support priorities it seems more trouble than it is worth to detect and handle this eventuality, especially given the lack of SME priority support in the host and potential user confusion that may result if we report that the feature is detected but do not provide any interface for it. With the current specification and host implementation we could disable read traps for all guests since we initialise SMPRI_EL1.Priority to 0 but for robustness when we do start implementing host support for priorities or if more fields are added just leave the traps enabled. Once we have physical implementations that implement SME priorities and an understanding of how their use impacts the system we can consider exposing the feature to guests in some form but this will require substantial study. Signed-off-by: Mark Brown --- arch/arm64/kvm/hyp/include/hyp/switch.h | 5 + arch/arm64/kvm/sys_regs.c | 14 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h index 72982e752972..0cf4770b9d70 100644 --- a/arch/arm64/kvm/hyp/include/hyp/switch.h +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h @@ -93,6 +93,11 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu) ctxt_sys_reg(hctxt, HFGWTR_EL2) = read_sysreg_s(SYS_HFGWTR_EL2); if (cpus_have_final_cap(ARM64_SME)) { + /* +* We hide priorities from guests so need to trap +* access to SMPRI_EL1 in order to map it to RES0 +* even if the guest has SME. +*/ tmp = HFGxTR_EL2_nSMPRI_EL1_MASK; if (!vcpu_has_sme(vcpu)) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index a33ad12dc3ab..b618bcab526e 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -351,6 +351,18 @@ static bool access_gic_sre(struct kvm_vcpu *vcpu, return true; } +static bool access_res0(struct kvm_vcpu *vcpu, + struct sys_reg_params *p, + const struct sys_reg_desc *r) +{ + if (p->is_write) + return ignore_write(vcpu, p); + + p->regval = 0; + + return true; +} + static bool trap_raz_wi(struct kvm_vcpu *vcpu, struct sys_reg_params *p, const struct sys_reg_desc *r) @@ -2224,7 +2236,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_ZCR_EL1), NULL, reset_val, ZCR_EL1, 0, .visibility = sve_visibility }, { SYS_DESC(SYS_TRFCR_EL1), undef_access }, - { SYS_DESC(SYS_SMPRI_EL1), undef_access }, + { SYS_DESC(SYS_SMPRI_EL1), .access = access_res0, .visibility = sme_visibility }, { SYS_DESC(SYS_SMCR_EL1), NULL, reset_val, SMCR_EL1, 0, .visibility = sme_visibility }, { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, -- 2.30.2
[PATCH RFC v2 12/22] KVM: arm64: Make SVCR a normal system register
As a placeholder while SME guests were not supported we provide a u64 in struct kvm_vcpu_arch for the host kernel's floating point save code to use when managing KVM guests. In order to support KVM guests we will need to replace this with a proper KVM system register, do so and update the system register definition to make it accessible to the guest if it has SME. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/fpsimd.c | 8 +--- arch/arm64/kvm/sys_regs.c | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 36bf9d7e92e1..690c439b5e2a 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -356,6 +356,7 @@ enum vcpu_sysreg { MDCCINT_EL1,/* Monitor Debug Comms Channel Interrupt Enable Reg */ OSLSR_EL1, /* OS Lock Status Register */ DISR_EL1, /* Deferred Interrupt Status Register */ + SVCR, /* Scalable Vector Control Register */ /* Performance Monitors Registers */ PMCR_EL0, /* Control Register */ @@ -518,7 +519,6 @@ struct kvm_vcpu_arch { void *sve_state; enum fp_type fp_type; unsigned int max_vl[ARM64_VEC_MAX]; - u64 svcr; /* Stage 2 paging state used by the hardware on next switch */ struct kvm_s2_mmu *hw_mmu; diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c index a402a072786a..1be18d719fce 100644 --- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c @@ -145,14 +145,16 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) if (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED) { /* -* Currently we do not support SME guests so SVCR is -* always 0 and we just need a variable to point to. +* We peer into the registers since SVCR is saved as +* part of the floating point state, determining which +* registers exist and their size, so is saved by +* fpsimd_save(). */ fp_state.st = &vcpu->arch.ctxt.fp_regs; fp_state.sve_state = vcpu->arch.sve_state; fp_state.sve_vl = vcpu->arch.max_vl[ARM64_VEC_SVE]; fp_state.sme_state = NULL; - fp_state.svcr = &vcpu->arch.svcr; + fp_state.svcr = &(vcpu->arch.ctxt.sys_regs[SVCR]); fp_state.fp_type = &vcpu->arch.fp_type; if (vcpu_has_sve(vcpu)) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index b618bcab526e..f908aa3fb606 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2336,7 +2336,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { .get_user = get_id_reg, .visibility = sme_visibility }, { SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 }, { SYS_DESC(SYS_CTR_EL0), access_ctr }, - { SYS_DESC(SYS_SVCR), undef_access }, + { SYS_DESC(SYS_SVCR), NULL, reset_val, SVCR, 0, .visibility = sme_visibility }, { PMU_SYS_REG(PMCR_EL0), .access = access_pmcr, .reset = reset_pmcr, .reg = PMCR_EL0, .get_user = get_pmcr, .set_user = set_pmcr }, -- 2.30.2
[PATCH RFC v2 13/22] KVM: arm64: Context switch SME state for guest
If the guest has access to SME then we need to restore state for it and configure SMCR_EL2 to grant the guest access to the vector lengths that it has access to, along with FA64 and ZT0 if they were enabled. SME has three sets of registers, ZA, ZT (only present if SME2 is supported for the guest) and streaming SVE. ZA and streaming SVE use a separately configured streaming mode vector length. The first two are fairly straightforward, they are enabled by PSTATE.ZA which is saved and restored via SVCR.ZA. We reuse the assembly the host already uses to load them from a single contiguous buffer. If PSTATE.ZA is not set they can only be accessed by setting PSTATE.ZA which will initialise them to all bits 0. Streaming mode SVE presents some complications, this is a version of the SVE registers which may optionally omit the FFR register and which uses the streaming mode vector length. Similarly to ZA and ZT it is controlled by PSTATE.SM which we save and restore with SVCR.SM. Any change in the value of PSTATE.SM initialises all bits of the floating point registers to 0 so we do not need to be concerned about information leakage due to changes in vector length between streaming and non-streaming modes. The overall floating point restore is modified to start by assuming that we will restore either FPSIMD or full SVE register state depending on if the guest has access to vanilla SVE. If the guest has SME we then check the SME configuration we are restoring and override as appropriate. Since SVE instructions are VL agnostic the hardware will load using the appropriate vector length without us needing to pass further flags. We also modify the floating point save to save SMCR and restore the host configuration in a similar way to ZCR, the code is slightly more complex since for SME as well as the vector length we also have enables for FA64 and ZT0 in the register. Since the layout of the SVE register state depends on the vector length we need to pass the active vector length when getting the pointer to FFR. Since for reasons discussed at length in aaa2f14e6f3f ("KVM: arm64: Clarify host SME state management") we always disable both PSTATE.SM and PSTATE.ZA prior to running the guest meaning that we do not need to worry about saving host SME state. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 7 ++- arch/arm64/include/asm/kvm_hyp.h| 1 + arch/arm64/kvm/fpsimd.c | 83 ++--- arch/arm64/kvm/hyp/fpsimd.S | 10 arch/arm64/kvm/hyp/include/hyp/switch.h | 76 -- 5 files changed, 143 insertions(+), 34 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 690c439b5e2a..022b9585e6f6 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -517,6 +517,7 @@ struct kvm_vcpu_arch { * records which view it saved in fp_type. */ void *sve_state; + void *sme_state; enum fp_type fp_type; unsigned int max_vl[ARM64_VEC_MAX]; @@ -818,13 +819,15 @@ struct kvm_vcpu_arch { #define vcpu_gp_regs(v)(&(v)->arch.ctxt.regs) /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */ -#define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) + \ -sve_ffr_offset((vcpu)->arch.max_vl[ARM64_VEC_SVE])) +#define vcpu_sve_pffr(vcpu, vec) (kern_hyp_va((vcpu)->arch.sve_state) + \ +sve_ffr_offset((vcpu)->arch.max_vl[vec])) #define vcpu_vec_max_vq(type, vcpu) sve_vq_from_vl((vcpu)->arch.max_vl[type]) #define vcpu_sve_max_vq(vcpu) vcpu_vec_max_vq(ARM64_VEC_SVE, vcpu) +#define vcpu_sme_max_vq(vcpu) vcpu_vec_max_vq(ARM64_VEC_SME, vcpu) + #define vcpu_sve_state_size(vcpu) ({ \ size_t __size_ret; \ unsigned int __vcpu_vq; \ diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h index 7ac38ed90062..e555af02ece3 100644 --- a/arch/arm64/include/asm/kvm_hyp.h +++ b/arch/arm64/include/asm/kvm_hyp.h @@ -112,6 +112,7 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu); void __fpsimd_save_state(struct user_fpsimd_state *fp_regs); void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs); void __sve_restore_state(void *sve_pffr, u32 *fpsr, bool restore_ffr); +void __sme_restore_state(void const *state, bool zt); u64 __guest_enter(struct kvm_vcpu *vcpu); diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c index 1be18d719fce..d9a56a4027a6 100644 --- a/arch/arm64/kvm/fpsimd.c +++ b/arch/arm64/kvm/fpsimd.c @@ -153,10 +153,15 @@ void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu) fp_state.st = &vcpu->arch.ctxt.fp_regs; fp_state.sve_state = vcpu->arch.sve_state; fp_stat
[PATCH RFC v2 14/22] KVM: arm64: Manage and handle SME traps
Now that we have support for managing SME state for KVM guests add handling for SME exceptions generated by guests. As with SVE these are routed to the generic floating point exception handlers for both VHE and nVHE, the floating point state is handled as a uniform block. Since we do not presently support SME for protected VMs handle exceptions from protected guests as UNDEF. For nVHE and hVHE modes we currently do a lazy restore of the host EL2 setup for SVE, do the same for SME. Since it is likely that there will be common situations where SVE and SME are both used in quick succession by the host (eg, saving the guest state) restore the configuration for both at once in order to minimise the number of EL2 entries. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_emulate.h | 12 arch/arm64/kvm/handle_exit.c | 11 +++ arch/arm64/kvm/hyp/nvhe/hyp-main.c | 56 ++-- arch/arm64/kvm/hyp/nvhe/switch.c | 13 - arch/arm64/kvm/hyp/vhe/switch.c | 3 ++ 5 files changed, 73 insertions(+), 22 deletions(-) diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h index 14d6ff2e2a39..756c2c28c592 100644 --- a/arch/arm64/include/asm/kvm_emulate.h +++ b/arch/arm64/include/asm/kvm_emulate.h @@ -584,16 +584,15 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) if (has_vhe()) { val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN | - CPACR_EL1_ZEN_EL1EN); - if (cpus_have_final_cap(ARM64_SME)) - val |= CPACR_EL1_SMEN_EL1EN; + CPACR_EL1_ZEN_EL1EN | CPACR_EL1_SMEN_EL1EN); } else if (has_hvhe()) { val = (CPACR_EL1_FPEN_EL0EN | CPACR_EL1_FPEN_EL1EN); if (!vcpu_has_sve(vcpu) || (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED)) val |= CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN; - if (cpus_have_final_cap(ARM64_SME)) + if (!vcpu_has_sme(vcpu) || + (vcpu->arch.fp_state != FP_STATE_GUEST_OWNED)) val |= CPACR_EL1_SMEN_EL1EN | CPACR_EL1_SMEN_EL0EN; } else { val = CPTR_NVHE_EL2_RES1; @@ -602,8 +601,9 @@ static __always_inline u64 kvm_get_reset_cptr_el2(struct kvm_vcpu *vcpu) if (vcpu_has_sve(vcpu) && (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)) val |= CPTR_EL2_TZ; - if (cpus_have_final_cap(ARM64_SME)) - val &= ~CPTR_EL2_TSM; + if (vcpu_has_sme(vcpu) && + (vcpu->arch.fp_state == FP_STATE_GUEST_OWNED)) + val |= CPTR_EL2_TSM; } return val; diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 617ae6dea5d5..e5d8d8767872 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -206,6 +206,16 @@ static int handle_sve(struct kvm_vcpu *vcpu) return 1; } +/* + * Guest access to SME registers should be routed to this handler only + * when the system doesn't support SME. + */ +static int handle_sme(struct kvm_vcpu *vcpu) +{ + kvm_inject_undefined(vcpu); + return 1; +} + /* * Guest usage of a ptrauth instruction (which the guest EL1 did not turn into * a NOP). If we get here, it is that we didn't fixup ptrauth on exit, and all @@ -268,6 +278,7 @@ static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_SVC64] = handle_svc, [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, [ESR_ELx_EC_SVE]= handle_sve, + [ESR_ELx_EC_SME]= handle_sme, [ESR_ELx_EC_ERET] = kvm_handle_eret, [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort, [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort, diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index 56808df6a078..b2da4800b673 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -411,6 +411,52 @@ static void handle_host_smc(struct kvm_cpu_context *host_ctxt) kvm_skip_host_instr(); } +static void handle_host_vec(void) +{ + u64 old_smcr, new_smcr; + u64 mask = 0; + + /* +* Handle lazy restore of the EL2 configuration for host SVE +* and SME usage. It is likely that when a host supports both +* SVE and SME it will use both in quick succession (eg, +* saving guest state) so we restore both when either traps. +*/ + if (has_hvhe()) { + if (cpus_have_final_cap(ARM64_SVE)) + mask |= CPACR_EL1_ZEN_EL1EN | CPACR_EL1_ZEN_EL0EN; + if (cpus_have_final_cap(ARM64_SME)) + mask |= CPACR_EL1_SMEN_EL1EN | CPACR_EL1_SMEN_EL0EN; + + sysreg_clear_set(cpacr_el1, 0, mask); + } el
[PATCH RFC v2 15/22] KVM: arm64: Implement SME vector length configuration
Configuration for SME vector lengths is done using a virtual register as for SVE, hook up the implementation for the virtual register. Since we do not yet have support for any of the new SME registers stub register access functions are provided. Signed-off-by: Mark Brown --- arch/arm64/include/uapi/asm/kvm.h | 9 arch/arm64/kvm/guest.c| 94 +++ 2 files changed, 84 insertions(+), 19 deletions(-) diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 3048890fac68..02642bb96496 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -353,6 +353,15 @@ struct kvm_arm_counter_offset { #define KVM_ARM64_SVE_VLS_WORDS\ ((KVM_ARM64_SVE_VQ_MAX - KVM_ARM64_SVE_VQ_MIN) / 64 + 1) +/* SME registers */ +#define KVM_REG_ARM64_SME (0x17 << KVM_REG_ARM_COPROC_SHIFT) + +/* Vector lengths pseudo-register: */ +#define KVM_REG_ARM64_SME_VLS (KVM_REG_ARM64 | KVM_REG_ARM64_SME | \ +KVM_REG_SIZE_U512 | 0x) +#define KVM_ARM64_SME_VLS_WORDS\ + ((KVM_ARM64_SVE_VQ_MAX - KVM_ARM64_SVE_VQ_MIN) / 64 + 1) + /* Bitmap feature firmware registers */ #define KVM_REG_ARM_FW_FEAT_BMAP (0x0016 << KVM_REG_ARM_COPROC_SHIFT) #define KVM_REG_ARM_FW_FEAT_BMAP_REG(r)(KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 6e116fd8a917..30446ae357f0 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -309,22 +309,20 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) #define vq_mask(vq) ((u64)1 << ((vq) - SVE_VQ_MIN) % 64) #define vq_present(vqs, vq) (!!((vqs)[vq_word(vq)] & vq_mask(vq))) -static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) +static int get_vec_vls(enum vec_type vec_type, struct kvm_vcpu *vcpu, + const struct kvm_one_reg *reg) { unsigned int max_vq, vq; u64 vqs[KVM_ARM64_SVE_VLS_WORDS]; - if (!vcpu_has_sve(vcpu)) - return -ENOENT; - - if (WARN_ON(!sve_vl_valid(vcpu->arch.max_vl[ARM64_VEC_SVE]))) + if (WARN_ON(!sve_vl_valid(vcpu->arch.max_vl[vec_type]))) return -EINVAL; memset(vqs, 0, sizeof(vqs)); - max_vq = vcpu_sve_max_vq(vcpu); + max_vq = vcpu_vec_max_vq(vec_type, vcpu); for (vq = SVE_VQ_MIN; vq <= max_vq; ++vq) - if (sve_vq_available(vq)) + if (vq_available(vec_type, vq)) vqs[vq_word(vq)] |= vq_mask(vq); if (copy_to_user((void __user *)reg->addr, vqs, sizeof(vqs))) @@ -333,18 +331,13 @@ static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return 0; } -static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) +static int set_vec_vls(enum vec_type vec_type, struct kvm_vcpu *vcpu, + const struct kvm_one_reg *reg) { unsigned int max_vq, vq; u64 vqs[KVM_ARM64_SVE_VLS_WORDS]; - if (!vcpu_has_sve(vcpu)) - return -ENOENT; - - if (kvm_arm_vcpu_vec_finalized(vcpu)) - return -EPERM; /* too late! */ - - if (WARN_ON(vcpu->arch.sve_state)) + if (WARN_ON(!sve_vl_valid(vcpu->arch.max_vl[vec_type]))) return -EINVAL; if (copy_from_user(vqs, (const void __user *)reg->addr, sizeof(vqs))) @@ -355,18 +348,18 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) if (vq_present(vqs, vq)) max_vq = vq; - if (max_vq > sve_vq_from_vl(kvm_vec_max_vl[ARM64_VEC_SVE])) + if (max_vq > sve_vq_from_vl(kvm_vec_max_vl[vec_type])) return -EINVAL; /* * Vector lengths supported by the host can't currently be * hidden from the guest individually: instead we can only set a -* maximum via ZCR_EL2.LEN. So, make sure the available vector +* maximum via xCR_EL2.LEN. So, make sure the available vector * lengths match the set requested exactly up to the requested * maximum: */ for (vq = SVE_VQ_MIN; vq <= max_vq; ++vq) - if (vq_present(vqs, vq) != sve_vq_available(vq)) + if (vq_present(vqs, vq) != vq_available(vec_type, vq)) return -EINVAL; /* Can't run with no vector lengths at all: */ @@ -374,11 +367,33 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return -EINVAL; /* vcpu->arch.sve_state will be alloc'd by kvm_vcpu_finalize_sve() */ - vcpu->arch.max_vl[ARM64_VEC_SVE] = sve_vl_from_vq(max_vq); + vcpu->arch.max_vl[vec_type] = sve_vl_from_vq(max_vq); return 0; } +static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
[PATCH RFC v2 16/22] KVM: arm64: Rename sve_state_reg_region
As for SVE we will need to pull parts of dynamically sized registers out of a block of memory for SME so we will use a similar code pattern for this. Rename the current struct sve_state_reg_region in preparation for this. Signed-off-by: Mark Brown --- arch/arm64/kvm/guest.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 30446ae357f0..1d161fa7c2be 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -418,9 +418,9 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) */ #define vcpu_sve_slices(vcpu) 1 -/* Bounds of a single SVE register slice within vcpu->arch.sve_state */ -struct sve_state_reg_region { - unsigned int koffset; /* offset into sve_state in kernel memory */ +/* Bounds of a single register slice within vcpu->arch.s[mv]e_state */ +struct vec_state_reg_region { + unsigned int koffset; /* offset into s[mv]e_state in kernel memory */ unsigned int klen; /* length in kernel memory */ unsigned int upad; /* extra trailing padding in user memory */ }; @@ -429,7 +429,7 @@ struct sve_state_reg_region { * Validate SVE register ID and get sanitised bounds for user/kernel SVE * register copy */ -static int sve_reg_to_region(struct sve_state_reg_region *region, +static int sve_reg_to_region(struct vec_state_reg_region *region, struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { @@ -499,7 +499,7 @@ static int sve_reg_to_region(struct sve_state_reg_region *region, static int get_sve_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { int ret; - struct sve_state_reg_region region; + struct vec_state_reg_region region; char __user *uptr = (char __user *)reg->addr; /* Handle the KVM_REG_ARM64_SVE_VLS pseudo-reg as a special case: */ @@ -525,7 +525,7 @@ static int get_sve_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) static int set_sve_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { int ret; - struct sve_state_reg_region region; + struct vec_state_reg_region region; const char __user *uptr = (const char __user *)reg->addr; /* Handle the KVM_REG_ARM64_SVE_VLS pseudo-reg as a special case: */ -- 2.30.2
[PATCH RFC v2 17/22] KVM: arm64: Support userspace access to streaming mode SVE registers
SME defines a new mode called streaming mode with an associated vector length which may be configured independently to the standard SVE vector length. SVE and SME also have no interdependency, it is valid to implement SME without SVE. When in streaming mode the SVE registers are accessible with the streaming mode vector length rather than the usual SVE vector length. In order to handle this we extend the existing SVE register interface to expose the vector length dependent registers with the larger of the SVE or SME vector length and requiring access to the V registers via the SVE register set if the system supports only SME. The main complication this creates is that the format that is sensible for saving and restoring the registers in the hypervisor is no longer the same as that which is exposed to userspace. This is especially true when the host does not have SVE, in that case we must use FPSIMD register save and restore code in non-streaming mode. Handle this by extending our set of register access owners to include userspace access, converting to the format used by the hypervisor if needed when preparing to run the guest and to userspace format when the registers are read. This avoids any conversions in normal hypervisor scheduling. The userspace format is the existing SVE format with the largest vector length used. The hypervisor format varies depending on the current value of SVCR.SM and if SVE is supported: - If SVCR.SM=1 then SVE format with the SME vector length is used. - If SVCR.SM=0 and SVE is not supported then the FPSIMD format is used. - If SVCR.SM=0 and SVE is supported the SVE format and vector length are used. When converting to a larger vector length we pad the high bits with 0. Since we already track the owner of the floating point register state introduce a new state FP_STATE_USER_OWNED in which the state is stored in the format we present to userspace. The guest starts out in this state. When we prepare to run the guest we check for this state and the guest we check for this state and if we are in it we do any rewrites needed to store in the format the hypervisor expects. In order to minimise overhead we only convert back to userspace format if userspace accesses the SVE registers. For simpliciy we always represent FFR for SVE storage, if the system lacks both SVE and streaming mode FFR then the value will be visible but ignored. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 6 +- arch/arm64/kvm/arm.c | 9 +- arch/arm64/kvm/fpsimd.c | 173 ++ arch/arm64/kvm/guest.c| 16 ++-- 4 files changed, 195 insertions(+), 9 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 022b9585e6f6..a5ed0433edc6 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -540,6 +540,7 @@ struct kvm_vcpu_arch { FP_STATE_FREE, FP_STATE_HOST_OWNED, FP_STATE_GUEST_OWNED, + FP_STATE_USER_OWNED, } fp_state; /* Configuration flags, set once and for all before the vcpu can run */ @@ -828,6 +829,9 @@ struct kvm_vcpu_arch { #define vcpu_sme_max_vq(vcpu) vcpu_vec_max_vq(ARM64_VEC_SME, vcpu) +int vcpu_max_vq(struct kvm_vcpu *vcpu); +void vcpu_fp_guest_to_user(struct kvm_vcpu *vcpu); + #define vcpu_sve_state_size(vcpu) ({ \ size_t __size_ret; \ unsigned int __vcpu_vq; \ @@ -835,7 +839,7 @@ struct kvm_vcpu_arch { if (WARN_ON(!sve_vl_valid((vcpu)->arch.max_vl[ARM64_VEC_SVE]))) { \ __size_ret = 0; \ } else {\ - __vcpu_vq = vcpu_sve_max_vq(vcpu); \ + __vcpu_vq = vcpu_max_vq(vcpu); \ __size_ret = SVE_SIG_REGS_SIZE(__vcpu_vq); \ } \ \ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index e5f75f1f1085..aa7e2031263c 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -376,9 +376,14 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) /* * Default value for the FP state, will be overloaded at load -* time if we support FP (pretty likely) +* time if we support FP (pretty likely). If we support both +* SVE and SME we may have to rewrite between the two VLs, +* default to formatting the registers for userspace access. */ - vcpu->arch.fp_state = FP_STATE_FREE; + if (system_supports_sve() && system_supports_sme()) + vcpu->arch.fp_state = FP_STATE_USER_OWN
[PATCH RFC v2 18/22] KVM: arm64: Expose ZA to userspace
The SME ZA matrix is a single SVL*SVL register which is available when PSTATE.ZA is set. We follow the pattern established by the architecture itself and expose this to userspace as a series of horizontal SVE vectors with the streaming mode vector length, using the format already established for the SVE vectors themselves. For the purposes of exporting to userspace we ignore the value of PSTATE.ZA, if PSTATE.ZA is clear when the guest is run then the guest will need to set it to access ZA which would cause the value to be cleared. If userspace reads ZA when PSTATE.ZA is clear then it will read whatever stale data was last saved. This removes ordering requirements from userspace, minimising the need to special case. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 14 ++ arch/arm64/include/uapi/asm/kvm.h | 15 +++ arch/arm64/kvm/guest.c| 95 ++- 3 files changed, 122 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index a5ed0433edc6..a1aa9471084d 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -846,6 +846,20 @@ void vcpu_fp_guest_to_user(struct kvm_vcpu *vcpu); __size_ret; \ }) +#define vcpu_sme_state_size(vcpu) ({ \ + size_t __size_ret; \ + unsigned int __vcpu_vq; \ + \ + if (WARN_ON(!sve_vl_valid((vcpu)->arch.max_vl[ARM64_VEC_SME]))) { \ + __size_ret = 0; \ + } else {\ + __vcpu_vq = vcpu_sme_max_vq(vcpu); \ + __size_ret = ZA_SIG_REGS_SIZE(__vcpu_vq); \ + } \ + \ + __size_ret; \ +}) + /* * Only use __vcpu_sys_reg/ctxt_sys_reg if you know you want the * memory backed version of a register, and not the one most recently diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 02642bb96496..00fb2ea4c057 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -356,6 +356,21 @@ struct kvm_arm_counter_offset { /* SME registers */ #define KVM_REG_ARM64_SME (0x17 << KVM_REG_ARM_COPROC_SHIFT) +#define KVM_ARM64_SME_VQ_MIN __SVE_VQ_MIN +#define KVM_ARM64_SME_VQ_MAX __SVE_VQ_MAX + +/* ZA and ZTn occupy blocks at the following offsets within this range: */ +#define KVM_REG_ARM64_SME_ZA_BASE 0 +#define KVM_REG_ARM64_SME_ZT_BASE 0x600 + +#define KVM_ARM64_SME_MAX_ZAHREG (__SVE_VQ_BYTES * KVM_ARM64_SME_VQ_MAX) + +#define KVM_REG_ARM64_SME_ZAHREG(n, i) \ + (KVM_REG_ARM64 | KVM_REG_ARM64_SME | KVM_REG_ARM64_SME_ZA_BASE | \ +KVM_REG_SIZE_U2048 | \ +(((n) & (KVM_ARM64_SME_MAX_ZAHREG - 1)) << 5) |\ +((i) & (KVM_ARM64_SVE_MAX_SLICES - 1))) + /* Vector lengths pseudo-register: */ #define KVM_REG_ARM64_SME_VLS (KVM_REG_ARM64 | KVM_REG_ARM64_SME | \ KVM_REG_SIZE_U512 | 0x) diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 5f2845625c55..cb38af891387 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -573,22 +573,113 @@ static int set_sme_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return set_vec_vls(ARM64_VEC_SME, vcpu, reg); } +/* + * Validate SVE register ID and get sanitised bounds for user/kernel SVE + * register copy + */ +static int sme_reg_to_region(struct vec_state_reg_region *region, +struct kvm_vcpu *vcpu, +const struct kvm_one_reg *reg) +{ + /* reg ID ranges for ZA.H[n] registers */ + unsigned int vq = vcpu_sme_max_vq(vcpu) - 1; + const u64 za_h_max = vq * __SVE_VQ_BYTES; + const u64 zah_id_min = KVM_REG_ARM64_SME_ZAHREG(0, 0); + const u64 zah_id_max = KVM_REG_ARM64_SME_ZAHREG(za_h_max - 1, + SVE_NUM_SLICES - 1); + + unsigned int reg_num; + + unsigned int reqoffset, reqlen; /* User-requested offset and length */ + unsigned int maxlen; /* Maximum permitted length */ + + size_t sme_state_size; + + reg_num = (reg->id & SVE_REG_ID_MASK) >> SVE_REG_ID_SHIFT; + + if (reg->id >= zah_id_min && reg->id <= zah_id_max) { + /* ZA is exposed as SVE vectors ZA.H[n] */ + if (!vcpu_has_sm
[PATCH RFC v2 19/22] KVM: arm64: Provide userspace access to ZT0
ZT0 is a single register with a refreshingly fixed size 512 bit register which is like ZA accessible only when PSTATE.ZA is set. Add support for it to the userspace API, as with ZA we allow the regster to be read or written regardless of the state of PSTATE.ZA in order to simplify userspace usage. The value will be reset to 0 whenever PSTATE.ZA changes from 0 to 1, userspace can read stale values but these are not observable by the guest without manipulation of PSTATE.ZA by userspace. While there is currently only one ZT register the naming as ZT0 and the instruction encoding clearly leave room for future extensions adding more ZT registers. This encoding can readily support such an extension if one is introduced. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 2 ++ arch/arm64/include/uapi/asm/kvm.h | 2 ++ arch/arm64/kvm/guest.c| 13 +++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index a1aa9471084d..6a5002ab8042 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -855,6 +855,8 @@ void vcpu_fp_guest_to_user(struct kvm_vcpu *vcpu); } else {\ __vcpu_vq = vcpu_sme_max_vq(vcpu); \ __size_ret = ZA_SIG_REGS_SIZE(__vcpu_vq); \ + if (system_supports_sme2()) \ + __size_ret += ZT_SIG_REG_SIZE; \ } \ \ __size_ret; \ diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 00fb2ea4c057..58640aeb88e4 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -371,6 +371,8 @@ struct kvm_arm_counter_offset { (((n) & (KVM_ARM64_SME_MAX_ZAHREG - 1)) << 5) |\ ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1))) +#define KVM_REG_ARM64_SME_ZTREG_SIZE (512 / 8) + /* Vector lengths pseudo-register: */ #define KVM_REG_ARM64_SME_VLS (KVM_REG_ARM64 | KVM_REG_ARM64_SME | \ KVM_REG_SIZE_U512 | 0x) diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index cb38af891387..fba5ff377b8b 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -587,7 +587,6 @@ static int sme_reg_to_region(struct vec_state_reg_region *region, const u64 zah_id_min = KVM_REG_ARM64_SME_ZAHREG(0, 0); const u64 zah_id_max = KVM_REG_ARM64_SME_ZAHREG(za_h_max - 1, SVE_NUM_SLICES - 1); - unsigned int reg_num; unsigned int reqoffset, reqlen; /* User-requested offset and length */ @@ -598,14 +597,24 @@ static int sme_reg_to_region(struct vec_state_reg_region *region, reg_num = (reg->id & SVE_REG_ID_MASK) >> SVE_REG_ID_SHIFT; if (reg->id >= zah_id_min && reg->id <= zah_id_max) { - /* ZA is exposed as SVE vectors ZA.H[n] */ if (!vcpu_has_sme(vcpu) || (reg->id & SVE_REG_SLICE_MASK) > 0) return -ENOENT; + /* ZA is exposed as SVE vectors ZA.H[n] */ reqoffset = ZA_SIG_ZAV_OFFSET(vq, reg_num) - ZA_SIG_REGS_OFFSET; reqlen = KVM_SVE_ZREG_SIZE; maxlen = SVE_SIG_ZREG_SIZE(vq); + } if (reg->id == KVM_REG_ARM64_SME_ZT_BASE) { + /* ZA is exposed as SVE vectors ZA.H[n] */ + if (!vcpu_has_sme2(vcpu) || + (reg->id & SVE_REG_SLICE_MASK) > 0 || + reg_num > 0) + return -ENOENT; + + /* ZT0 is stored after ZA */ + reqlen = KVM_REG_ARM64_SME_ZTREG_SIZE; + maxlen = KVM_REG_ARM64_SME_ZTREG_SIZE; } else { return -EINVAL; } -- 2.30.2
[PATCH RFC v2 20/22] KVM: arm64: Support SME version configuration via ID registers
As well as a substantial set of features which provide additional instructions there are also two current extensions which add new architectural state, SME2 (which adds ZT) and FA64 (which makes FFR valid in streaming mode SVE). Allow all of these to be configured through writes to the ID registers. At present the guest support for SME2 and FA64 does not use the values configured here pending clarity on the approach to be taken generally with regards to parsing supported features from ID registers. We always allocate state for the new architectural state which might be enabled if the host supports it, in the case of FFR this simplifies the already fiddly allocation and is needed when SVE is also supported. In the case of ZT the register is 64 bytes which is not completely trivial (though not so much relative to the other SME state) but it is not expected that there will be many practical users that want SME1 only so it is expected that guests with SME1 only would not be common enough to justify the complication of handling this. If this proves to be a problem we can improve things incrementally. Signed-off-by: Mark Brown --- arch/arm64/kvm/sys_regs.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index f908aa3fb606..1ea658615467 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1412,13 +1412,6 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu, val = read_sanitised_ftr_reg(id); switch (id) { - case SYS_ID_AA64PFR1_EL1: - if (!kvm_has_mte(vcpu->kvm)) - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_MTE); - - if (!vcpu_has_sme(vcpu)) - val &= ~ID_AA64PFR1_EL1_SME_MASK; - break; case SYS_ID_AA64ISAR1_EL1: if (!vcpu_has_ptrauth(vcpu)) val &= ~(ARM64_FEATURE_MASK(ID_AA64ISAR1_EL1_APA) | @@ -1582,6 +1575,20 @@ static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu, return val; } +static u64 read_sanitised_id_aa64pfr1_el1(struct kvm_vcpu *vcpu, + const struct sys_reg_desc *rd) +{ + u64 val = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1); + + if (!vcpu_has_sme(vcpu)) + val &= ~ID_AA64PFR1_EL1_SME_MASK; + + if (!kvm_has_mte(vcpu->kvm)) + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_MTE); + + return val; +} + #define ID_REG_LIMIT_FIELD_ENUM(val, reg, field, limit) \ ({\ u64 __f_val = FIELD_GET(reg##_##field##_MASK, val);\ @@ -2164,7 +2171,14 @@ static const struct sys_reg_desc sys_reg_descs[] = { ID_AA64PFR0_EL1_GIC | ID_AA64PFR0_EL1_AdvSIMD | ID_AA64PFR0_EL1_FP), }, - ID_SANITISED(ID_AA64PFR1_EL1), + { SYS_DESC(SYS_ID_AA64PFR1_EL1), + .access = access_id_reg, + .get_user = get_id_reg, + .set_user = set_id_reg, + .reset = read_sanitised_id_aa64pfr1_el1, + .val = ~(ID_AA64PFR1_EL1_MPAM_frac | + ID_AA64PFR1_EL1_RAS_frac | + ID_AA64PFR1_EL1_MTE), }, ID_UNALLOCATED(4,2), ID_UNALLOCATED(4,3), ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0), -- 2.30.2
[PATCH RFC v2 21/22] KVM: arm64: Provide userspace ABI for enabling SME
Now that everything else is in place allow userspace to enable SME with a vCPU flag KVM_ARM_VCPU_SME similar to that for SVE, finalized with the same KVM_ARM_VCPU_VEC/SVE flag used for SVE. As with SVE when when SME feature is enabled we default the vector length to the highest vector length that can be exposed to guests without them being able to observe inconsistencies between the set of supported vector lengths on processors, discovering this during host SME enumeration. When the vectors are finalised we allocate state for both SVE and SME. Currently SME2 and FA64 are enabled for the guest if supported by the host, once it is clear how we intend to handle parsing features from ID registers these features should be handled based on the configured ID registers. Signed-off-by: Mark Brown --- arch/arm64/include/asm/kvm_host.h | 3 +- arch/arm64/include/uapi/asm/kvm.h | 1 + arch/arm64/kernel/fpsimd.c| 28 + arch/arm64/kvm/arm.c | 7 +++ arch/arm64/kvm/reset.c| 120 +- include/uapi/linux/kvm.h | 1 + 6 files changed, 145 insertions(+), 15 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 6a5002ab8042..c054aee8160a 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -38,7 +38,7 @@ #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS -#define KVM_VCPU_MAX_FEATURES 7 +#define KVM_VCPU_MAX_FEATURES 8 #define KVM_VCPU_VALID_FEATURES(BIT(KVM_VCPU_MAX_FEATURES) - 1) #define KVM_REQ_SLEEP \ @@ -76,6 +76,7 @@ DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use); extern unsigned int __ro_after_init kvm_vec_max_vl[ARM64_VEC_MAX]; int __init kvm_arm_init_sve(void); +int __init kvm_arm_init_sme(void); u32 __attribute_const__ kvm_target_cpu(void); void kvm_reset_vcpu(struct kvm_vcpu *vcpu); diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 58640aeb88e4..0f5e678c3ef3 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -110,6 +110,7 @@ struct kvm_regs { #define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */ #define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */ #define KVM_ARM_VCPU_HAS_EL2 7 /* Support nested virtualization */ +#define KVM_ARM_VCPU_SME 8 /* enable SME for this CPU */ /* * An alias for _SVE since we finalize VL configuration for both SVE and SME diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index e6a4dd68f62a..05e806a580f0 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -1307,6 +1307,8 @@ void __init sme_setup(void) { struct vl_info *info = &vl_info[ARM64_VEC_SME]; int min_bit, max_bit; + DECLARE_BITMAP(tmp_map, SVE_VQ_MAX); + unsigned long b; if (!cpus_have_cap(ARM64_SME)) return; @@ -1342,6 +1344,32 @@ void __init sme_setup(void) info->max_vl); pr_info("SME: default vector length %u bytes per vector\n", get_sme_default_vl()); + + /* +* KVM can't cope with any mismatch in supported VLs between +* CPUs, detect any inconsistencies. Unlike SVE it is +* architecturally possible to end up with no VLs. +*/ + bitmap_andnot(tmp_map, info->vq_partial_map, info->vq_map, + SVE_VQ_MAX); + + b = find_last_bit(tmp_map, SVE_VQ_MAX); + if (b >= SVE_VQ_MAX) + /* No non-virtualisable VLs found */ + info->max_virtualisable_vl = SVE_VQ_MAX; + else if (WARN_ON(b == SVE_VQ_MAX - 1)) + /* No virtualisable VLs? Architecturally possible... */ + info->max_virtualisable_vl = 0; + else /* b + 1 < SVE_VQ_MAX */ + info->max_virtualisable_vl = sve_vl_from_vq(__bit_to_vq(b + 1)); + + if (info->max_virtualisable_vl > info->max_vl) + info->max_virtualisable_vl = info->max_vl; + + /* KVM decides whether to support mismatched systems. Just warn here: */ + if (sve_max_virtualisable_vl() < sve_max_vl()) + pr_warn("%s: unvirtualisable vector lengths present\n", + info->name); } #endif /* CONFIG_ARM64_SME */ diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index aa7e2031263c..baa4a8074aaf 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -305,6 +305,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_ARM_SVE: r = system_supports_sve(); break; + case KVM_CAP_ARM_SME: + r = system_supports_sme(); + break; case KVM_CAP_ARM_PTRAUTH_ADDRESS: case KVM_CAP_ARM_PTRAUTH_GENERIC: r = system_has_full_ptr_auth(); @@ -2559,6 +2562,10 @@ static __init int kvm_arm_init(void) i
[PATCH RFC v2 22/22] KVM: arm64: selftests: Add SME system registers to get-reg-list
SME adds a number of new system registers, update get-reg-list to check for them based on the visibility of SME. Signed-off-by: Mark Brown --- tools/testing/selftests/kvm/aarch64/get-reg-list.c | 32 +- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c index 709d7d721760..a1c2853388b6 100644 --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c @@ -23,6 +23,18 @@ struct feature_id_reg { }; static struct feature_id_reg feat_id_regs[] = { + { + ARM64_SYS_REG(3, 0, 1, 2, 4), /* SMPRI_EL1 */ + ARM64_SYS_REG(3, 0, 0, 4, 1), /* ID_AA64PFR1_EL1 */ + 24, + 1 + }, + { + ARM64_SYS_REG(3, 0, 1, 2, 6), /* SMCR_EL1 */ + ARM64_SYS_REG(3, 0, 0, 4, 1), /* ID_AA64PFR1_EL1 */ + 24, + 1 + }, { ARM64_SYS_REG(3, 0, 2, 0, 3), /* TCR2_EL1 */ ARM64_SYS_REG(3, 0, 0, 7, 3), /* ID_AA64MMFR3_EL1 */ @@ -40,7 +52,25 @@ static struct feature_id_reg feat_id_regs[] = { ARM64_SYS_REG(3, 0, 0, 7, 3), /* ID_AA64MMFR3_EL1 */ 4, 1 - } + }, + { + ARM64_SYS_REG(3, 1, 0, 0, 6), /* SMIDR_EL1 */ + ARM64_SYS_REG(3, 0, 0, 4, 1), /* ID_AA64PFR1_EL1 */ + 24, + 1 + }, + { + ARM64_SYS_REG(3, 3, 4, 2, 2), /* SVCR */ + ARM64_SYS_REG(3, 0, 0, 4, 1), /* ID_AA64PFR1_EL1 */ + 24, + 1 + }, + { + ARM64_SYS_REG(3, 3, 13, 0, 5), /* TPIDR2_EL0 */ + ARM64_SYS_REG(3, 0, 0, 4, 1), /* ID_AA64PFR1_EL1 */ + 24, + 1 + }, }; bool filter_reg(__u64 reg) -- 2.30.2
Re: [PATCH v3] tracing/selftests: Add ownership modification tests for eventfs
On Fri, 22 Dec 2023 09:15:42 -0700 Shuah Khan wrote: > On 12/21/23 19:16, Steven Rostedt wrote: > > > > Shuah, > > > > This patch has no dependencies. You can take it through your tree for the > > next merge window if you want. If not, I can take it. > > > Tried to apply this and seeing a couple of issues: > > -- missing SPDX Hmm, I copied this from the snapshot.tc. I guess there's several test files that are missing SPDX. That should probably be fixed. > -- this file has executable permission set unlike the existing > .tc files in the same directory Oh, I forgot to disable that. When developing a new test I make it standalone as it's easier to test the test, and then copy the file into the directory. I'll fix the above and send a v4. Thanks, -- Steve
[PATCH v4] tracing/selftests: Add ownership modification tests for eventfs
From: "Steven Rostedt (Google)" As there were bugs found with the ownership of eventfs dynamic file creation. Add a test to test it. It will remount tracefs with a different gid and check the ownership of the eventfs directory, as well as the system and event directories. It will also check the event file directories. It then does a chgrp on each of these as well to see if they all get updated as expected. Then it remounts the tracefs file system back to the original group and makes sure that all the updated files and directories were reset back to the original ownership. It does the same for instances that change the ownership of he instance directory. Note, because the uid is not reset by a remount, it is tested for every file by switching it to a new owner and then back again. Acked-by: Masami Hiramatsu (Google) Tested-by: Masami Hiramatsu (Google) Signed-off-by: Steven Rostedt (Google) --- Changes since v3: https://lore.kernel.org/linux-trace-kernel/20231221211229.13398...@gandalf.local.home - Added missing SPDX and removed exec permission from file (Shuah Khan) .../ftrace/test.d/00basic/test_ownership.tc | 114 ++ 1 file changed, 114 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc new file mode 100644 index ..add7d5bf585d --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc @@ -0,0 +1,114 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: Test file and directory owership changes for eventfs + +original_group=`stat -c "%g" .` +original_owner=`stat -c "%u" .` + +mount_point=`stat -c '%m' .` +mount_options=`mount | grep "$mount_point" | sed -e 's/.*(\(.*\)).*/\1/'` + +# find another owner and group that is not the original +other_group=`tac /etc/group | grep -v ":$original_group:" | head -1 | cut -d: -f3` +other_owner=`tac /etc/passwd | grep -v ":$original_owner:" | head -1 | cut -d: -f3` + +# Remove any group ownership already +new_options=`echo "$mount_options" | sed -e "s/gid=[0-9]*/gid=$other_group/"` + +if [ "$new_options" = "$mount_options" ]; then + new_options="$mount_options,gid=$other_group" + mount_options="$mount_options,gid=$original_group" +fi + +canary="events/timer events/timer/timer_cancel events/timer/timer_cancel/format" + +test() { + file=$1 + test_group=$2 + + owner=`stat -c "%u" $file` + group=`stat -c "%g" $file` + + echo "testing $file $owner=$original_owner and $group=$test_group" + if [ $owner -ne $original_owner ]; then + exit_fail + fi + if [ $group -ne $test_group ]; then + exit_fail + fi + + # Note, the remount does not update ownership so test going to and from owner + echo "test owner $file to $other_owner" + chown $other_owner $file + owner=`stat -c "%u" $file` + if [ $owner -ne $other_owner ]; then + exit_fail + fi + + chown $original_owner $file + owner=`stat -c "%u" $file` + if [ $owner -ne $original_owner ]; then + exit_fail + fi + +} + +run_tests() { + for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do + test "$d" $other_group + done + + chgrp $original_group events + test "events" $original_group + for d in "." "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do + test "$d" $other_group + done + + chgrp $original_group events/sched + test "events/sched" $original_group + for d in "." "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do + test "$d" $other_group + done + + chgrp $original_group events/sched/sched_switch + test "events/sched/sched_switch" $original_group + for d in "." "events/sched/sched_switch/enable" $canary; do + test "$d" $other_group + done + + chgrp $original_group events/sched/sched_switch/enable + test "events/sched/sched_switch/enable" $original_group + for d in "." $canary; do + test "$d" $other_group + done +} + +mount -o remount,"$new_options" . + +run_tests + +mount -o remount,"$mount_options" . + +for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do + test "$d" $original_group +done + +# check instances as well + +chgrp $other_group instances + +instance="$(mktemp -u test-XX)" + +mkdir instances/$instance + +cd instances/$instance + +run_tests + +cd ../.. + +rmdir instances/$instance + +chgrp $original_group instances + +exit 0 -- 2.42.0
Re: [PATCH v3] tracing/selftests: Add ownership modification tests for eventfs
On Fri, 22 Dec 2023 11:28:31 -0500 Steven Rostedt wrote: > > -- this file has executable permission set unlike the existing > > .tc files in the same directory > > Oh, I forgot to disable that. When developing a new test I make it > standalone as it's easier to test the test, and then copy the file into the > directory. > I noticed that I did the same for trace_marker.tc that's in my for-next queue. Instead of rebasing, as there's other commits on top of it, I'm just going to push this to my for-next queue. -- Steve From 26547691107eda45b0f20ee79bad19bbe5fcbfd7 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Google)" Date: Fri, 22 Dec 2023 11:35:22 -0500 Subject: [PATCH] tracing/selftests: Remove exec permissions from trace_marker.tc test The tests in the selftests should not have the exec permissions set. The trace_marker.tc test accidentally was committed with the exec permission. Set the permission to that file back to just read/write. No functional nor code changes. Link: https://lore.kernel.org/linux-trace-kernel/20231222112831.4c7fa...@gandalf.local.home/ Signed-off-by: Steven Rostedt (Google) --- tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc old mode 100755 new mode 100644 -- 2.42.0
Re: [PATCH v4] tracing/selftests: Add ownership modification tests for eventfs
On 12/22/23 09:34, Steven Rostedt wrote: From: "Steven Rostedt (Google)" As there were bugs found with the ownership of eventfs dynamic file creation. Add a test to test it. It will remount tracefs with a different gid and check the ownership of the eventfs directory, as well as the system and event directories. It will also check the event file directories. It then does a chgrp on each of these as well to see if they all get updated as expected. Then it remounts the tracefs file system back to the original group and makes sure that all the updated files and directories were reset back to the original ownership. It does the same for instances that change the ownership of he instance directory. Note, because the uid is not reset by a remount, it is tested for every file by switching it to a new owner and then back again. Acked-by: Masami Hiramatsu (Google) Tested-by: Masami Hiramatsu (Google) Signed-off-by: Steven Rostedt (Google) --- Changes since v3: https://lore.kernel.org/linux-trace-kernel/20231221211229.13398...@gandalf.local.home - Added missing SPDX and removed exec permission from file (Shuah Khan) Thank you. Applied to linux-kselftest next branch for Linux 6.8-rc1 thanks, -- Shuah
Re: livepatch: Move modules to selftests and add a new test
Hello: This conversation is now tracked by Kernel.org Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218303 There is no need to do anything else, just keep talking. -- Deet-doot-dot, I am a bot. Kernel.org Bugzilla (peebz 0.1)
Re: livepatch: Move modules to selftests and add a new test
mricon writes via Kernel.org Bugzilla: Oh, hmm... I uncovered a bug in bugbot -- this was a different subthread that I started about b4 and supporting --dry-run-to that Marcos was looking for. Not sure how to fix bugbot yet, but I'm going to close this bug for now. Sorry about the noise, everyone. -K View: https://bugzilla.kernel.org/show_bug.cgi?id=218303#c6 You can reply to this message to join the discussion. -- Deet-doot-dot, I am a bot. Kernel.org Bugzilla (peebz 0.1)
Re: [PATCH][next] selftests/net: Fix various spelling mistakes in TCP-AO tests
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller : On Mon, 18 Dec 2023 13:30:22 + you wrote: > There are a handful of spelling mistakes in test messages in the > TCP-AIO selftests. Fix these. > > Signed-off-by: Colin Ian King > --- > tools/testing/selftests/net/tcp_ao/connect-deny.c | 2 +- > tools/testing/selftests/net/tcp_ao/lib/proc.c | 4 ++-- > tools/testing/selftests/net/tcp_ao/setsockopt-closed.c | 2 +- > tools/testing/selftests/net/tcp_ao/unsigned-md5.c | 2 +- > 4 files changed, 5 insertions(+), 5 deletions(-) Here is the summary with links: - [next] selftests/net: Fix various spelling mistakes in TCP-AO tests https://git.kernel.org/netdev/net-next/c/67f440c05dd2 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH net-next] selftest/tcp-ao: Rectify out-of-tree build
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller : On Tue, 19 Dec 2023 02:03:05 + you wrote: > Trivial fix for out-of-tree build that I wasn't testing previously: > > 1. Create a directory for library object files, fixes: > > gcc lib/kconfig.c -Wall -O2 -g -D_GNU_SOURCE -fno-strict-aliasing -I > > ../../../../../usr/include/ -iquote /tmp/kselftest/kselftest/net/tcp_ao/lib > > -I ../../../../include/ -o > > /tmp/kselftest/kselftest/net/tcp_ao/lib/kconfig.o -c > > Assembler messages: > > Fatal error: can't create > > /tmp/kselftest/kselftest/net/tcp_ao/lib/kconfig.o: No such file or directory > > make[1]: *** [Makefile:46: > > /tmp/kselftest/kselftest/net/tcp_ao/lib/kconfig.o] Error 1 > > [...] Here is the summary with links: - [net-next] selftest/tcp-ao: Rectify out-of-tree build https://git.kernel.org/netdev/net-next/c/826eb9bcc184 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH net-next 0/8] Convert net selftests to run in unique namespace (last part)
Hello: This series was applied to netdev/net-next.git (main) by David S. Miller : On Tue, 19 Dec 2023 17:48:48 +0800 you wrote: > Here is the last part of converting net selftests to run in unique namespace. > This part converts all left tests. After the conversion, we can run the net > sleftests in parallel. e.g. > > # ./run_kselftest.sh -n -t net:reuseport_bpf > TAP version 13 > 1..1 > # selftests: net: reuseport_bpf > ok 1 selftests: net: reuseport_bpf > mod 10... > # Socket 0: 0 > # Socket 1: 1 > ... > # Socket 4: 19 > # Testing filter add without bind... > # SUCCESS > > [...] Here is the summary with links: - [net-next,1/8] selftests/net: convert gre_gso.sh to run it in unique namespace https://git.kernel.org/netdev/net-next/c/b84c2faeb986 - [net-next,2/8] selftests/net: convert netns-name.sh to run it in unique namespace https://git.kernel.org/netdev/net-next/c/f6476dedf08d - [net-next,3/8] selftests/net: convert rtnetlink.sh to run it in unique namespace https://git.kernel.org/netdev/net-next/c/d3b6b1116127 - [net-next,4/8] selftests/net: convert stress_reuseport_listen.sh to run it in unique namespace https://git.kernel.org/netdev/net-next/c/098f1ce08bbc - [net-next,5/8] selftests/net: convert xfrm_policy.sh to run it in unique namespace https://git.kernel.org/netdev/net-next/c/976fd1fe4f58 - [net-next,6/8] selftests/net: use unique netns name for setup_loopback.sh setup_veth.sh https://git.kernel.org/netdev/net-next/c/4416c5f53b43 - [net-next,7/8] selftests/net: convert pmtu.sh to run it in unique namespace https://git.kernel.org/netdev/net-next/c/378f082eaf37 - [net-next,8/8] kselftest/runner.sh: add netns support https://git.kernel.org/netdev/net-next/c/9d0b4ad82d61 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] selftests/net: remove unneeded semicolon
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller : On Tue, 19 Dec 2023 13:54:04 +0800 you wrote: > No functional modification involved. > > ./tools/testing/selftests/net/tcp_ao/setsockopt-closed.c:121:2-3: Unneeded > semicolon. > > Reported-by: Abaci Robot > Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7771 > Signed-off-by: Jiapeng Chong > > [...] Here is the summary with links: - selftests/net: remove unneeded semicolon https://git.kernel.org/netdev/net-next/c/6530b29f77c8 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH net-next v11 00/10] net: ethernet: am65-cpsw: Add mqprio, frame preemption & coalescing
Hello: This series was applied to netdev/net-next.git (main) by David S. Miller : On Tue, 19 Dec 2023 12:57:55 +0200 you wrote: > Hi, > > This series adds mqprio qdisc offload in channel mode, > Frame Preemption MAC merge support and RX/TX coalesing > for AM65 CPSW driver. > > In v11 following changes were made > - Fix patch "net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in > channel mode" > by including units.h > > [...] Here is the summary with links: - [net-next,v11,01/10] selftests: forwarding: ethtool_mm: support devices with higher rx-min-frag-size https://git.kernel.org/netdev/net-next/c/2491d66ae66c - [net-next,v11,02/10] selftests: forwarding: ethtool_mm: fall back to aggregate if device does not report pMAC stats https://git.kernel.org/netdev/net-next/c/c8659bd9d1c0 - [net-next,v11,03/10] net: ethernet: am65-cpsw: Build am65-cpsw-qos only if required https://git.kernel.org/netdev/net-next/c/c92b1321bbf3 - [net-next,v11,04/10] net: ethernet: am65-cpsw: Rename TI_AM65_CPSW_TAS to TI_AM65_CPSW_QOS https://git.kernel.org/netdev/net-next/c/d0f9535b3182 - [net-next,v11,05/10] net: ethernet: am65-cpsw: cleanup TAPRIO handling https://git.kernel.org/netdev/net-next/c/5db81bdc486d - [net-next,v11,06/10] net: ethernet: ti: am65-cpsw: Move code to avoid forward declaration https://git.kernel.org/netdev/net-next/c/1374841ad477 - [net-next,v11,07/10] net: ethernet: am65-cpsw: Move register definitions to header file https://git.kernel.org/netdev/net-next/c/8f5a75610698 - [net-next,v11,08/10] net: ethernet: ti: am65-cpsw: add mqprio qdisc offload in channel mode https://git.kernel.org/netdev/net-next/c/bc8d62e16ec2 - [net-next,v11,09/10] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support https://git.kernel.org/netdev/net-next/c/49a2eb906824 - [net-next,v11,10/10] net: ethernet: ti: am65-cpsw: add sw tx/rx irq coalescing based on hrtimers https://git.kernel.org/netdev/net-next/c/e4918f9d4882 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH] kunit: Fix some comments which were mistakenly kerneldoc
The KUnit device helpers are documented with kerneldoc in their header file, but also have short comments over their implementation. These were mistakenly formatted as kerneldoc comments, even though they're not valid kerneldoc. It shouldn't cause any serious problems -- this file isn't included in the docs -- but it could be confusing, and causes warnings. Remove the extra '*' so that these aren't treated as kerneldoc. Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202312181920.h4epah20-...@intel.com/ Signed-off-by: David Gow --- lib/kunit/device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/kunit/device.c b/lib/kunit/device.c index 1db4305b615a..f5371287b375 100644 --- a/lib/kunit/device.c +++ b/lib/kunit/device.c @@ -60,7 +60,7 @@ static void kunit_device_release(struct device *d) kfree(to_kunit_device(d)); } -/** +/* * Create and register a KUnit-managed struct device_driver on the kunit_bus. * Returns an error pointer on failure. */ @@ -124,7 +124,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test, return kunit_dev; } -/** +/* * Create and register a new KUnit-managed device, using the user-supplied device_driver. * On failure, returns an error pointer. */ @@ -141,7 +141,7 @@ struct device *kunit_device_register_with_driver(struct kunit *test, } EXPORT_SYMBOL_GPL(kunit_device_register_with_driver); -/** +/* * Create and register a new KUnit-managed device, including a matching device_driver. * On failure, returns an error pointer. */ -- 2.43.0.472.g3155946c3a-goog
Re: [PATCH] kunit: Fix some comments which were mistakenly kerneldoc
On 12/22/23 20:18, David Gow wrote: > The KUnit device helpers are documented with kerneldoc in their header > file, but also have short comments over their implementation. These were > mistakenly formatted as kerneldoc comments, even though they're not > valid kerneldoc. It shouldn't cause any serious problems -- this file > isn't included in the docs -- but it could be confusing, and causes > warnings. > > Remove the extra '*' so that these aren't treated as kerneldoc. > > Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202312181920.h4epah20-...@intel.com/ > Signed-off-by: David Gow Reviewed-by: Randy Dunlap Thanks. > --- > lib/kunit/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/kunit/device.c b/lib/kunit/device.c > index 1db4305b615a..f5371287b375 100644 > --- a/lib/kunit/device.c > +++ b/lib/kunit/device.c > @@ -60,7 +60,7 @@ static void kunit_device_release(struct device *d) > kfree(to_kunit_device(d)); > } > > -/** > +/* > * Create and register a KUnit-managed struct device_driver on the kunit_bus. > * Returns an error pointer on failure. > */ > @@ -124,7 +124,7 @@ static struct kunit_device > *kunit_device_register_internal(struct kunit *test, > return kunit_dev; > } > > -/** > +/* > * Create and register a new KUnit-managed device, using the user-supplied > device_driver. > * On failure, returns an error pointer. > */ > @@ -141,7 +141,7 @@ struct device *kunit_device_register_with_driver(struct > kunit *test, > } > EXPORT_SYMBOL_GPL(kunit_device_register_with_driver); > > -/** > +/* > * Create and register a new KUnit-managed device, including a matching > device_driver. > * On failure, returns an error pointer. > */ -- #Randy https://people.kernel.org/tglx/notes-about-netiquette https://subspace.kernel.org/etiquette.html
Re: [PATCH 1/6] kunit: add parameter generation macro using description from array
On Fri, 22 Dec 2023 at 23:09, Benjamin Berg wrote: > > On Fri, 2023-12-22 at 18:02 +0800, David Gow wrote: > > On Wed, 20 Dec 2023 at 23:20, wrote: > > > > > > From: Benjamin Berg > > > > > > The existing KUNIT_ARRAY_PARAM macro requires a separate function > > > to > > > get the description. However, in a lot of cases the description can > > > just be copied directly from the array. Add a second macro that > > > avoids having to write a static function just for a single strscpy. > > > > > > Signed-off-by: Benjamin Berg > > > --- > > > > I'm generally pretty happy with this, though note the checkpatch > > warning below. > > > > There was some discussion at plumbers about expanding the > > parameterised test APIs, so we may need to adjust the implementation > > of this down the line, but I don't think that'll happen for a while, > > so don't worry. > > > > With the warnings fixed, this is: > > I think the checkpatch warning is a false positive. It seems to confuse > the * as a multiplication due to typeof() looking like a function call > rather than a variable declaration. > > Benjamin > Ah, yeah: this appears to be due to checkpatch not handling nested () within a typeof(). Reviewed-by: David Gow Cheers, -- David smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH net-next 3/3] selftests/net: fix GRO coalesce test and add ext
Hi Richard, kernel test robot noticed the following build warnings: [auto build test WARNING on net-next/main] url: https://github.com/intel-lab-lkp/linux/commits/Richard-Gobert/net-gso-add-HBH-extension-header-offload-support/20231222-172059 base: net-next/main patch link: https://lore.kernel.org/r/641157c0-f224-4910-874d-7906a48d914b%40gmail.com patch subject: [PATCH net-next 3/3] selftests/net: fix GRO coalesce test and add ext compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231223/202312231344.swss5pik-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202312231344.swss5pik-...@intel.com/ All warnings (new ones prefixed by >>): gro.c: In function 'add_ipv6_exthdr': >> gro.c:600:13: warning: variable 'opt_len' set but not used >> [-Wunused-but-set-variable] 600 | int opt_len; | ^~~ -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki