Re: [PATCH v4 4/5] selftests/resctrl: Add resource_info_file_exists()
Hi Reinette! On 2024-02-05 at 20:17:48 -0800, Reinette Chatre wrote: >Hi Maciej, > >On 2/5/2024 4:08 AM, Maciej Wieczor-Retman wrote: >> Feature checking done by resctrl_mon_feature_exists() covers features >> represented by the feature name presence inside the 'mon_features' file >> in /sys/fs/resctrl/info/L3_MON directory. There exists a different way >> to represent feature support and that is by the presence of 0 or 1 in a >> single file in the info/resource directory. In this case the filename >> represents what feature support is being indicated. >> >> Add a generic function to check file presence in the >> /sys/fs/resctrl/info/ directory. >> >> Signed-off-by: Maciej Wieczor-Retman >> Reviewed-by: Ilpo Järvinen >> --- >> Changelog v4: >> - Remove unnecessary new lines. >> - Change 'feature' -> 'file' to keep things generic. (Reinette) >> - Add Ilpo's reviewed-by tag. >> >> Changelog v3: >> - Split off the new function into this patch. (Reinette) >> >> Changelog v2: >> - Add this patch. >> >> tools/testing/selftests/resctrl/resctrl.h | 1 + >> tools/testing/selftests/resctrl/resctrlfs.c | 25 + >> 2 files changed, 26 insertions(+) >> >> diff --git a/tools/testing/selftests/resctrl/resctrl.h >> b/tools/testing/selftests/resctrl/resctrl.h >> index 4603b215b97e..2b9a3d0570c7 100644 >> --- a/tools/testing/selftests/resctrl/resctrl.h >> +++ b/tools/testing/selftests/resctrl/resctrl.h >> @@ -138,6 +138,7 @@ int umount_resctrlfs(void); >> int validate_bw_report_request(char *bw_report); >> bool resctrl_resource_exists(const char *resource); >> bool resctrl_mon_feature_exists(const char *feature); >> +bool resource_info_file_exists(const char *resource, const char *feature); > >One stray "feature" usage. Thank you for catching that! > >> bool test_resource_feature_check(const struct resctrl_test *test); >> char *fgrep(FILE *inf, const char *str); >> int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity); >> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c >> b/tools/testing/selftests/resctrl/resctrlfs.c >> index 0cfec8bb23fd..6a3082ca58b5 100644 >> --- a/tools/testing/selftests/resctrl/resctrlfs.c >> +++ b/tools/testing/selftests/resctrl/resctrlfs.c >> @@ -760,6 +760,31 @@ bool resctrl_mon_feature_exists(const char *feature) >> return !!res; >> } >> >> +/* >> + * resource_info_file_exists - Check if a file is present inside >> + * /sys/fs/resctrl/info/RESOURCE. >> + * @resource: Required resource (Eg: MB, L3, L2, etc.) >> + * @file: Required file. >> + * >> + * Return: True if the file exists, else false. > >How about "True if /sys/fs/resctrl/info/@resource/@file exists, else false"? Sure. I was wondering what the best format of paths in the function comments could be and that does look very sensible. I'll redo the other paths in the comments of this series for consistency. > >Reinette -- Kind regards Maciej Wieczór-Retman
[broonie-misc:kselftest-seccomp-benchmark-ktap] [kselftest/seccomp] 626fa92237: kernel-selftests.seccomp.seccomp_benchmark.fail
Hello,
kernel test robot noticed "kernel-selftests.seccomp.seccomp_benchmark.fail" on:
commit: 626fa9223749db85f03678573dd49ba2c7b6cd8b ("kselftest/seccomp: Report
each expectation we assert as a KTAP test")
https://git.kernel.org/cgit/linux/kernel/git/broonie/misc.git
kselftest-seccomp-benchmark-ktap
in testcase: kernel-selftests
version: kernel-selftests-x86_64-60acb023-1_20230329
with following parameters:
group: group-s
compiler: gcc-12
test machine: 36 threads 1 sockets Intel(R) Core(TM) i9-10980XE CPU @ 3.00GHz
(Cascade Lake) with 32G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
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-lkp/202402061002.3a8722fd-oliver.s...@intel.com
# timeout set to 120
# selftests: seccomp: seccomp_benchmark
# TAP version 13
# 1..7
# # Running on:
# # Linux lkp-csl-d01 6.8.0-rc1-3-g626fa9223749 #1 SMP PREEMPT_DYNAMIC Wed
Jan 31 08:33:40 CST 2024 x86_64 GNU/Linux
# # Current BPF sysctl settings:
# # /proc/sys/net/core/bpf_jit_enable:1
# # /proc/sys/net/core/bpf_jit_harden:0
# # Calibrating sample size for 15 seconds worth of syscalls ...
# # Benchmarking 36800370 syscalls...
# # 15.443201110 - 1.042366576 = 14400834534 (14.4s)
# # getpid native: 391 ns
# # 31.586833659 - 15.443583738 = 16143249921 (16.1s)
# # getpid RET_ALLOW 1 filter (bitmap): 438 ns
# # 47.494976754 - 31.587621280 = 15907355474 (15.9s)
# # getpid RET_ALLOW 2 filters (bitmap): 432 ns
# # 66.262898246 - 47.495560365 = 18767337881 (18.8s)
# # getpid RET_ALLOW 3 filters (full): 509 ns
# # 86.089613909 - 66.263287445 = 19826326464 (19.8s)
# # getpid RET_ALLOW 4 filters (full): 538 ns
# # Estimated total seccomp overhead for 1 bitmapped filter: 47 ns
# # Estimated total seccomp overhead for 2 bitmapped filters: 41 ns
# # Estimated total seccomp overhead for 3 full filters: 118 ns
# # Estimated total seccomp overhead for 4 full filters: 147 ns
# # Estimated seccomp entry overhead: 53 ns
# # Estimated seccomp per-filter overhead (last 2 diff): 29 ns
# # Estimated seccomp per-filter overhead (filters / 4): 23 ns
# # Expectations:
# # native ≤ 1 bitmap (391 ≤ 438): ✔️
# ok 1 native ≤ 1 bitmap
# # native ≤ 1 filter (391 ≤ 509): ✔️
# ok 2 native ≤ 1 filter
# # per-filter (last 2 diff) ≈ per-filter (filters / 4) (29 ≈ 23): ❌
# not ok 3 per-filter (last 2 diff) ≈ per-filter (filters / 4)
# # 1 bitmapped ≈ 2 bitmapped (47 ≈ 41): ❌
# not ok 4 1 bitmapped ≈ 2 bitmapped
# # Skipping constant action bitmap expectations: they appear unsupported.
# ok 5 # SKIP entry ≈ 1 bitmapped
# ok 6 # SKIP entry ≈ 2 bitmapped
# ok 7 # SKIP native + entry + (per filter * 4) ≈ 4 filters total
# # Saw unexpected benchmark result. Try running again with more samples?
# # Totals: pass:2 fail:2 xfail:0 xpass:0 skip:3 error:0
not ok 2 selftests: seccomp: seccomp_benchmark # exit=1
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240206/202402061002.3a8722fd-oliver.s...@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT test
Hi Maciej, On 2/5/2024 4:08 AM, Maciej Wieczor-Retman wrote: > Add tests for both L2 and L3 CAT to verify the return values > generated by writing non-contiguous CBMs don't contradict the > reported non-contiguous support information. > > Use a logical XOR to confirm return value of write_schemata() and > non-contiguous CBMs support information match. > > Signed-off-by: Maciej Wieczor-Retman > --- Thank you very much. Reviewed-by: Reinette Chatre Reinette
Re: [PATCH v4 4/5] selftests/resctrl: Add resource_info_file_exists()
Hi Maciej, On 2/5/2024 4:08 AM, Maciej Wieczor-Retman wrote: > Feature checking done by resctrl_mon_feature_exists() covers features > represented by the feature name presence inside the 'mon_features' file > in /sys/fs/resctrl/info/L3_MON directory. There exists a different way > to represent feature support and that is by the presence of 0 or 1 in a > single file in the info/resource directory. In this case the filename > represents what feature support is being indicated. > > Add a generic function to check file presence in the > /sys/fs/resctrl/info/ directory. > > Signed-off-by: Maciej Wieczor-Retman > Reviewed-by: Ilpo Järvinen > --- > Changelog v4: > - Remove unnecessary new lines. > - Change 'feature' -> 'file' to keep things generic. (Reinette) > - Add Ilpo's reviewed-by tag. > > Changelog v3: > - Split off the new function into this patch. (Reinette) > > Changelog v2: > - Add this patch. > > tools/testing/selftests/resctrl/resctrl.h | 1 + > tools/testing/selftests/resctrl/resctrlfs.c | 25 + > 2 files changed, 26 insertions(+) > > diff --git a/tools/testing/selftests/resctrl/resctrl.h > b/tools/testing/selftests/resctrl/resctrl.h > index 4603b215b97e..2b9a3d0570c7 100644 > --- a/tools/testing/selftests/resctrl/resctrl.h > +++ b/tools/testing/selftests/resctrl/resctrl.h > @@ -138,6 +138,7 @@ int umount_resctrlfs(void); > int validate_bw_report_request(char *bw_report); > bool resctrl_resource_exists(const char *resource); > bool resctrl_mon_feature_exists(const char *feature); > +bool resource_info_file_exists(const char *resource, const char *feature); One stray "feature" usage. > bool test_resource_feature_check(const struct resctrl_test *test); > char *fgrep(FILE *inf, const char *str); > int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity); > diff --git a/tools/testing/selftests/resctrl/resctrlfs.c > b/tools/testing/selftests/resctrl/resctrlfs.c > index 0cfec8bb23fd..6a3082ca58b5 100644 > --- a/tools/testing/selftests/resctrl/resctrlfs.c > +++ b/tools/testing/selftests/resctrl/resctrlfs.c > @@ -760,6 +760,31 @@ bool resctrl_mon_feature_exists(const char *feature) > return !!res; > } > > +/* > + * resource_info_file_exists - Check if a file is present inside > + * /sys/fs/resctrl/info/RESOURCE. > + * @resource:Required resource (Eg: MB, L3, L2, etc.) > + * @file:Required file. > + * > + * Return: True if the file exists, else false. How about "True if /sys/fs/resctrl/info/@resource/@file exists, else false"? Reinette
Re: [PATCH v4 3/5] selftests/resctrl: Split validate_resctrl_feature_request()
On 2/5/2024 5:24 AM, Maciej Wieczor-Retman wrote:
> On 2024-02-05 at 14:41:30 +0200, Ilpo Järvinen wrote:
>> On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
>>
>>> validate_resctrl_feature_request() is used to test both if a resource is
>>> present in the info directory, and if a passed monitoring feature is
>>> present in the mon_features file.
>>>
>>> Refactor validate_resctrl_feature_request() into two smaller functions
>>> that each accomplish one check to give feature checking more
>>> granularity:
>>> - Resource directory presence in the /sys/fs/resctrl/info directory.
>>> - Feature name presence in the /sys/fs/resctrl/info/L3_MON/mon_features
>>> file.
>>>
>>> Signed-off-by: Maciej Wieczor-Retman
>>> ---
>>> Changelog v4:
>>> - Roll back to using test_resource_feature_check() for CMT and MBA.
>>> (Ilpo).
>>>
>>> Changelog v3:
>>> - Move new function to a separate patch. (Reinette)
>>> - Rewrite resctrl_mon_feature_exists() only for L3_MON.
>>>
>>> Changelog v2:
>>> - Add this patch.
>>>
>>> tools/testing/selftests/resctrl/cmt_test.c | 2 +-
>>> tools/testing/selftests/resctrl/mba_test.c | 2 +-
>>> tools/testing/selftests/resctrl/mbm_test.c | 6 ++--
>>> tools/testing/selftests/resctrl/resctrl.h | 3 +-
>>> tools/testing/selftests/resctrl/resctrlfs.c | 33 +
>>> 5 files changed, 28 insertions(+), 18 deletions(-)
>>>
>>
>>> diff --git a/tools/testing/selftests/resctrl/cmt_test.c
>>> b/tools/testing/selftests/resctrl/cmt_test.c
>>> index dd5ca343c469..c1157917a814 100644
>>> --- a/tools/testing/selftests/resctrl/cmt_test.c
>>> +++ b/tools/testing/selftests/resctrl/cmt_test.c
>>> @@ -170,7 +170,7 @@ static int cmt_run_test(const struct resctrl_test
>>> *test, const struct user_param
>>> static bool cmt_feature_check(const struct resctrl_test *test)
>>> {
>>> return test_resource_feature_check(test) &&
>>> - validate_resctrl_feature_request("L3_MON", "llc_occupancy");
>>> + resctrl_resource_exists("L3");
>>
>> This not correctly transformed.
>
> Oops, sorry, I'll fix it for the next version.
>
>>
>>> +/*
>>> + * resctrl_mon_feature_exists - Check if requested monitoring L3_MON
>>> feature is valid.
>>> + * @feature: Required monitor feature (in mon_features file).
>>> + *
>>> + * Return: True if the feature is supported, else false.
>>> + */
>>> +bool resctrl_mon_feature_exists(const char *feature)
>>> +{
>>> + char *res;
>>> + FILE *inf;
>>> +
>>> if (!feature)
>>> - return true;
>>> + return false;
>>>
>>> - snprintf(res_path, sizeof(res_path), "%s/%s/mon_features", INFO_PATH,
>>> resource);
>>> - inf = fopen(res_path, "r");
>>> + inf = fopen("/sys/fs/resctrl/info/L3_MON/mon_features", "r");
>>
>> This became less generic? Could there be other MON resource besides L3
>> one? Perhaps there aren't today but why remove the ability give it as a
>> parameter?
This does make the function less generic but the benefit is that four copies of
the same hardcoded parameter is no longer needed. In my opinion this patch thus
makes the code cleaner but it is not a requirement that I will use to hold this
series back.
> During v2 discussion [1] Reinette made me realize this functionality only
> interfaces with L3_MON/mon_features file and the 'resource' parameter isn't
> needed. The 'mon_features' file is only mentioned for L3_MON and I don't know
> of
> any plans for other MON resources so I assumed it doesn't need to be generic.
>
> But sure, I can make it use a parameter if Reinette doesn't mind.
I prefer what is in this patch, but I will not object if the function
is changed to take the resource as parameter.
>
> [1]
> https://lore.kernel.org/all/2o7adr2cos6qcikcu7oop4ss7vib2n6ue33djgfeds3v6gj53f@uu45lomrp5qv/
>
Reinette
Re: [PATCH v4 2/5] selftests/resctrl: Add helpers for the non-contiguous test
Hi Maciej,
The subject mentions "helpers" (plural) that may not be accurate
anymore.
On 2/5/2024 4:08 AM, Maciej Wieczor-Retman wrote:
> The CAT non-contiguous selftests have to read the file responsible for
> reporting support of non-contiguous CBMs in kernel (resctrl). Then the
> test compares if that information matches what is reported by CPUID
> output.
>
> Add a generic helper function to read an unsigned number from a file in
> /sys/fs/resctrl/info//.
The "a file in" above can be dropped or it should read "from a file in
/sys/fs/resctrl/info/".
>
> Signed-off-by: Maciej Wieczor-Retman
> ---
> Changelog v4:
> - Rewrite function comment.
> - Redo ksft_perror() as ksft_print_msg(). (Reinette)
>
> Changelog v3:
> - Rewrite patch message.
> - Add documentation and rewrote the function. (Reinette)
>
> Changelog v2:
> - Add this patch.
>
> tools/testing/selftests/resctrl/resctrl.h | 1 +
> tools/testing/selftests/resctrl/resctrlfs.c | 36 +
> 2 files changed, 37 insertions(+)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index a1462029998e..5116ea082d03 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val,
> unsigned int *start);
> int get_full_cbm(const char *cache_type, unsigned long *mask);
> int get_mask_no_shareable(const char *cache_type, unsigned long *mask);
> int get_cache_size(int cpu_no, const char *cache_type, unsigned long
> *cache_size);
> +int resource_info_unsigned_get(const char *resource, const char *filename,
> unsigned int *val);
> void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
> int signal_handler_register(void);
> void signal_handler_unregister(void);
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index 5750662cce57..e0fbc46a917a 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned
> long *mask)
> return 0;
> }
>
> +/**
Apologies for not being clear in my previous comment. I do not
think we want to start exposing resctrl selftest comments to the
kernel-doc tool using this change. The tests are using kernel-doc
style but this is done outside from the kernel-doc tool's view.
> + * resource_info_unsigned_get - Read an unsigned value from
> + * /sys/fs/resctrl/info/RESOURCE/FILENAME
> + * @resource:Resource name that matches directory name in
> + * /sys/fs/resctrl/info
> + * @filename:File in /sys/fs/resctrl/info/@resource
> + * @val: Contains read value on success.
> + *
> + * Return: = 0 on success, < 0 on failure. On success the read
> + * value is saved into the @val.
It can just be "saved into @val"
> + */
> +int resource_info_unsigned_get(const char *resource, const char *filename,
> +unsigned int *val)
> +{
> + char file_path[PATH_MAX];
> + FILE *fp;
> +
> + snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource,
> + filename);
> +
> + fp = fopen(file_path, "r");
> + if (!fp) {
> + ksft_print_msg("Error in opening %s\n: %m\n", file_path);
> + return -1;
> + }
Apart from Ilpo's comment this can also just be "Error opening %s: ..."
> +
> + if (fscanf(fp, "%u", val) <= 0) {
> + ksft_print_msg("Could not get contents of %s\n: %m\n",
> file_path);
> + fclose(fp);
> + return -1;
> + }
> +
> + fclose(fp);
> + return 0;
> +}
> +
> /*
> * create_bit_mask- Create bit mask from start, len pair
> * @start: LSB of the mask
Reinette
Re: [PATCH v4 1/5] selftests/resctrl: Add test groups and name L3 CAT test L3_CAT
Hi Maciej, On 2/5/2024 4:07 AM, Maciej Wieczor-Retman wrote: > From: Ilpo Järvinen > > To select test to run -t parameter can be used. However, -t cat > currently maps to L3 CAT test which will be confusing after more CAT > related tests will be added. > > Allow selecting tests as groups and call L3 CAT test "L3_CAT", "CAT" > group will enable all CAT related tests. > > Signed-off-by: Ilpo Järvinen > Signed-off-by: Maciej Wieczor-Retman > --- Thank you. Reviewed-by: Reinette Chatre Reinette
Re: [KTAP V2 PATCH v2] ktap_v2: add test metadata
On Sun, Feb 4, 2024 at 8:03 AM Kees Cook wrote: > > > > On January 26, 2024 11:14:26 PM GMT+01:00, Rae Moar wrote: > > KTAP version 2 > > # ktap_test: main > > # ktap_arch: uml > > 1..1 > > KTAP version 2 > > # ktap_test: suite_1 > > # ktap_subsystem: example > > # ktap_test_file: lib/test.c > > I think it's a mistake to mix "diagnostics" lines with semantic lines. Since > the diagnostic prefix is [# ] (hash space) how about make the test metadata > lines be [#:] (hash colon). For example: > > > 1..2 > ok 1 test_1 > #:ktap_test: test_2 > #:ktap_speed: very_slow > #:custom_is_flaky: true > # format-free stuff goes here > ok 2 test_2 > ... Hello! I really like this idea. The reason I chose the diagnostic line format was to make it easier for existing parsers to parse the KTAP Metadata lines. However, if it won't be too much of an issue for current parsers, I think this idea would be better. So I am happy to change this in the next version if there are no complaints. > > > ok 1 test_suite > > > >The changes to the KTAP specification outline the format, location, and > >different types of metadata. > > > >Here is a link to a version of the KUnit parser that is able to parse test > >metadata lines for KTAP version 2. Note this includes test metadata > >lines for the main level of KTAP. > > > >Link: https://kunit-review.googlesource.com/c/linux/+/5889 > > > >Signed-off-by: Rae Moar > >--- > > Documentation/dev-tools/ktap.rst | 163 ++- > > 1 file changed, 159 insertions(+), 4 deletions(-) > > > >diff --git a/Documentation/dev-tools/ktap.rst > >b/Documentation/dev-tools/ktap.rst > >index ff77f4aaa6ef..4480eaf5bbc3 100644 > >--- a/Documentation/dev-tools/ktap.rst > >+++ b/Documentation/dev-tools/ktap.rst > >@@ -17,19 +17,20 @@ KTAP test results describe a series of tests (which may > >be nested: i.e., test > > can have subtests), each of which can contain both diagnostic data -- e.g., > > log > > lines -- and a final result. The test structure and results are > > machine-readable, whereas the diagnostic data is unstructured and is there > > to > > We even say it's unstructured... :) > > > >+prefix must not include spaces or the characters ":" or "_". > > Why not _? My thought here was that the first "_" character in the KTAP Metadata line would indicate the separation between the prefix and metadata type. So the prefix could not contain "_". This makes it easier to parse. I'm inclined to keep this but also willing to change it if there is a proposed prefix that contains "_". Thanks! -Rae > > -- > Kees Cook
Re: [KTAP V2 PATCH v2] ktap_v2: add test metadata
On Sat, Feb 3, 2024 at 1:50 AM David Gow wrote: > > On Sat, 27 Jan 2024 at 06:15, Rae Moar wrote: > > > > Add specification for test metadata to the KTAP v2 spec. > > > > KTAP v1 only specifies the output format of very basic test information: > > test result and test name. Any additional test information either gets > > added to general diagnostic data or is not included in the output at all. > > > > The purpose of KTAP metadata is to create a framework to include and > > easily identify additional important test information in KTAP. > > > > KTAP metadata could include any test information that is pertinent for > > user interaction before or after the running of the test. For example, > > the test file path or the test speed. > > > > Since this includes a large variety of information, this specification > > will recognize notable types of KTAP metadata to ensure consistent format > > across test frameworks. See the full list of types in the specification. > > > > Example of KTAP Metadata: > > > > KTAP version 2 > > # ktap_test: main > > # ktap_arch: uml > > 1..1 > > KTAP version 2 > > # ktap_test: suite_1 > > # ktap_subsystem: example > > # ktap_test_file: lib/test.c > > 1..2 > > ok 1 test_1 > > # ktap_test: test_2 > > # ktap_speed: very_slow > > # custom_is_flaky: true > > ok 2 test_2 > > ok 1 test_suite > > This 'test_suite' name doesn't match the 'suite_1' name above. > Hello! Thanks for your review of this documentation. And sorry about that typo. I will change that in the next version. > It also could be clearer that it's supposed to match 'suite_1', not > 'main', due to the indentation difference. Maybe we should add an > explicit note pointing that out? This is a good point. I will add a note in the specification example. > > > > > The changes to the KTAP specification outline the format, location, and > > different types of metadata. > > > > Here is a link to a version of the KUnit parser that is able to parse test > > metadata lines for KTAP version 2. Note this includes test metadata > > lines for the main level of KTAP. > > > > Link: https://kunit-review.googlesource.com/c/linux/+/5889 > > I tested this, and it works well. I think there's a couple of changes > we'd want for a more useful set of KUnit parser changes (namely the > option to support non ktap_ prefixes, as well as an actual way of > using this data), but I'll leave those for a future review of that > patch -- it's not relevant to this spec. Thanks for testing this! My thought was to only support ktap_ prefixes in the parser for now and we could add on others as needed. I will make a separate patch series for this once KTAP Metadata goes through. > > > > > Signed-off-by: Rae Moar > > --- > > I like this: it covers all of the requirements we have in KUnit, as > well as a few things we'd like to add. > > Is there anything obviously missing for this to work with other > usecases? Are there any other examples of metadata people want to > capture? > Yes, I am also curious about what other use cases people are interested in as well? > For me, this is > Reviewed-by: David Gow > > > Documentation/dev-tools/ktap.rst | 163 ++- > > 1 file changed, 159 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/dev-tools/ktap.rst > > b/Documentation/dev-tools/ktap.rst > > index ff77f4aaa6ef..4480eaf5bbc3 100644 > > --- a/Documentation/dev-tools/ktap.rst > > +++ b/Documentation/dev-tools/ktap.rst > > @@ -17,19 +17,20 @@ KTAP test results describe a series of tests (which may > > be nested: i.e., test > > can have subtests), each of which can contain both diagnostic data -- > > e.g., log > > lines -- and a final result. The test structure and results are > > machine-readable, whereas the diagnostic data is unstructured and is there > > to > > -aid human debugging. > > +aid human debugging. One exception to this is test metadata lines - a type > > +of diagnostic lines. Test metadata is used to identify important > > supplemental > > +test information and can be machine-readable. > > > > KTAP output is built from four different types of lines: > > - Version lines > > - Plan lines > > - Test case result lines > > -- Diagnostic lines > > +- Diagnostic lines (including test metadata) > > > > In general, valid KTAP output should also form valid TAP output, but some > > information, in particular nested test results, may be lost. Also note that > > there is a stagnant draft specification for TAP14, KTAP diverges from this > > in > > -a couple of places (notably the "Subtest" header), which are described > > where > > -relevant later in this document. > > +a couple of places, which are described where relevant later in this > > document. > > > > Version lines > > - > > @@ -166,6 +167,154 @@ even if they do not start with a "#": this is to > > capture any other useful > > kernel output which may help debug the test. It
Re: [KTAP V2 PATCH v2] ktap_v2: add test metadata
On Wed, Jan 31, 2024 at 5:22 PM Bird, Tim wrote: > > > > > -Original Message- > > From: Rae Moar > > Add specification for test metadata to the KTAP v2 spec. > > > > KTAP v1 only specifies the output format of very basic test information: > > test result and test name. Any additional test information either gets > > added to general diagnostic data or is not included in the output at all. > > > > The purpose of KTAP metadata is to create a framework to include and > > easily identify additional important test information in KTAP. > > > > KTAP metadata could include any test information that is pertinent for > > user interaction before or after the running of the test. For example, > > the test file path or the test speed. > > > > Since this includes a large variety of information, this specification > > will recognize notable types of KTAP metadata to ensure consistent format > > across test frameworks. See the full list of types in the specification. > > > > Example of KTAP Metadata: > > > > KTAP version 2 > > # ktap_test: main > > # ktap_arch: uml > > 1..1 > > KTAP version 2 > > # ktap_test: suite_1 > > # ktap_subsystem: example > > # ktap_test_file: lib/test.c > > 1..2 > > ok 1 test_1 > > # ktap_test: test_2 > > # ktap_speed: very_slow > > # custom_is_flaky: true > > ok 2 test_2 > > ok 1 test_suite > > > > The changes to the KTAP specification outline the format, location, and > > different types of metadata. > > > > Here is a link to a version of the KUnit parser that is able to parse test > > metadata lines for KTAP version 2. Note this includes test metadata > > lines for the main level of KTAP. > > > > Link: https://kunit-review.googlesource.com/c/linux/+/5889 > > > > Signed-off-by: Rae Moar > > --- > > Documentation/dev-tools/ktap.rst | 163 ++- > > 1 file changed, 159 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/dev-tools/ktap.rst > > b/Documentation/dev-tools/ktap.rst > > index ff77f4aaa6ef..4480eaf5bbc3 100644 > > --- a/Documentation/dev-tools/ktap.rst > > +++ b/Documentation/dev-tools/ktap.rst > > @@ -17,19 +17,20 @@ KTAP test results describe a series of tests (which may > > be nested: i.e., test > > can have subtests), each of which can contain both diagnostic data -- > > e.g., log > > lines -- and a final result. The test structure and results are > > machine-readable, whereas the diagnostic data is unstructured and is there > > to > > -aid human debugging. > > +aid human debugging. One exception to this is test metadata lines - a type > > +of diagnostic lines. Test metadata is used to identify important > > supplemental > > +test information and can be machine-readable. > > > > KTAP output is built from four different types of lines: > > - Version lines > > - Plan lines > > - Test case result lines > > -- Diagnostic lines > > +- Diagnostic lines (including test metadata) > > > > In general, valid KTAP output should also form valid TAP output, but some > > information, in particular nested test results, may be lost. Also note that > > there is a stagnant draft specification for TAP14, KTAP diverges from this > > in > > -a couple of places (notably the "Subtest" header), which are described > > where > > -relevant later in this document. > > +a couple of places, which are described where relevant later in this > > document. > > > > Version lines > > - > > @@ -166,6 +167,154 @@ even if they do not start with a "#": this is to > > capture any other useful > > kernel output which may help debug the test. It is nevertheless recommended > > that tests always prefix any diagnostic output they have with a "#" > > character. > > > > +KTAP metadata lines > > +--- > > + > > +KTAP metadata lines are a subset of diagnostic lines that are used to > > include > > +and easily identify important supplemental test information in KTAP. > > + > > +.. code-block:: none > > + > > + # _: > > + > > +The indicates where to find the specification for the type of > > +metadata. The metadata types listed below use the prefix "ktap" (See Types > > of > > +KTAP Metadata). > > + > > +Types that are instead specified by an individual test framework use the > > +framework name as the prefix. For example, a metadata type documented by > > the > > +kselftest specification would use the prefix "kselftest". Any metadata type > > +that is not listed in a specification must use the prefix "custom". Note > > the > > +prefix must not include spaces or the characters ":" or "_". > > + > > +The format of and varies based on the type. See the > > +individual specification. For "custom" types the can be any > > +string excluding ":", spaces, or newline characters and the can be > > any > > +string. > > + > > +**Location:** > > + > > +The first KTAP metadata entry for a test must be "# ktap_test: > name>", > > +which acts as a header to associate metadata
Re: [PATCH v5 00/12] RISCV: Add kvm Sstc timer selftests
On Tue, Feb 6, 2024 at 12:24 AM Marc Zyngier wrote: > > On Mon, 05 Feb 2024 13:10:26 +, > Haibo Xu wrote: > > > > Hi Marc, > > > > Could you help review the first 3 patches in this series? > > For these 3 patches: > > Reviewed-by: Marc Zyngier > Thanks for the review! > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
Re: [PATCH v3 3/3] selftests: add zswapin and no zswap tests
On Mon, Feb 5, 2024 at 3:05 PM Yosry Ahmed wrote:
>
> On Mon, Feb 05, 2024 at 02:56:08PM -0800, Nhat Pham wrote:
> > Add a selftest to cover the zswapin code path, allocating more memory
> > than the cgroup limit to trigger swapout/zswapout, then reading the
> > pages back in memory several times. This is inspired by a recently
> > encountered kernel crash on the zswapin path in our internal kernel,
> > which went undetected because of a lack of test coverage for this path.
> >
> > Add a selftest to verify that when memory.zswap.max = 0, no pages can go
> > to the zswap pool for the cgroup.
> >
> > Suggested-by: Rik van Riel
> > Suggested-by: Yosry Ahmed
> > Signed-off-by: Nhat Pham
>
> LGTM with a few nits below:
> Acked-by: Yosry Ahmed
>
> Thanks!
>
> > ---
> > tools/testing/selftests/cgroup/test_zswap.c | 120 +++-
> > 1 file changed, 119 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/cgroup/test_zswap.c
> > b/tools/testing/selftests/cgroup/test_zswap.c
> > index 32ce975b21d1..c263610a4a60 100644
> > --- a/tools/testing/selftests/cgroup/test_zswap.c
> > +++ b/tools/testing/selftests/cgroup/test_zswap.c
> > @@ -60,6 +60,27 @@ static long get_zswpout(const char *cgroup)
> > return cg_read_key_long(cgroup, "memory.stat", "zswpout ");
> > }
> >
> > +static int allocate_and_read_bytes(const char *cgroup, void *arg)
> > +{
> > + size_t size = (size_t)arg;
> > + char *mem = (char *)malloc(size);
> > + int ret = 0;
> > +
> > + if (!mem)
> > + return -1;
> > + for (int i = 0; i < size; i += 4095)
> > + mem[i] = 'a';
> > +
> > + /* go through the allocated memory to (z)swap in and out pages */
>
> nit: s/go/Go
>
> > + for (int i = 0; i < size; i += 4095) {
> > + if (mem[i] != 'a')
> > + ret = -1;
> > + }
> > +
> > + free(mem);
> > + return ret;
> > +}
> > +
> > static int allocate_bytes(const char *cgroup, void *arg)
> > {
> > size_t size = (size_t)arg;
> > @@ -100,7 +121,6 @@ static int test_zswap_usage(const char *root)
> > int ret = KSFT_FAIL;
> > char *test_group;
> >
> > - /* Set up */
>
> We removed this comment here.
>
> > test_group = cg_name(root, "no_shrink_test");
> > if (!test_group)
> > goto out;
> > @@ -133,6 +153,102 @@ static int test_zswap_usage(const char *root)
> > return ret;
> > }
> >
> > +/*
> > + * Check that when memory.zswap.max = 0, no pages can go to the zswap pool
> > for
> > + * the cgroup.
> > + */
> > +static int test_swapin_nozswap(const char *root)
> > +{
> > + int ret = KSFT_FAIL;
> > + char *test_group;
> > + long swap_peak, zswpout;
> > +
> > + test_group = cg_name(root, "no_zswap_test");
> > + if (!test_group)
> > + goto out;
> > + if (cg_create(test_group))
> > + goto out;
> > + if (cg_write(test_group, "memory.max", "8M"))
> > + goto out;
> > + if (cg_write(test_group, "memory.zswap.max", "0"))
> > + goto out;
> > +
> > + /* Allocate and read more than memory.max to trigger swapin */
> > + if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
> > + goto out;
> > +
> > + /* Verify that pages are swapped out, but no zswap happened */
> > + swap_peak = cg_read_long(test_group, "memory.swap.peak");
> > + if (swap_peak < 0) {
> > + ksft_print_msg("failed to get cgroup's swap_peak\n");
> > + goto out;
> > + }
> > +
> > + if (swap_peak == 0) {
> > + ksft_print_msg("pages should be swapped out\n");
> > + goto out;
> > + }
>
> We can actually check that this number is >= 24M instead. Not a big
> deal, but might as well.
>
> > +
> > + zswpout = get_zswpout(test_group);
> > + if (zswpout < 0) {
> > + ksft_print_msg("failed to get zswpout\n");
> > + goto out;
> > + }
> > +
> > + if (zswpout > 0) {
> > + ksft_print_msg("zswapout > 0 when memory.zswap.max = 0\n");
> > + goto out;
> > + }
> > +
> > + ret = KSFT_PASS;
> > +
> > +out:
> > + cg_destroy(test_group);
> > + free(test_group);
> > + return ret;
> > +}
> > +
> > +/* Simple test to verify the (z)swapin code paths */
> > +static int test_zswapin(const char *root)
> > +{
> > + int ret = KSFT_FAIL;
> > + char *test_group;
> > + long zswpin;
> > +
> > + /* Set up */
>
> Yet we added a similar one here :)
Urgh I am stupid :) I meant to remove this, but accidentally removed
it from the old test instead :)
>
> > + test_group = cg_name(root, "zswapin_test");
> > + if (!test_group)
> > + goto out;
> > + if (cg_create(test_group))
> > + goto out;
> > + if (cg_write(test_group, "memory.max", "8M"))
> > + goto out;
> > + if (cg_write(test_group, "memory.zswap.max", "max"))
> > + goto out;
Re: [PATCH] bpf: Separate bpf_local_storage_lookup() fast and slow paths
On 2/5/24 7:00 AM, Marco Elver wrote:
On Wed, 31 Jan 2024 at 20:52, Martin KaFai Lau wrote:
[...]
| num_maps: 1000
| local_storage cache sequential get:
| |
| hits throughput: 0.357 ± 0.005 M ops/s | 0.325 ± 0.005 M ops/s
(-9.0%)
| hits latency: 2803.738 ns/op | 3076.923 ns/op
(+9.7%)
Is it understood why the slow down here? The same goes for the "num_maps: 32"
case above but not as bad as here.
It turned out that there's a real slowdown due to the outlined
slowpath. If I inline everything except for inserting the entry into
the cache (cacheit_lockit codepath is still outlined), the results
look much better even for the case where it always misses the cache.
[...]
diff --git a/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
b/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
index a043d8fefdac..9895087a9235 100644
--- a/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
+++ b/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
@@ -21,7 +21,7 @@ struct {
__type(value, long);
} map_b SEC(".maps");
-SEC("fentry/bpf_local_storage_lookup")
+SEC("fentry/bpf_local_storage_lookup_slowpath")
The selftest is trying to catch recursion. The change here cannot test the same
thing because the slowpath will never be hit in the test_progs. I don't have a
better idea for now also.
Trying to prepare a v2, and for the test, the only option I see is to
introduce a tracepoint ("bpf_local_storage_lookup"). If unused, should
be a no-op due to static branch.
Or can you suggest different functions to hook to for the recursion test?
I don't prefer to add another tracepoint for the selftest.
The test in "SEC("fentry/bpf_local_storage_lookup")" is testing that the initial
bpf_local_storage_lookup() should work and the immediate recurred
bpf_task_storage_delete() will fail.
Depends on how the new slow path function will look like in v2. The test can
probably be made to go through the slow path, e.g. by creating a lot of task
storage maps before triggering the lookup.
Re: [PATCH v3 3/3] selftests: add zswapin and no zswap tests
On Mon, Feb 05, 2024 at 02:56:08PM -0800, Nhat Pham wrote:
> Add a selftest to cover the zswapin code path, allocating more memory
> than the cgroup limit to trigger swapout/zswapout, then reading the
> pages back in memory several times. This is inspired by a recently
> encountered kernel crash on the zswapin path in our internal kernel,
> which went undetected because of a lack of test coverage for this path.
>
> Add a selftest to verify that when memory.zswap.max = 0, no pages can go
> to the zswap pool for the cgroup.
>
> Suggested-by: Rik van Riel
> Suggested-by: Yosry Ahmed
> Signed-off-by: Nhat Pham
LGTM with a few nits below:
Acked-by: Yosry Ahmed
Thanks!
> ---
> tools/testing/selftests/cgroup/test_zswap.c | 120 +++-
> 1 file changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/cgroup/test_zswap.c
> b/tools/testing/selftests/cgroup/test_zswap.c
> index 32ce975b21d1..c263610a4a60 100644
> --- a/tools/testing/selftests/cgroup/test_zswap.c
> +++ b/tools/testing/selftests/cgroup/test_zswap.c
> @@ -60,6 +60,27 @@ static long get_zswpout(const char *cgroup)
> return cg_read_key_long(cgroup, "memory.stat", "zswpout ");
> }
>
> +static int allocate_and_read_bytes(const char *cgroup, void *arg)
> +{
> + size_t size = (size_t)arg;
> + char *mem = (char *)malloc(size);
> + int ret = 0;
> +
> + if (!mem)
> + return -1;
> + for (int i = 0; i < size; i += 4095)
> + mem[i] = 'a';
> +
> + /* go through the allocated memory to (z)swap in and out pages */
nit: s/go/Go
> + for (int i = 0; i < size; i += 4095) {
> + if (mem[i] != 'a')
> + ret = -1;
> + }
> +
> + free(mem);
> + return ret;
> +}
> +
> static int allocate_bytes(const char *cgroup, void *arg)
> {
> size_t size = (size_t)arg;
> @@ -100,7 +121,6 @@ static int test_zswap_usage(const char *root)
> int ret = KSFT_FAIL;
> char *test_group;
>
> - /* Set up */
We removed this comment here.
> test_group = cg_name(root, "no_shrink_test");
> if (!test_group)
> goto out;
> @@ -133,6 +153,102 @@ static int test_zswap_usage(const char *root)
> return ret;
> }
>
> +/*
> + * Check that when memory.zswap.max = 0, no pages can go to the zswap pool
> for
> + * the cgroup.
> + */
> +static int test_swapin_nozswap(const char *root)
> +{
> + int ret = KSFT_FAIL;
> + char *test_group;
> + long swap_peak, zswpout;
> +
> + test_group = cg_name(root, "no_zswap_test");
> + if (!test_group)
> + goto out;
> + if (cg_create(test_group))
> + goto out;
> + if (cg_write(test_group, "memory.max", "8M"))
> + goto out;
> + if (cg_write(test_group, "memory.zswap.max", "0"))
> + goto out;
> +
> + /* Allocate and read more than memory.max to trigger swapin */
> + if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
> + goto out;
> +
> + /* Verify that pages are swapped out, but no zswap happened */
> + swap_peak = cg_read_long(test_group, "memory.swap.peak");
> + if (swap_peak < 0) {
> + ksft_print_msg("failed to get cgroup's swap_peak\n");
> + goto out;
> + }
> +
> + if (swap_peak == 0) {
> + ksft_print_msg("pages should be swapped out\n");
> + goto out;
> + }
We can actually check that this number is >= 24M instead. Not a big
deal, but might as well.
> +
> + zswpout = get_zswpout(test_group);
> + if (zswpout < 0) {
> + ksft_print_msg("failed to get zswpout\n");
> + goto out;
> + }
> +
> + if (zswpout > 0) {
> + ksft_print_msg("zswapout > 0 when memory.zswap.max = 0\n");
> + goto out;
> + }
> +
> + ret = KSFT_PASS;
> +
> +out:
> + cg_destroy(test_group);
> + free(test_group);
> + return ret;
> +}
> +
> +/* Simple test to verify the (z)swapin code paths */
> +static int test_zswapin(const char *root)
> +{
> + int ret = KSFT_FAIL;
> + char *test_group;
> + long zswpin;
> +
> + /* Set up */
Yet we added a similar one here :)
> + test_group = cg_name(root, "zswapin_test");
> + if (!test_group)
> + goto out;
> + if (cg_create(test_group))
> + goto out;
> + if (cg_write(test_group, "memory.max", "8M"))
> + goto out;
> + if (cg_write(test_group, "memory.zswap.max", "max"))
> + goto out;
> +
> + /* Allocate and read more than memory.max to trigger (z)swap in */
> + if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
> + goto out;
> +
> + zswpin = cg_read_key_long(test_group, "memory.stat", "zswpin ");
> + if (zswpin < 0) {
> + ksft_print_msg("failed to get zswpin\n");
> + goto out;
> + }
> +
> + if (zswpin == 0) {
> + ksft_print_msg("zswpin
Re: [PATCH] bpf: Separate bpf_local_storage_lookup() fast and slow paths
On Wed, Jan 31, 2024 at 6:19 AM Marco Elver wrote:
>
[...]
>
> Signed-off-by: Marco Elver
> ---
> include/linux/bpf_local_storage.h | 17 -
> kernel/bpf/bpf_local_storage.c | 14 --
> .../selftests/bpf/progs/cgrp_ls_recursion.c | 2 +-
> .../selftests/bpf/progs/task_ls_recursion.c | 2 +-
> 4 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/bpf_local_storage.h
> b/include/linux/bpf_local_storage.h
> index 173ec7f43ed1..c8cecf7fff87 100644
> --- a/include/linux/bpf_local_storage.h
> +++ b/include/linux/bpf_local_storage.h
> @@ -130,9 +130,24 @@ bpf_local_storage_map_alloc(union bpf_attr *attr,
> bool bpf_ma);
>
> struct bpf_local_storage_data *
> +bpf_local_storage_lookup_slowpath(struct bpf_local_storage *local_storage,
> + struct bpf_local_storage_map *smap,
> + bool cacheit_lockit);
> +static inline struct bpf_local_storage_data *
> bpf_local_storage_lookup(struct bpf_local_storage *local_storage,
> struct bpf_local_storage_map *smap,
> -bool cacheit_lockit);
> +bool cacheit_lockit)
> +{
> + struct bpf_local_storage_data *sdata;
> +
> + /* Fast path (cache hit) */
> + sdata = rcu_dereference_check(local_storage->cache[smap->cache_idx],
> + bpf_rcu_lock_held());
> + if (likely(sdata && rcu_access_pointer(sdata->smap) == smap))
> + return sdata;
We have two changes here: 1) inlining; 2) likely() annotation. Could you please
include in the commit log how much do the two contribute to the performance
improvement?
Thanks,
Song
> +
> + return bpf_local_storage_lookup_slowpath(local_storage, smap,
> cacheit_lockit);
> +}
>
[...]
[PATCH v3 3/3] selftests: add zswapin and no zswap tests
Add a selftest to cover the zswapin code path, allocating more memory
than the cgroup limit to trigger swapout/zswapout, then reading the
pages back in memory several times. This is inspired by a recently
encountered kernel crash on the zswapin path in our internal kernel,
which went undetected because of a lack of test coverage for this path.
Add a selftest to verify that when memory.zswap.max = 0, no pages can go
to the zswap pool for the cgroup.
Suggested-by: Rik van Riel
Suggested-by: Yosry Ahmed
Signed-off-by: Nhat Pham
---
tools/testing/selftests/cgroup/test_zswap.c | 120 +++-
1 file changed, 119 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c
b/tools/testing/selftests/cgroup/test_zswap.c
index 32ce975b21d1..c263610a4a60 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -60,6 +60,27 @@ static long get_zswpout(const char *cgroup)
return cg_read_key_long(cgroup, "memory.stat", "zswpout ");
}
+static int allocate_and_read_bytes(const char *cgroup, void *arg)
+{
+ size_t size = (size_t)arg;
+ char *mem = (char *)malloc(size);
+ int ret = 0;
+
+ if (!mem)
+ return -1;
+ for (int i = 0; i < size; i += 4095)
+ mem[i] = 'a';
+
+ /* go through the allocated memory to (z)swap in and out pages */
+ for (int i = 0; i < size; i += 4095) {
+ if (mem[i] != 'a')
+ ret = -1;
+ }
+
+ free(mem);
+ return ret;
+}
+
static int allocate_bytes(const char *cgroup, void *arg)
{
size_t size = (size_t)arg;
@@ -100,7 +121,6 @@ static int test_zswap_usage(const char *root)
int ret = KSFT_FAIL;
char *test_group;
- /* Set up */
test_group = cg_name(root, "no_shrink_test");
if (!test_group)
goto out;
@@ -133,6 +153,102 @@ static int test_zswap_usage(const char *root)
return ret;
}
+/*
+ * Check that when memory.zswap.max = 0, no pages can go to the zswap pool for
+ * the cgroup.
+ */
+static int test_swapin_nozswap(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *test_group;
+ long swap_peak, zswpout;
+
+ test_group = cg_name(root, "no_zswap_test");
+ if (!test_group)
+ goto out;
+ if (cg_create(test_group))
+ goto out;
+ if (cg_write(test_group, "memory.max", "8M"))
+ goto out;
+ if (cg_write(test_group, "memory.zswap.max", "0"))
+ goto out;
+
+ /* Allocate and read more than memory.max to trigger swapin */
+ if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
+ goto out;
+
+ /* Verify that pages are swapped out, but no zswap happened */
+ swap_peak = cg_read_long(test_group, "memory.swap.peak");
+ if (swap_peak < 0) {
+ ksft_print_msg("failed to get cgroup's swap_peak\n");
+ goto out;
+ }
+
+ if (swap_peak == 0) {
+ ksft_print_msg("pages should be swapped out\n");
+ goto out;
+ }
+
+ zswpout = get_zswpout(test_group);
+ if (zswpout < 0) {
+ ksft_print_msg("failed to get zswpout\n");
+ goto out;
+ }
+
+ if (zswpout > 0) {
+ ksft_print_msg("zswapout > 0 when memory.zswap.max = 0\n");
+ goto out;
+ }
+
+ ret = KSFT_PASS;
+
+out:
+ cg_destroy(test_group);
+ free(test_group);
+ return ret;
+}
+
+/* Simple test to verify the (z)swapin code paths */
+static int test_zswapin(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *test_group;
+ long zswpin;
+
+ /* Set up */
+ test_group = cg_name(root, "zswapin_test");
+ if (!test_group)
+ goto out;
+ if (cg_create(test_group))
+ goto out;
+ if (cg_write(test_group, "memory.max", "8M"))
+ goto out;
+ if (cg_write(test_group, "memory.zswap.max", "max"))
+ goto out;
+
+ /* Allocate and read more than memory.max to trigger (z)swap in */
+ if (cg_run(test_group, allocate_and_read_bytes, (void *)MB(32)))
+ goto out;
+
+ zswpin = cg_read_key_long(test_group, "memory.stat", "zswpin ");
+ if (zswpin < 0) {
+ ksft_print_msg("failed to get zswpin\n");
+ goto out;
+ }
+
+ if (zswpin == 0) {
+ ksft_print_msg("zswpin should not be 0\n");
+ goto out;
+ }
+
+ ret = KSFT_PASS;
+
+out:
+ cg_destroy(test_group);
+ free(test_group);
+ return ret;
+}
+
/*
* When trying to store a memcg page in zswap, if the memcg hits its memory
* limit in zswap, writeback should affect only the zswapped pages of that
@@ -309,6 +425,8 @@ struct zswap_test {
const char *name;
} tests[] = {
[PATCH v3 2/3] selftests: fix the zswap invasive shrink test
The zswap no invasive shrink selftest breaks because we rename the zswap
writeback counter (see [1]). Fix the test.
[1]:
https://patchwork.kernel.org/project/linux-kselftest/patch/20231205193307.2432803-1-npha...@gmail.com/
Fixes: a697dc2be925 ("selftests: cgroup: update per-memcg zswap writeback
selftest")
Signed-off-by: Nhat Pham
Acked-by: Yosry Ahmed
---
tools/testing/selftests/cgroup/test_zswap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_zswap.c
b/tools/testing/selftests/cgroup/test_zswap.c
index 47fdaa146443..32ce975b21d1 100644
--- a/tools/testing/selftests/cgroup/test_zswap.c
+++ b/tools/testing/selftests/cgroup/test_zswap.c
@@ -52,7 +52,7 @@ static int get_zswap_stored_pages(size_t *value)
static int get_cg_wb_count(const char *cg)
{
- return cg_read_key_long(cg, "memory.stat", "zswp_wb");
+ return cg_read_key_long(cg, "memory.stat", "zswpwb");
}
static long get_zswpout(const char *cgroup)
--
2.39.3
[PATCH v3 1/3] selftests: zswap: add zswap selftest file to zswap maintainer entry
Make it easier for contributors to find the zswap maintainers when they update the zswap tests. Signed-off-by: Nhat Pham Acked-by: Yosry Ahmed --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6527850f24d1..126090f0e418 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -24398,6 +24398,7 @@ F: include/linux/zpool.h F: include/linux/zswap.h F: mm/zpool.c F: mm/zswap.c +F: tools/testing/selftests/cgroup/test_zswap.c THE REST M: Linus Torvalds -- 2.39.3
[PATCH v3 0/3] fix and extend zswap kselftests
Changelog: v3: * More cleanup (patch 3) (suggested by Yosry Ahmed). * Check swap peak in swapin test v2: * Make the swapin test also checks for zswap usage (patch 3) (suggested by Yosry Ahmed) * Some test simplifications/cleanups (patch 3) (suggested by Yosry Ahmed). Fix a broken zswap kselftest due to cgroup zswap writeback counter renaming, and add 2 zswap kselftests, one to cover the (z)swapin case, and another to check that no zswapping happens when the cgroup limit is 0. Also, add the zswap kselftest file to zswap maintainer entry so that get_maintainers script can find zswap maintainers. Nhat Pham (3): selftests: zswap: add zswap selftest file to zswap maintainer entry selftests: fix the zswap invasive shrink test selftests: add zswapin and no zswap tests MAINTAINERS | 1 + tools/testing/selftests/cgroup/test_zswap.c | 122 +++- 2 files changed, 121 insertions(+), 2 deletions(-) base-commit: 91f3daa1765ee4e0c89987dc25f72c40f07af34d -- 2.39.3
Re: [PATCH v8 0/4] Introduce mseal
On Fri, Feb 2, 2024 at 8:46 PM Liam R. Howlett wrote: > > * Linus Torvalds [240202 18:36]: > > On Fri, 2 Feb 2024 at 13:18, Liam R. Howlett > > wrote: > > > > > > There will be a larger performance cost to checking up front without > > > allowing the partial completion. > > > > I suspect that for mseal(), the only half-way common case will be > > sealing an area that is entirely contained within one vma. > > Agreed. > > > > > So the cost will be the vma splitting (if it's not the whole vma), and > > very unlikely to be any kind of "walk the vma's to check that they can > > all be sealed" loop up-front. > > That's the cost of calling mseal(), and I think that will be totally > reasonable. > > I'm more concerned with the other calls that do affect more than one vma > that will now have to ensure there is not an mseal'ed vma among the > affected area. > > As you pointed out, we don't do atomic updates and so we have to add a > loop at the beginning to check this new special case, which is what this > patch set does today. That means we're going to be looping through > twice for any call that could fail if one is mseal'ed. This includes > munmap() and mprotect(). > > The impact will vary based on how many vma's are handled. I'd like some > numbers on this so we can see if it is a concern, which Jeff has agreed > to provide in the future - Thank you, Jeff. Yes please. The additional walk Liam points to seems to be happening even if we don't use mseal at all. Android apps often create thousands of VMAs, so a small regression to a syscall like mprotect might cause a very visible regression to app launch times (one of the key metrics for Android). Having performance impact numbers here would be very helpful. > > It also means we're modifying the behaviour of those calls so they could > fail before anything changes (regardless of where the failure would > occur), and we could still fail later when another aspect of a vma would > cause a failure as we do today. We are paying the price for a more > atomic update, but we aren't trying very hard to be atomic with our > updates - we don't have many (virtually no) vma checks before > modifications start. > > For instance, we could move the mprotect check for map_deny_write_exec() > to the pre-update loop to make it more atomic in nature. This one seems > somewhat related to mseal, so it would be better if they were both > checked atomic(ish) together. Although, I wonder if the user visible > changes would be acceptable and worth the risk. > > We will have two classes of updates to vma's: the more atomic view and > the legacy view. The question of what happens when the two mix, or > where a specific check should go will get (more) confusing. > > Thanks, > Liam >
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Hi Stephen,
On Mon, Feb 5, 2024 at 8:19 PM Stephen Boyd wrote:
> Quoting David Gow (2024-02-02 20:10:17)
> > On Sat, 3 Feb 2024 at 03:59, Stephen Boyd wrote:
> > > Add a KUnit test that confirms a DTB has been loaded, i.e. there is a
> > > root node, and that the of_have_populated_dt() API works properly.
> > >
> > > Cc: Rob Herring
> > > Cc: Frank Rowand
> > > Cc: David Gow
> > > Cc: Brendan Higgins
> > > Signed-off-by: Stephen Boyd
> > > ---
> >
> > This looks pretty good to me test-wise, though it still fails on m68k.
> > (Everything else I tried it on works, though I've definitely not tried
> > _every_ architecture.)
> >
> > aarch64: PASSED
> > i386: PASSED
> > x86_64: PASSED
> > x86_64 KASAN: PASSED
> > powerpc64: PASSED
> > UML: PASSED
> > UML LLVM: PASSED
> > m68k: FAILED
> > > $ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux
> > > -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot
> > > -nographic -serial stdio -machine virt
> > > [11:55:05] = dtb (2 subtests) =
> > > [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at
> > > drivers/of/of_test.c:18
> > > [11:55:05] Expected np is not null, but is
> > > [11:55:05] [FAILED] dtb_root_node_found_by_path
> > > [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at
> > > drivers/of/of_test.c:28
> > > [11:55:05] Expected of_root is not null, but is
> > > [11:55:05] [FAILED] dtb_root_node_populates_of_root
> > > [11:55:05] # module: of_test
> > > [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2
> > > [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2
> > > [11:55:05] === [FAILED] dtb ===
>
> Ah yeah I forgot to mention that. m68k fails because it doesn't call the
> unflatten_(and_copy)?_device_tree() function, so we don't populate a
> root node on that architecture. One solution would be to make CONFIG_OF
> unavailable on m68k. Or we have to make sure DT works on any
> architecture. Rob, what do you prefer here?
I guess the latter?
Alpha, hexagon, parisc, s390, and sparc are also lacking calls
to unflatten.*device_tree().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 7/7] of: Add KUnit test to confirm DTB is loaded
Quoting David Gow (2024-02-02 20:10:17) > On Sat, 3 Feb 2024 at 03:59, Stephen Boyd wrote: > > > > Add a KUnit test that confirms a DTB has been loaded, i.e. there is a > > root node, and that the of_have_populated_dt() API works properly. > > > > Cc: Rob Herring > > Cc: Frank Rowand > > Cc: David Gow > > Cc: Brendan Higgins > > Signed-off-by: Stephen Boyd > > --- > > This looks pretty good to me test-wise, though it still fails on m68k. > (Everything else I tried it on works, though I've definitely not tried > _every_ architecture.) > > aarch64: PASSED > i386: PASSED > x86_64: PASSED > x86_64 KASAN: PASSED > powerpc64: PASSED > UML: PASSED > UML LLVM: PASSED > m68k: FAILED > > $ qemu-system-m68k -nodefaults -m 1024 -kernel .kunit-all-m68k/vmlinux > > -append 'kunit.enable=1 console=hvc0 kunit_shutdown=reboot' -no-reboot > > -nographic -serial stdio -machine virt > > [11:55:05] = dtb (2 subtests) = > > [11:55:05] # dtb_root_node_found_by_path: EXPECTATION FAILED at > > drivers/of/of_test.c:18 > > [11:55:05] Expected np is not null, but is > > [11:55:05] [FAILED] dtb_root_node_found_by_path > > [11:55:05] # dtb_root_node_populates_of_root: EXPECTATION FAILED at > > drivers/of/of_test.c:28 > > [11:55:05] Expected of_root is not null, but is > > [11:55:05] [FAILED] dtb_root_node_populates_of_root > > [11:55:05] # module: of_test > > [11:55:05] # dtb: pass:0 fail:2 skip:0 total:2 > > [11:55:05] # Totals: pass:0 fail:2 skip:0 total:2 > > [11:55:05] === [FAILED] dtb === Ah yeah I forgot to mention that. m68k fails because it doesn't call the unflatten_(and_copy)?_device_tree() function, so we don't populate a root node on that architecture. One solution would be to make CONFIG_OF unavailable on m68k. Or we have to make sure DT works on any architecture. Rob, what do you prefer here? > > > My only other question is about the test names: the mix of 'of' and > 'dtb' can be a bit confusing. As is, we have: > kconfig name: OF_KUNIT_TEST > module name: of_test > suite name: dtb > test names: all start with dtb_ > > Given KUnit only really deals with the suite/test names directly, it's > not trivial to see that 'dtb.dtb_*' is controlled by OF_KUNIT_TEST and > in of_test if built as a module. (This is getting a bit easier now > that we have the 'module' attribute in the output, but still.) > > Would 'of_dtb' work as a suite name if it's important to keep both > 'of' and 'dtb'? Sure, I can add of_ prefix to the tests. > > In general, though, this test looks good to me. Particularly if m68k > can be fixed. > > Reviewed-by: David Gow > Thanks!
[PATCH v2 2/2] selftests/mm: run_vmtests.sh: add hugetlb_madv_vs_map
hugetlb_madv_vs_map selftest was not part of the mm test-suite since we didn't have a fix for the problem it found. Now that the problem is already fixed (see previous commit), let's enable this selftest in the default test-suite. Signed-off-by: Breno Leitao --- tools/testing/selftests/mm/run_vmtests.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 246d53a5d7f2..50e2094ed761 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -253,6 +253,7 @@ nr_hugepages_tmp=$(cat /proc/sys/vm/nr_hugepages) # For this test, we need one and just one huge page echo 1 > /proc/sys/vm/nr_hugepages CATEGORY="hugetlb" run_test ./hugetlb_fault_after_madv +CATEGORY="hugetlb" run_test ./hugetlb_madv_vs_map # Restore the previous number of huge pages, since further tests rely on it echo "$nr_hugepages_tmp" > /proc/sys/vm/nr_hugepages -- 2.34.1
Re: [PATCH bpf-next v4 0/3] Annotate kfuncs in .BTF_ids section
On 2/3/24 19:45, Manu Bretelle wrote:
> On Sat, Feb 03, 2024 at 03:40:24PM +0100, Jiri Olsa wrote:
>> On Fri, Feb 02, 2024 at 03:09:05PM -0800, Manu Bretelle wrote:
>>> On Sun, Jan 28, 2024 at 06:24:05PM -0700, Daniel Xu wrote:
=== Description ===
This is a bpf-treewide change that annotates all kfuncs as such inside
.BTF_ids. This annotation eventually allows us to automatically generate
kfunc prototypes from bpftool.
We store this metadata inside a yet-unused flags field inside struct
btf_id_set8 (thanks Kumar!). pahole will be taught where to look.
More details about the full chain of events are available in commit 3's
description.
The accompanying pahole and bpftool changes can be viewed
here on these "frozen" branches [0][1].
[0]: https://github.com/danobi/pahole/tree/kfunc_btf-v3-mailed
[1]: https://github.com/danobi/linux/tree/kfunc_bpftool-mailed
>>>
>>>
>>> I hit a similar issue to [0] on master
>>> 943b043aeecc ("selftests/bpf: Fix bench runner SIGSEGV")
>>> when cross-compiling on x86_64 (LE) to s390x (BE).
>>> I do have CONFIG_DEBUG_INFO_BTF enable and the issue would not trigger if
>>> I disabled CONFIG_DEBUG_INFO_BTF (and with the fix mentioned in [0]).
>>>
>>> What seems to happen is that `tools/resolve_btfids` is ran in the context
>>> of the
>>> host endianess and if I printk before the WARN_ON:
>>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>>> index ef380e546952..a9ed7a1a4936 100644
>>> --- a/kernel/bpf/btf.c
>>> +++ b/kernel/bpf/btf.c
>>> @@ -8128,6 +8128,7 @@ int register_btf_kfunc_id_set(enum bpf_prog_type
>>> prog_type,
>>>* WARN() for initcall registrations that do not check errors.
>>>*/
>>> if (!(kset->set->flags & BTF_SET8_KFUNCS)) {
>>> +printk("Flag 0x%08X, expected 0x%08X\n", kset->set->flags,
>>> BTF_SET8_KFUNCS);
>>> WARN_ON(!kset->owner);
>>> return -EINVAL;
>>> }
>>>
>>> the boot logs would show:
>>> Flag 0x0100, expected 0x0001
>>>
>>> The issue did not happen prior to
>>> 6f3189f38a3e ("bpf: treewide: Annotate BPF kfuncs in BTF")
>>> has only 0 was written before.
>>>
>>> It seems [1] will be addressing cross-compilation, but it did not fix it as
>>> is
>>> by just applying on top of master, so probably some of the changes will
>>> also need
>>> to be ported to `tools/include/linux/btf_ids.h`?
>>
>> the fix in [1] is fixing flags in set8's pairs, but not the global flags
>>
>> it looks like Viktor's fix should now also swap that as well? like in the
>> change below on top of Viktor's changes (untested)
>>
>> jirka
>>
>>
>> ---
>> diff --git a/tools/bpf/resolve_btfids/main.c
>> b/tools/bpf/resolve_btfids/main.c
>> index d01603ef6283..c44d57fec390 100644
>> --- a/tools/bpf/resolve_btfids/main.c
>> +++ b/tools/bpf/resolve_btfids/main.c
>> @@ -706,6 +706,8 @@ static int sets_patch(struct object *obj)
>> * correctly translate everything.
>> */
>> if (need_bswap) {
>> +set8->flags = bswap_32(set8->flags);
>> +
>> for (i = 0; i < cnt; i++) {
>> set8->pairs[i].flags =
>> bswap_32(set8->pairs[i].flags);
>>
>
> That should work. Here are a few tests I ran:
>
> $ md5sum /tmp/kbuild-s390x/vmlinux.*
> eb658e51e089f3c5b2c8909a29dc9997 /tmp/kbuild-s390x/vmlinux.a
> # plain vmlinux before running resolv_btfids (all 0s)
> ea907cd46a1a73b8276b5f2a82af00ca /tmp/kbuild-s390x/vmlinux.before_resolv
> # x86_64 resolv_btfids on master without Viktor's patch
> 980a40c3a3ff563d1c2d1ebdd5071a23 /tmp/kbuild-s390x/vmlinux.resolv_native
> # x86_64 resolv_btfids on master with Viktor's patch
> b986d19e242719ebea41c578235da662
> /tmp/kbuild-s390x/vmlinux.resolv_native_patch_viktor
> # x86_64 resolv_btfids on master with Viktor's patch and your suggested patch
> 4edd8752ff01129945bd442689b1927b
> /tmp/kbuild-s390x/vmlinux.resolv_native_patch_viktor_patched
> # s390x resolv_btfids run with qemu-s390x-static
> 4edd8752ff01129945bd442689b1927b /tmp/kbuild-s390x/vmlinux.resolv_s390x
>
>
> and some hexdiff of those binaries:
>
>
> # difference between master's native build and s390x build has byte
> swapping for set8 and others
> diff -ruN <(xxd /tmp/kbuild-s390x/vmlinux.resolv_s390x) <(xxd
> /tmp/kbuild-s390x/vmlinux.resolv_native) > diff_s390x_native.diff
> https://gist.github.com/chantra/c3d58637a08a6f7340953dc155bb18cc
>
> # difference betwee Viktor's version and s390x build squinting my eyes I
> only see the global set8 is missing
> diff -ruN <(xxd /tmp/kbuild-s390x/vmlinux.resolv_s390x) <(xxd
> /tmp/kbuild-s390x/vmlinux.resolv_native_patch_viktor) >
> diff_s390x_native_viktor.diff
> https://gist.github.com/chantra/61cfff02b456ae72d3c0161ce1897097
Thanks for the testing
Re: [PATCH v8 13/38] KVM: arm64: Manage GCS registers for guests
On Mon, Feb 05, 2024 at 03:34:05PM +, Marc Zyngier wrote: > Mark Brown wrote: > > On Mon, Feb 05, 2024 at 09:46:16AM +, Marc Zyngier wrote: > > > We have had this discussion in the past. This must be based on the > > > VM's configuration. Guarding the check with the host capability is a > > > valuable optimisation, but that's nowhere near enough. See the series > > > that I have posted on this very subject (you're on Cc), but you are > > > welcome to invent your own mechanism in the meantime. > > Right, which postdates the version you're replying to and isn't merged > > yet - the current code was what you were asking for at the time. > v1 and v2 predate it. And if the above is what I did ask, then I must > have done a very poor job of explaining what was required. For which I > apologise profusely. To be clear it's what was asked for prior to the switch to the forthcoming switch to the parsing idregs scheme, I haven't pulled in your idregs work yet since it's being rapidly iterated and this is an already large series with dependencies. > > I'm > > expecting to update all these feature series to work with that once it > > gets finalised and merged but it's not there yet, I do see I forgot to > > put a note in v9 about that like I did for dpISA - sorry about that, I > > was too focused on the clone3() rework when rebasing onto the new > > kernel. > > This particular series isn't going to get merged for a while yet anyway > > due to the time it'll take for userspace testing, I'm expecting your > > series to be in by the time it becomes an issue. > Right. Then I'll ignore it for the foreseeable future. Actually now I think about it would you be open to merging the guest context switching bit without the rest of the series (pending me fixing the issues you raise of course)? If so I'll split that bit out in the hope that we can reduce the size of the series and CC list for the userspace support which I imagine would make people a bit happier. > > > > + write_sysreg_s(ctxt_sys_reg(ctxt, GCSCRE0_EL1), > > > > + SYS_GCSCRE0_EL1); > > > > + } > > > For the benefit of the unsuspecting reviewers, and in the absence of a > > > public specification (which the XML drop isn't), it would be good to > > > have the commit message explaining the rationale of what gets saved > > > when. > > What are you looking for in terms of rationale here? The KVM house > > style is often very reliant on reader context so it would be good to > > know what considerations you'd like to see explicitly addressed. > Nothing to do with style, everything to do with substance: if nothing The style I'm referring to there is the style for documentation. > in the host kernel makes any use of these registers, why are they > eagerly saved/restored on nVHE/hVHE? I'm sure you have a reason for > it, but it isn't that obvious. Because these two modes need all the > help they can get in terms of overhead reduction. Ah, I see - yes, they should probably be moved somewhere else. Though I'm not clear why some of the other registers that we're saving and restoring in the same place are being done eagerly? The userspace TPIDRs stand out for example, they're in taken care of in __sysreg_save_user_state() which is called in the same paths. IIRC my thinking there was something along the lines of "this is where we save and restore everything else that's just a general system register, I should be consistent". Am I right in thinking kvm_arch_vcpu_load()/_put() would make sense? Everything in there currently looked like it was there more due to doing something more complex than simple register save/restore and we weren't worrying too much about what was going on with just the sysregs. > > These > > registers shouldn't do anything when we aren't running the guest so > > they're not terribly ordering sensitive, the EL2 ones will need a bit > > more consideration in the face of nested virt. > The EL2 registers should follow the exact same pattern, specially once > you fix the VNCR bugs I pointed out. Great, that's what I'd thought thanks - I hadn't checked yet. signature.asc Description: PGP signature
[PATCH v14 6/6] ring-buffer/selftest: Add ring-buffer mapping test
This test maps a ring-buffer and validate the meta-page after reset and
after emitting few events.
Cc: Shuah Khan
Cc: Shuah Khan
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: Vincent Donnefort
diff --git a/tools/testing/selftests/ring-buffer/Makefile
b/tools/testing/selftests/ring-buffer/Makefile
new file mode 100644
index ..627c5fa6d1ab
--- /dev/null
+++ b/tools/testing/selftests/ring-buffer/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+CFLAGS += -Wl,-no-as-needed -Wall
+CFLAGS += $(KHDR_INCLUDES)
+CFLAGS += -D_GNU_SOURCE
+
+TEST_GEN_PROGS = map_test
+
+include ../lib.mk
diff --git a/tools/testing/selftests/ring-buffer/config
b/tools/testing/selftests/ring-buffer/config
new file mode 100644
index ..d936f8f00e78
--- /dev/null
+++ b/tools/testing/selftests/ring-buffer/config
@@ -0,0 +1,2 @@
+CONFIG_FTRACE=y
+CONFIG_TRACER_SNAPSHOT=y
diff --git a/tools/testing/selftests/ring-buffer/map_test.c
b/tools/testing/selftests/ring-buffer/map_test.c
new file mode 100644
index ..56c44b29d998
--- /dev/null
+++ b/tools/testing/selftests/ring-buffer/map_test.c
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ring-buffer memory mapping tests
+ *
+ * Copyright (c) 2024 Vincent Donnefort
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+
+#include "../user_events/user_events_selftests.h" /* share tracefs setup */
+#include "../kselftest_harness.h"
+
+#define TRACEFS_ROOT "/sys/kernel/tracing"
+
+static int __tracefs_write(const char *path, const char *value)
+{
+ int fd, ret;
+
+ fd = open(path, O_WRONLY | O_TRUNC);
+ if (fd < 0)
+ return fd;
+
+ ret = write(fd, value, strlen(value));
+
+ close(fd);
+
+ return ret == -1 ? -errno : 0;
+}
+
+static int __tracefs_write_int(const char *path, int value)
+{
+ char *str;
+ int ret;
+
+ if (asprintf(, "%d", value) < 0)
+ return -1;
+
+ ret = __tracefs_write(path, str);
+
+ free(str);
+
+ return ret;
+}
+
+#define tracefs_write_int(path, value) \
+ ASSERT_EQ(__tracefs_write_int((path), (value)), 0)
+
+#define tracefs_write(path, value) \
+ ASSERT_EQ(__tracefs_write((path), (value)), 0)
+
+static int tracefs_reset(void)
+{
+ if (__tracefs_write_int(TRACEFS_ROOT"/tracing_on", 0))
+ return -1;
+ if (__tracefs_write(TRACEFS_ROOT"/trace", ""))
+ return -1;
+ if (__tracefs_write(TRACEFS_ROOT"/set_event", ""))
+ return -1;
+ if (__tracefs_write(TRACEFS_ROOT"/current_tracer", "nop"))
+ return -1;
+
+ return 0;
+}
+
+struct tracefs_cpu_map_desc {
+ struct trace_buffer_meta*meta;
+ void*data;
+ int cpu_fd;
+};
+
+int tracefs_cpu_map(struct tracefs_cpu_map_desc *desc, int cpu)
+{
+ unsigned long meta_len, data_len;
+ int page_size = getpagesize();
+ char *cpu_path;
+ void *map;
+
+ if (asprintf(_path,
+TRACEFS_ROOT"/per_cpu/cpu%d/trace_pipe_raw",
+cpu) < 0)
+ return -ENOMEM;
+
+ desc->cpu_fd = open(cpu_path, O_RDONLY | O_NONBLOCK);
+ free(cpu_path);
+ if (desc->cpu_fd < 0)
+ return -ENODEV;
+
+ map = mmap(NULL, page_size, PROT_READ, MAP_SHARED, desc->cpu_fd, 0);
+ if (map == MAP_FAILED)
+ return -errno;
+
+ desc->meta = (struct trace_buffer_meta *)map;
+
+ meta_len = desc->meta->meta_page_size;
+ data_len = desc->meta->subbuf_size * desc->meta->nr_subbufs;
+
+ map = mmap(NULL, data_len, PROT_READ, MAP_SHARED, desc->cpu_fd,
meta_len);
+ if (map == MAP_FAILED) {
+ munmap(desc->meta, desc->meta->meta_page_size);
+ return -EINVAL;
+ }
+
+ desc->data = map;
+
+ return 0;
+}
+
+void tracefs_cpu_unmap(struct tracefs_cpu_map_desc *desc)
+{
+ munmap(desc->data, desc->meta->subbuf_size * desc->meta->nr_subbufs);
+ munmap(desc->meta, desc->meta->meta_page_size);
+ close(desc->cpu_fd);
+}
+
+FIXTURE(map) {
+ struct tracefs_cpu_map_desc map_desc;
+ boolumount;
+};
+
+FIXTURE_VARIANT(map) {
+ int subbuf_size;
+};
+
+FIXTURE_VARIANT_ADD(map, subbuf_size_4k) {
+ .subbuf_size = 4,
+};
+
+FIXTURE_VARIANT_ADD(map, subbuf_size_8k) {
+ .subbuf_size = 8,
+};
+
+FIXTURE_SETUP(map)
+{
+ int cpu = sched_getcpu();
+ cpu_set_t cpu_mask;
+ bool fail, umount;
+ char *message;
+
+ if (!tracefs_enabled(, , )) {
+ if (fail) {
+ TH_LOG("Tracefs setup failed: %s", message);
+ ASSERT_FALSE(fail);
+ }
+ SKIP(return, "Skipping: %s", message);
+ }
+
+ self->umount = umount;
+
+
Re: [PATCH v5 00/12] RISCV: Add kvm Sstc timer selftests
On Mon, 05 Feb 2024 13:10:26 +, Haibo Xu wrote: > > Hi Marc, > > Could you help review the first 3 patches in this series? For these 3 patches: Reviewed-by: Marc Zyngier Thanks, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH v3] KVM: selftests: Fix the dirty_log_test semaphore imbalance
On Mon, Feb 05, 2024, Peter Xu wrote: > Shaoqin, Sean, > > Apologies for a late comment. I'm trying to remember what I wrote.. > > On Fri, Feb 02, 2024 at 01:43:32AM -0500, Shaoqin Huang wrote: > > Why sem_vcpu_cont and sem_vcpu_stop can be non-zero value? It's because > > the dirty_ring_before_vcpu_join() execute the sem_post(_vcpu_cont) > > at the end of each dirty-ring test. It can cause two cases: > > As a possible alternative, would it work if we simply reset all the sems > for each run? Then we don't care about the leftovers. E.g. sem_destroy() > at the end of run_test(), then always init to 0 at entry. Gah, I posted v4[*] and didn't Cc you. I would prefer not to reset the semaphores, mostly because it encourages sloppiness in the test. There's no reason to allow the two threads to effectively get out of sync. [*] https://lore.kernel.org/all/17eefa60-cf30-4830-943e-793a63d4e...@redhat.com
Re: [PATCH v8 13/38] KVM: arm64: Manage GCS registers for guests
On Mon, 05 Feb 2024 12:35:53 +,
Mark Brown wrote:
>
> On Mon, Feb 05, 2024 at 09:46:16AM +, Marc Zyngier wrote:
> > On Sat, 03 Feb 2024 12:25:39 +,
> > Mark Brown wrote:
>
> > > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > > @@ -25,6 +25,8 @@ 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 (has_gcs())
> > > + ctxt_sys_reg(ctxt, GCSPR_EL0) = read_sysreg_s(SYS_GCSPR_EL0);
>
> > We have had this discussion in the past. This must be based on the
> > VM's configuration. Guarding the check with the host capability is a
> > valuable optimisation, but that's nowhere near enough. See the series
> > that I have posted on this very subject (you're on Cc), but you are
> > welcome to invent your own mechanism in the meantime.
>
> Right, which postdates the version you're replying to and isn't merged
> yet - the current code was what you were asking for at the time.
v1 and v2 predate it. And if the above is what I did ask, then I must
have done a very poor job of explaining what was required. For which I
apologise profusely.
> I'm
> expecting to update all these feature series to work with that once it
> gets finalised and merged but it's not there yet, I do see I forgot to
> put a note in v9 about that like I did for dpISA - sorry about that, I
> was too focused on the clone3() rework when rebasing onto the new
> kernel.
>
> This particular series isn't going to get merged for a while yet anyway
> due to the time it'll take for userspace testing, I'm expecting your
> series to be in by the time it becomes an issue.
Right. Then I'll ignore it for the foreseeable future.
>
> > > + if (has_gcs()) {
> > > + write_sysreg_el1(ctxt_sys_reg(ctxt, GCSPR_EL1), SYS_GCSPR);
> > > + write_sysreg_el1(ctxt_sys_reg(ctxt, GCSCR_EL1), SYS_GCSCR);
> > > + write_sysreg_s(ctxt_sys_reg(ctxt, GCSCRE0_EL1),
> > > +SYS_GCSCRE0_EL1);
> > > + }
>
> > For the benefit of the unsuspecting reviewers, and in the absence of a
> > public specification (which the XML drop isn't), it would be good to
> > have the commit message explaining the rationale of what gets saved
> > when.
>
> What are you looking for in terms of rationale here? The KVM house
> style is often very reliant on reader context so it would be good to
> know what considerations you'd like to see explicitly addressed.
Nothing to do with style, everything to do with substance: if nothing
in the host kernel makes any use of these registers, why are they
eagerly saved/restored on nVHE/hVHE? I'm sure you have a reason for
it, but it isn't that obvious. Because these two modes need all the
help they can get in terms of overhead reduction.
> These
> registers shouldn't do anything when we aren't running the guest so
> they're not terribly ordering sensitive, the EL2 ones will need a bit
> more consideration in the face of nested virt.
The EL2 registers should follow the exact same pattern, specially once
you fix the VNCR bugs I pointed out.
M.
--
Without deviation from the norm, progress is not possible.
Re: [PATCH] bpf: Separate bpf_local_storage_lookup() fast and slow paths
On Wed, 31 Jan 2024 at 20:52, Martin KaFai Lau wrote:
[...]
> > | num_maps: 1000
> > | local_storage cache sequential get:
> > | |
> > | hits throughput: 0.357 ± 0.005 M ops/s | 0.325 ± 0.005 M
> > ops/s(-9.0%)
> > | hits latency: 2803.738 ns/op | 3076.923 ns/op
> > (+9.7%)
>
> Is it understood why the slow down here? The same goes for the "num_maps: 32"
> case above but not as bad as here.
It turned out that there's a real slowdown due to the outlined
slowpath. If I inline everything except for inserting the entry into
the cache (cacheit_lockit codepath is still outlined), the results
look much better even for the case where it always misses the cache.
[...]
> > diff --git a/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
> > b/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
> > index a043d8fefdac..9895087a9235 100644
> > --- a/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
> > +++ b/tools/testing/selftests/bpf/progs/cgrp_ls_recursion.c
> > @@ -21,7 +21,7 @@ struct {
> > __type(value, long);
> > } map_b SEC(".maps");
> >
> > -SEC("fentry/bpf_local_storage_lookup")
> > +SEC("fentry/bpf_local_storage_lookup_slowpath")
>
> The selftest is trying to catch recursion. The change here cannot test the
> same
> thing because the slowpath will never be hit in the test_progs. I don't have
> a
> better idea for now also.
Trying to prepare a v2, and for the test, the only option I see is to
introduce a tracepoint ("bpf_local_storage_lookup"). If unused, should
be a no-op due to static branch.
Or can you suggest different functions to hook to for the recursion test?
Preferences?
[PATCH] selftests/mm: uffd-unit-test check if huge page size is 0
If HUGETLBFS is not enabled then the default_huge_page_size function will
return 0 and cause a divide by 0 error. Add a check to see if the huge page
size is 0 and skip the hugetlb tests if it is.
Signed-off-by: Terry Tritton
---
tools/testing/selftests/mm/uffd-unit-tests.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c
b/tools/testing/selftests/mm/uffd-unit-tests.c
index cce90a10515a..2b9f8cc52639 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -1517,6 +1517,12 @@ int main(int argc, char *argv[])
continue;
uffd_test_start("%s on %s", test->name, mem_type->name);
+ if ((mem_type->mem_flag == MEM_HUGETLB ||
+ mem_type->mem_flag == MEM_HUGETLB_PRIVATE) &&
+ (default_huge_page_size() == 0)) {
+ uffd_test_skip("huge page size is 0, feature
missing?");
+ continue;
+ }
if (!uffd_feature_supported(test)) {
uffd_test_skip("feature missing");
continue;
--
2.43.0.594.gd9cf4e227d-goog
Re: [PATCH v4 2/5] selftests/resctrl: Add helpers for the non-contiguous test
On 2024-02-05 at 15:16:27 +0200, Ilpo Järvinen wrote:
>On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
>
>> The CAT non-contiguous selftests have to read the file responsible for
>> reporting support of non-contiguous CBMs in kernel (resctrl). Then the
>> test compares if that information matches what is reported by CPUID
>> output.
>>
>> Add a generic helper function to read an unsigned number from a file in
>> /sys/fs/resctrl/info//.
>>
>> Signed-off-by: Maciej Wieczor-Retman
>> ---
>> Changelog v4:
>> - Rewrite function comment.
>> - Redo ksft_perror() as ksft_print_msg(). (Reinette)
>>
>> Changelog v3:
>> - Rewrite patch message.
>> - Add documentation and rewrote the function. (Reinette)
>>
>> Changelog v2:
>> - Add this patch.
>>
>> tools/testing/selftests/resctrl/resctrl.h | 1 +
>> tools/testing/selftests/resctrl/resctrlfs.c | 36 +
>> 2 files changed, 37 insertions(+)
>>
>> diff --git a/tools/testing/selftests/resctrl/resctrl.h
>> b/tools/testing/selftests/resctrl/resctrl.h
>> index a1462029998e..5116ea082d03 100644
>> --- a/tools/testing/selftests/resctrl/resctrl.h
>> +++ b/tools/testing/selftests/resctrl/resctrl.h
>> @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val,
>> unsigned int *start);
>> int get_full_cbm(const char *cache_type, unsigned long *mask);
>> int get_mask_no_shareable(const char *cache_type, unsigned long *mask);
>> int get_cache_size(int cpu_no, const char *cache_type, unsigned long
>> *cache_size);
>> +int resource_info_unsigned_get(const char *resource, const char *filename,
>> unsigned int *val);
>> void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
>> int signal_handler_register(void);
>> void signal_handler_unregister(void);
>> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
>> b/tools/testing/selftests/resctrl/resctrlfs.c
>> index 5750662cce57..e0fbc46a917a 100644
>> --- a/tools/testing/selftests/resctrl/resctrlfs.c
>> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
>> @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned
>> long *mask)
>> return 0;
>> }
>>
>> +/**
>> + * resource_info_unsigned_get - Read an unsigned value from
>> + * /sys/fs/resctrl/info/RESOURCE/FILENAME
>> + * @resource: Resource name that matches directory name in
>> + * /sys/fs/resctrl/info
>> + * @filename: File in /sys/fs/resctrl/info/@resource
>> + * @val:Contains read value on success.
>> + *
>> + * Return: = 0 on success, < 0 on failure. On success the read
>> + * value is saved into the @val.
>> + */
>> +int resource_info_unsigned_get(const char *resource, const char *filename,
>> + unsigned int *val)
>> +{
>> +char file_path[PATH_MAX];
>> +FILE *fp;
>> +
>> +snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource,
>> + filename);
>> +
>> +fp = fopen(file_path, "r");
>> +if (!fp) {
>> +ksft_print_msg("Error in opening %s\n: %m\n", file_path);
>
>Adding the extra \n in between there will likely mess up the test output
>formatting (the expected prefixes will not appear). Therefore, manually
>adding newlines should be avoided.
>
>> +return -1;
>> +}
>> +
>> +if (fscanf(fp, "%u", val) <= 0) {
>> +ksft_print_msg("Could not get contents of %s\n: %m\n",
>> file_path);
>
>Ditto.
I thought I've seen some other error checking paths where '%m' was on the next
line but I couldn't find it again right now so I'll remove the newline.
>
>With those two fixed,
>
>Reviewed-by: Ilpo Järvinen
>
>
>--
> i.
--
Kind regards
Maciej Wieczór-Retman
Re: [PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT test
On 2024-02-05 at 15:11:22 +0200, Ilpo Järvinen wrote:
>On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
>
>> Add tests for both L2 and L3 CAT to verify the return values
>> generated by writing non-contiguous CBMs don't contradict the
>> reported non-contiguous support information.
>>
>> Use a logical XOR to confirm return value of write_schemata() and
>> non-contiguous CBMs support information match.
>>
>> Signed-off-by: Maciej Wieczor-Retman
>> ---
>> Changelog v4:
>> - Return failure instead of error on check of cpuid against sparse_masks
>> and on contiguous write_schemata fail. (Reinette)
>>
>> Changelog v3:
>> - Roll back __cpuid_count part. (Reinette)
>> - Update function name to read sparse_masks file.
>> - Roll back get_cache_level() changes.
>> - Add ksft_print_msg() to contiguous schemata write error handling
>> (Reinette).
>>
>> Changelog v2:
>> - Redo the patch message. (Ilpo)
>> - Tidy up __cpuid_count calls. (Ilpo)
>> - Remove redundant AND in noncont_mask calculations (Ilpo)
>> - Fix bit_center offset.
>> - Add newline before function return. (Ilpo)
>> - Group non-contiguous tests with CAT tests. (Ilpo)
>> - Use a helper for reading sparse_masks file. (Ilpo)
>> - Make get_cache_level() available in other source files. (Ilpo)
>>
>> tools/testing/selftests/resctrl/cat_test.c| 81 +++
>> tools/testing/selftests/resctrl/resctrl.h | 2 +
>> .../testing/selftests/resctrl/resctrl_tests.c | 2 +
>> 3 files changed, 85 insertions(+)
>>
>> diff --git a/tools/testing/selftests/resctrl/cat_test.c
>> b/tools/testing/selftests/resctrl/cat_test.c
>> index 39fc9303b8e8..20eb978e624b 100644
>> --- a/tools/testing/selftests/resctrl/cat_test.c
>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>> @@ -294,6 +294,71 @@ static int cat_run_test(const struct resctrl_test
>> *test, const struct user_param
>> return ret;
>> }
>>
>> +static int noncont_cat_run_test(const struct resctrl_test *test,
>> +const struct user_params *uparams)
>> +{
>> +unsigned long full_cache_mask, cont_mask, noncont_mask;
>> +unsigned int eax, ebx, ecx, edx, ret, sparse_masks;
>> +char schemata[64];
>> +int bit_center;
>> +
>> +/* Check to compare sparse_masks content to CPUID output. */
>> +ret = resource_info_unsigned_get(test->resource, "sparse_masks",
>> _masks);
>> +if (ret)
>> +return ret;
>> +
>> +if (!strcmp(test->resource, "L3"))
>> +__cpuid_count(0x10, 1, eax, ebx, ecx, edx);
>> +else if (!strcmp(test->resource, "L2"))
>> +__cpuid_count(0x10, 2, eax, ebx, ecx, edx);
>> +else
>> +return -EINVAL;
>> +
>> +if (sparse_masks != ((ecx >> 3) & 1)) {
>> +ksft_print_msg("CPUID output doesn't match 'sparse_masks' file
>> content!\n");
>> +return 1;
>> +}
>> +
>> +/* Write checks initialization. */
>> +ret = get_full_cbm(test->resource, _cache_mask);
>> +if (ret < 0)
>> +return ret;
>> +bit_center = count_bits(full_cache_mask) / 2;
>> +cont_mask = full_cache_mask >> bit_center;
>> +
>> +/* Contiguous mask write check. */
>> +snprintf(schemata, sizeof(schemata), "%lx", cont_mask);
>> +ret = write_schemata("", schemata, uparams->cpu, test->resource);
>> +if (ret) {
>> +ksft_print_msg("Write of contiguous CBM failed\n");
>> +return 1;
>> +}
>> +
>> +/*
>> + * Non-contiguous mask write check. CBM has a 0xf hole approximately in
>> the middle.
>> + * Output is compared with support information to catch any edge case
>> errors.
>> + */
>> +noncont_mask = ~(0xf << (bit_center - 2)) & full_cache_mask;
>
>To be on the safe side, I think the types could be made to match here
>with 0xfUL to avoid sizeof(int) vs sizeof(unsigned long) related bit
>drops in the & (although it feel somewhat theoretical given the bitmask
>sizes we are currently seeing).
Sure, I'll add that for the next version. Thanks!
>
>Reviewed-by: Ilpo Järvinen
>
>
>--
> i.
--
Kind regards
Maciej Wieczór-Retman
Re: [PATCH v4 3/5] selftests/resctrl: Split validate_resctrl_feature_request()
On 2024-02-05 at 14:41:30 +0200, Ilpo Järvinen wrote:
>On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
>
>> validate_resctrl_feature_request() is used to test both if a resource is
>> present in the info directory, and if a passed monitoring feature is
>> present in the mon_features file.
>>
>> Refactor validate_resctrl_feature_request() into two smaller functions
>> that each accomplish one check to give feature checking more
>> granularity:
>> - Resource directory presence in the /sys/fs/resctrl/info directory.
>> - Feature name presence in the /sys/fs/resctrl/info/L3_MON/mon_features
>> file.
>>
>> Signed-off-by: Maciej Wieczor-Retman
>> ---
>> Changelog v4:
>> - Roll back to using test_resource_feature_check() for CMT and MBA.
>> (Ilpo).
>>
>> Changelog v3:
>> - Move new function to a separate patch. (Reinette)
>> - Rewrite resctrl_mon_feature_exists() only for L3_MON.
>>
>> Changelog v2:
>> - Add this patch.
>>
>> tools/testing/selftests/resctrl/cmt_test.c | 2 +-
>> tools/testing/selftests/resctrl/mba_test.c | 2 +-
>> tools/testing/selftests/resctrl/mbm_test.c | 6 ++--
>> tools/testing/selftests/resctrl/resctrl.h | 3 +-
>> tools/testing/selftests/resctrl/resctrlfs.c | 33 +
>> 5 files changed, 28 insertions(+), 18 deletions(-)
>>
>
>> diff --git a/tools/testing/selftests/resctrl/cmt_test.c
>> b/tools/testing/selftests/resctrl/cmt_test.c
>> index dd5ca343c469..c1157917a814 100644
>> --- a/tools/testing/selftests/resctrl/cmt_test.c
>> +++ b/tools/testing/selftests/resctrl/cmt_test.c
>> @@ -170,7 +170,7 @@ static int cmt_run_test(const struct resctrl_test *test,
>> const struct user_param
>> static bool cmt_feature_check(const struct resctrl_test *test)
>> {
>> return test_resource_feature_check(test) &&
>> - validate_resctrl_feature_request("L3_MON", "llc_occupancy");
>> + resctrl_resource_exists("L3");
>
>This not correctly transformed.
Oops, sorry, I'll fix it for the next version.
>
>> +/*
>> + * resctrl_mon_feature_exists - Check if requested monitoring L3_MON
>> feature is valid.
>> + * @feature:Required monitor feature (in mon_features file).
>> + *
>> + * Return: True if the feature is supported, else false.
>> + */
>> +bool resctrl_mon_feature_exists(const char *feature)
>> +{
>> +char *res;
>> +FILE *inf;
>> +
>> if (!feature)
>> -return true;
>> +return false;
>>
>> -snprintf(res_path, sizeof(res_path), "%s/%s/mon_features", INFO_PATH,
>> resource);
>> -inf = fopen(res_path, "r");
>> +inf = fopen("/sys/fs/resctrl/info/L3_MON/mon_features", "r");
>
>This became less generic? Could there be other MON resource besides L3
>one? Perhaps there aren't today but why remove the ability give it as a
>parameter?
During v2 discussion [1] Reinette made me realize this functionality only
interfaces with L3_MON/mon_features file and the 'resource' parameter isn't
needed. The 'mon_features' file is only mentioned for L3_MON and I don't know of
any plans for other MON resources so I assumed it doesn't need to be generic.
But sure, I can make it use a parameter if Reinette doesn't mind.
[1]
https://lore.kernel.org/all/2o7adr2cos6qcikcu7oop4ss7vib2n6ue33djgfeds3v6gj53f@uu45lomrp5qv/
>
>
>--
> i.
>
--
Kind regards
Maciej Wieczór-Retman
Re: [PATCH] selftests/ftrace: Limit length in subsystem-enable tests
On Mon, 5 Feb 2024 21:12:33 +0800
Yuanhe Shu wrote:
> While sched* events being traced and sched* events continuously happen,
> "[xx] event tracing - enable/disable with subsystem level files" would
> never stop as it cat an endless output.
> Select the first 100 lines of output would be enough to judge whether
> there are more than 3 types of sched events.
It's not that it never stops but on some slower systems it does seem to
take forever.
Acked-by: Steven Rostedt (Google)
Shuah,
Can you take this through your tree?
Thanks,
-- Steve
>
> Signed-off-by: Yuanhe Shu
> ---
> .../selftests/ftrace/test.d/event/subsystem-enable.tc | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
> b/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
> index b1ede6249866..74c1114603a7 100644
> --- a/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
> +++ b/tools/testing/selftests/ftrace/test.d/event/subsystem-enable.tc
> @@ -18,7 +18,7 @@ echo 'sched:*' > set_event
>
> yield
>
> -count=`cat trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
> +count=`head -n 100 trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
> if [ $count -lt 3 ]; then
> fail "at least fork, exec and exit events should be recorded"
> fi
> @@ -29,7 +29,7 @@ echo 1 > events/sched/enable
>
> yield
>
> -count=`cat trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
> +count=`head -n 100 | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
> if [ $count -lt 3 ]; then
> fail "at least fork, exec and exit events should be recorded"
> fi
> @@ -40,7 +40,7 @@ echo 0 > events/sched/enable
>
> yield
>
> -count=`cat trace | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
> +count=`head -n 100 | grep -v ^# | awk '{ print $5 }' | sort -u | wc -l`
> if [ $count -ne 0 ]; then
> fail "any of scheduler events should not be recorded"
> fi
Re: [PATCH v4 2/5] selftests/resctrl: Add helpers for the non-contiguous test
On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
> The CAT non-contiguous selftests have to read the file responsible for
> reporting support of non-contiguous CBMs in kernel (resctrl). Then the
> test compares if that information matches what is reported by CPUID
> output.
>
> Add a generic helper function to read an unsigned number from a file in
> /sys/fs/resctrl/info//.
>
> Signed-off-by: Maciej Wieczor-Retman
> ---
> Changelog v4:
> - Rewrite function comment.
> - Redo ksft_perror() as ksft_print_msg(). (Reinette)
>
> Changelog v3:
> - Rewrite patch message.
> - Add documentation and rewrote the function. (Reinette)
>
> Changelog v2:
> - Add this patch.
>
> tools/testing/selftests/resctrl/resctrl.h | 1 +
> tools/testing/selftests/resctrl/resctrlfs.c | 36 +
> 2 files changed, 37 insertions(+)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index a1462029998e..5116ea082d03 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val,
> unsigned int *start);
> int get_full_cbm(const char *cache_type, unsigned long *mask);
> int get_mask_no_shareable(const char *cache_type, unsigned long *mask);
> int get_cache_size(int cpu_no, const char *cache_type, unsigned long
> *cache_size);
> +int resource_info_unsigned_get(const char *resource, const char *filename,
> unsigned int *val);
> void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
> int signal_handler_register(void);
> void signal_handler_unregister(void);
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index 5750662cce57..e0fbc46a917a 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned
> long *mask)
> return 0;
> }
>
> +/**
> + * resource_info_unsigned_get - Read an unsigned value from
> + * /sys/fs/resctrl/info/RESOURCE/FILENAME
> + * @resource:Resource name that matches directory name in
> + * /sys/fs/resctrl/info
> + * @filename:File in /sys/fs/resctrl/info/@resource
> + * @val: Contains read value on success.
> + *
> + * Return: = 0 on success, < 0 on failure. On success the read
> + * value is saved into the @val.
> + */
> +int resource_info_unsigned_get(const char *resource, const char *filename,
> +unsigned int *val)
> +{
> + char file_path[PATH_MAX];
> + FILE *fp;
> +
> + snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource,
> + filename);
> +
> + fp = fopen(file_path, "r");
> + if (!fp) {
> + ksft_print_msg("Error in opening %s\n: %m\n", file_path);
Adding the extra \n in between there will likely mess up the test output
formatting (the expected prefixes will not appear). Therefore, manually
adding newlines should be avoided.
> + return -1;
> + }
> +
> + if (fscanf(fp, "%u", val) <= 0) {
> + ksft_print_msg("Could not get contents of %s\n: %m\n",
> file_path);
Ditto.
With those two fixed,
Reviewed-by: Ilpo Järvinen
--
i.
Re: [PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT test
On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
> Add tests for both L2 and L3 CAT to verify the return values
> generated by writing non-contiguous CBMs don't contradict the
> reported non-contiguous support information.
>
> Use a logical XOR to confirm return value of write_schemata() and
> non-contiguous CBMs support information match.
>
> Signed-off-by: Maciej Wieczor-Retman
> ---
> Changelog v4:
> - Return failure instead of error on check of cpuid against sparse_masks
> and on contiguous write_schemata fail. (Reinette)
>
> Changelog v3:
> - Roll back __cpuid_count part. (Reinette)
> - Update function name to read sparse_masks file.
> - Roll back get_cache_level() changes.
> - Add ksft_print_msg() to contiguous schemata write error handling
> (Reinette).
>
> Changelog v2:
> - Redo the patch message. (Ilpo)
> - Tidy up __cpuid_count calls. (Ilpo)
> - Remove redundant AND in noncont_mask calculations (Ilpo)
> - Fix bit_center offset.
> - Add newline before function return. (Ilpo)
> - Group non-contiguous tests with CAT tests. (Ilpo)
> - Use a helper for reading sparse_masks file. (Ilpo)
> - Make get_cache_level() available in other source files. (Ilpo)
>
> tools/testing/selftests/resctrl/cat_test.c| 81 +++
> tools/testing/selftests/resctrl/resctrl.h | 2 +
> .../testing/selftests/resctrl/resctrl_tests.c | 2 +
> 3 files changed, 85 insertions(+)
>
> diff --git a/tools/testing/selftests/resctrl/cat_test.c
> b/tools/testing/selftests/resctrl/cat_test.c
> index 39fc9303b8e8..20eb978e624b 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -294,6 +294,71 @@ static int cat_run_test(const struct resctrl_test *test,
> const struct user_param
> return ret;
> }
>
> +static int noncont_cat_run_test(const struct resctrl_test *test,
> + const struct user_params *uparams)
> +{
> + unsigned long full_cache_mask, cont_mask, noncont_mask;
> + unsigned int eax, ebx, ecx, edx, ret, sparse_masks;
> + char schemata[64];
> + int bit_center;
> +
> + /* Check to compare sparse_masks content to CPUID output. */
> + ret = resource_info_unsigned_get(test->resource, "sparse_masks",
> _masks);
> + if (ret)
> + return ret;
> +
> + if (!strcmp(test->resource, "L3"))
> + __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
> + else if (!strcmp(test->resource, "L2"))
> + __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
> + else
> + return -EINVAL;
> +
> + if (sparse_masks != ((ecx >> 3) & 1)) {
> + ksft_print_msg("CPUID output doesn't match 'sparse_masks' file
> content!\n");
> + return 1;
> + }
> +
> + /* Write checks initialization. */
> + ret = get_full_cbm(test->resource, _cache_mask);
> + if (ret < 0)
> + return ret;
> + bit_center = count_bits(full_cache_mask) / 2;
> + cont_mask = full_cache_mask >> bit_center;
> +
> + /* Contiguous mask write check. */
> + snprintf(schemata, sizeof(schemata), "%lx", cont_mask);
> + ret = write_schemata("", schemata, uparams->cpu, test->resource);
> + if (ret) {
> + ksft_print_msg("Write of contiguous CBM failed\n");
> + return 1;
> + }
> +
> + /*
> + * Non-contiguous mask write check. CBM has a 0xf hole approximately in
> the middle.
> + * Output is compared with support information to catch any edge case
> errors.
> + */
> + noncont_mask = ~(0xf << (bit_center - 2)) & full_cache_mask;
To be on the safe side, I think the types could be made to match here
with 0xfUL to avoid sizeof(int) vs sizeof(unsigned long) related bit
drops in the & (although it feel somewhat theoretical given the bitmask
sizes we are currently seeing).
Reviewed-by: Ilpo Järvinen
--
i.
Re: [PATCH v5 00/12] RISCV: Add kvm Sstc timer selftests
Hi Marc, Could you help review the first 3 patches in this series? Thanks, Haibo On Mon, Jan 22, 2024 at 5:45 PM Haibo Xu wrote: > > The RISC-V arch_timer selftests is used to validate Sstc timer > functionality in a guest, which sets up periodic timer interrupts > and check the basic interrupt status upon its receipt. > > This KVM selftests was ported from aarch64 arch_timer and tested > with Linux v6.7-rc8 on a Qemu riscv64 virt machine. > > --- > Changed since v4: > * Rebased to Linux 6.7-rc8 > * Added new patch(2/12) to clean up the data type in struct test_args > * Re-ordered patch(11/11) in v4 to patch(3/12) > * Changed the timer_err_margin_us type from int to uint32_t > > Haibo Xu (11): > KVM: arm64: selftests: Data type cleanup for arch_timer test > KVM: arm64: selftests: Enable tuning of error margin in arch_timer > test > KVM: arm64: selftests: Split arch_timer test code > KVM: selftests: Add CONFIG_64BIT definition for the build > tools: riscv: Add header file csr.h > tools: riscv: Add header file vdso/processor.h > KVM: riscv: selftests: Switch to use macro from csr.h > KVM: riscv: selftests: Add exception handling support > KVM: riscv: selftests: Add guest helper to get vcpu id > KVM: riscv: selftests: Change vcpu_has_ext to a common function > KVM: riscv: selftests: Add sstc timer test > > Paolo Bonzini (1): > selftests/kvm: Fix issues with $(SPLIT_TESTS) > > tools/arch/riscv/include/asm/csr.h| 541 ++ > tools/arch/riscv/include/asm/vdso/processor.h | 32 ++ > tools/testing/selftests/kvm/Makefile | 27 +- > .../selftests/kvm/aarch64/arch_timer.c| 295 +- > tools/testing/selftests/kvm/arch_timer.c | 259 + > .../selftests/kvm/include/aarch64/processor.h | 4 - > .../selftests/kvm/include/kvm_util_base.h | 9 + > .../selftests/kvm/include/riscv/arch_timer.h | 71 +++ > .../selftests/kvm/include/riscv/processor.h | 65 ++- > .../testing/selftests/kvm/include/test_util.h | 2 + > .../selftests/kvm/include/timer_test.h| 45 ++ > .../selftests/kvm/lib/riscv/handlers.S| 101 > .../selftests/kvm/lib/riscv/processor.c | 87 +++ > .../testing/selftests/kvm/riscv/arch_timer.c | 111 > .../selftests/kvm/riscv/get-reg-list.c| 11 +- > 15 files changed, 1353 insertions(+), 307 deletions(-) > create mode 100644 tools/arch/riscv/include/asm/csr.h > create mode 100644 tools/arch/riscv/include/asm/vdso/processor.h > create mode 100644 tools/testing/selftests/kvm/arch_timer.c > create mode 100644 tools/testing/selftests/kvm/include/riscv/arch_timer.h > create mode 100644 tools/testing/selftests/kvm/include/timer_test.h > create mode 100644 tools/testing/selftests/kvm/lib/riscv/handlers.S > create mode 100644 tools/testing/selftests/kvm/riscv/arch_timer.c > > -- > 2.34.1 >
Re: [PATCH v4 3/5] selftests/resctrl: Split validate_resctrl_feature_request()
On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
> validate_resctrl_feature_request() is used to test both if a resource is
> present in the info directory, and if a passed monitoring feature is
> present in the mon_features file.
>
> Refactor validate_resctrl_feature_request() into two smaller functions
> that each accomplish one check to give feature checking more
> granularity:
> - Resource directory presence in the /sys/fs/resctrl/info directory.
> - Feature name presence in the /sys/fs/resctrl/info/L3_MON/mon_features
> file.
>
> Signed-off-by: Maciej Wieczor-Retman
> ---
> Changelog v4:
> - Roll back to using test_resource_feature_check() for CMT and MBA.
> (Ilpo).
>
> Changelog v3:
> - Move new function to a separate patch. (Reinette)
> - Rewrite resctrl_mon_feature_exists() only for L3_MON.
>
> Changelog v2:
> - Add this patch.
>
> tools/testing/selftests/resctrl/cmt_test.c | 2 +-
> tools/testing/selftests/resctrl/mba_test.c | 2 +-
> tools/testing/selftests/resctrl/mbm_test.c | 6 ++--
> tools/testing/selftests/resctrl/resctrl.h | 3 +-
> tools/testing/selftests/resctrl/resctrlfs.c | 33 +
> 5 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cmt_test.c
> b/tools/testing/selftests/resctrl/cmt_test.c
> index dd5ca343c469..c1157917a814 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -170,7 +170,7 @@ static int cmt_run_test(const struct resctrl_test *test,
> const struct user_param
> static bool cmt_feature_check(const struct resctrl_test *test)
> {
> return test_resource_feature_check(test) &&
> -validate_resctrl_feature_request("L3_MON", "llc_occupancy");
> +resctrl_resource_exists("L3");
This not correctly transformed.
> +/*
> + * resctrl_mon_feature_exists - Check if requested monitoring L3_MON feature
> is valid.
> + * @feature: Required monitor feature (in mon_features file).
> + *
> + * Return: True if the feature is supported, else false.
> + */
> +bool resctrl_mon_feature_exists(const char *feature)
> +{
> + char *res;
> + FILE *inf;
> +
> if (!feature)
> - return true;
> + return false;
>
> - snprintf(res_path, sizeof(res_path), "%s/%s/mon_features", INFO_PATH,
> resource);
> - inf = fopen(res_path, "r");
> + inf = fopen("/sys/fs/resctrl/info/L3_MON/mon_features", "r");
This became less generic? Could there be other MON resource besides L3
one? Perhaps there aren't today but why remove the ability give it as a
parameter?
--
i.
Re: [PATCH net] selftests: net: let big_tcp test cope with slow env
Hello: This patch was applied to netdev/net.git (main) by David S. Miller : On Fri, 2 Feb 2024 17:06:59 +0100 you wrote: > In very slow environments, most big TCP cases including > segmentation and reassembly of big TCP packets have a good > chance to fail: by default the TCP client uses write size > well below 64K. If the host is low enough autocorking is > unable to build real big TCP packets. > > Address the issue using much larger write operations. > > [...] Here is the summary with links: - [net] selftests: net: let big_tcp test cope with slow env https://git.kernel.org/netdev/net/c/a19747c3b9bf You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH v8 13/38] KVM: arm64: Manage GCS registers for guests
On Mon, Feb 05, 2024 at 09:46:16AM +, Marc Zyngier wrote:
> On Sat, 03 Feb 2024 12:25:39 +,
> Mark Brown wrote:
> > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > @@ -25,6 +25,8 @@ 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 (has_gcs())
> > + ctxt_sys_reg(ctxt, GCSPR_EL0) = read_sysreg_s(SYS_GCSPR_EL0);
> We have had this discussion in the past. This must be based on the
> VM's configuration. Guarding the check with the host capability is a
> valuable optimisation, but that's nowhere near enough. See the series
> that I have posted on this very subject (you're on Cc), but you are
> welcome to invent your own mechanism in the meantime.
Right, which postdates the version you're replying to and isn't merged
yet - the current code was what you were asking for at the time. I'm
expecting to update all these feature series to work with that once it
gets finalised and merged but it's not there yet, I do see I forgot to
put a note in v9 about that like I did for dpISA - sorry about that, I
was too focused on the clone3() rework when rebasing onto the new
kernel.
This particular series isn't going to get merged for a while yet anyway
due to the time it'll take for userspace testing, I'm expecting your
series to be in by the time it becomes an issue.
> > + if (has_gcs()) {
> > + write_sysreg_el1(ctxt_sys_reg(ctxt, GCSPR_EL1), SYS_GCSPR);
> > + write_sysreg_el1(ctxt_sys_reg(ctxt, GCSCR_EL1), SYS_GCSCR);
> > + write_sysreg_s(ctxt_sys_reg(ctxt, GCSCRE0_EL1),
> > + SYS_GCSCRE0_EL1);
> > + }
> For the benefit of the unsuspecting reviewers, and in the absence of a
> public specification (which the XML drop isn't), it would be good to
> have the commit message explaining the rationale of what gets saved
> when.
What are you looking for in terms of rationale here? The KVM house
style is often very reliant on reader context so it would be good to
know what considerations you'd like to see explicitly addressed. These
registers shouldn't do anything when we aren't running the guest so
they're not terribly ordering sensitive, the EL2 ones will need a bit
more consideration in the face of nested virt.
signature.asc
Description: PGP signature
[PATCH 2/3] Documentation: adjust pstore backend related document
Pstore now supports multiple backends, adjust related document. Signed-off-by: Yuanhe Shu --- Documentation/ABI/testing/pstore| 8 Documentation/admin-guide/kernel-parameters.txt | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/ABI/testing/pstore b/Documentation/ABI/testing/pstore index d3cff4a7ee10..2cd67b502f11 100644 --- a/Documentation/ABI/testing/pstore +++ b/Documentation/ABI/testing/pstore @@ -41,7 +41,7 @@ Description: Generic interface to platform dependent persistent storage. persistent storage until at least this amount is reached. Default is 10 Kbytes. - Pstore only supports one backend at a time. If multiple - backends are available, the preferred backend may be - set by passing the pstore.backend= argument to the kernel at - boot time. + Pstore now supports multiple backends at a time. If multiple + backends are available, the registrable backends may be + set by passing the pstore.backend= argument1, argument2... + or pstore.backend= all to the kernel at boot time. diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 31b3a25680d0..a8a109b822a9 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4748,7 +4748,9 @@ [HW,MOUSE] Controls Logitech smartscroll autorepeat. 0 = disabled, 1 = enabled (default). - pstore.backend= Specify the name of the pstore backend to use + pstore.backend=backend1,...,backendN + Specify the names of the pstore backends to use + or =all for all available backends pti=[X86-64] Control Page Table Isolation of user and kernel address spaces. Disabling this feature -- 2.39.3
[PATCH 3/3] tools/testing: adjust pstore backend related selftest
Pstore now supports multiple backends, the module parameter
pstore.backend varies from 'registered backend' to 'backends that are
allowed to register'. Adjust selftests to match the change.
Signed-off-by: Yuanhe Shu
---
tools/testing/selftests/pstore/common_tests | 8 +--
.../selftests/pstore/pstore_post_reboot_tests | 65 ++-
tools/testing/selftests/pstore/pstore_tests | 2 +-
3 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/tools/testing/selftests/pstore/common_tests
b/tools/testing/selftests/pstore/common_tests
index 4509f0cc9c91..497e6fc3215f 100755
--- a/tools/testing/selftests/pstore/common_tests
+++ b/tools/testing/selftests/pstore/common_tests
@@ -27,9 +27,9 @@ show_result() { # result_value
}
check_files_exist() { # type of pstorefs file
-if [ -e ${1}-${backend}-0 ]; then
+if [ -e ${1}-${2}-0 ]; then
prlog "ok"
- for f in `ls ${1}-${backend}-*`; do
+ for f in `ls ${1}-${2}-*`; do
prlog -e "\t${f}"
done
else
@@ -74,9 +74,9 @@ prlog "=== Pstore unit tests (`basename $0`) ==="
prlog "UUID="$UUID
prlog -n "Checking pstore backend is registered ... "
-backend=`cat /sys/module/pstore/parameters/backend`
+backends=$(dmesg | sed -n 's/.*pstore: Registered \(.*\) as persistent store
backend.*/\1/p')
show_result $?
-prlog -e "\tbackend=${backend}"
+prlog -e "\tbackends="$backends
prlog -e "\tcmdline=`cat /proc/cmdline`"
if [ $rc -ne 0 ]; then
exit 1
diff --git a/tools/testing/selftests/pstore/pstore_post_reboot_tests
b/tools/testing/selftests/pstore/pstore_post_reboot_tests
index d6da5e86efbf..9e40ccb9c918 100755
--- a/tools/testing/selftests/pstore/pstore_post_reboot_tests
+++ b/tools/testing/selftests/pstore/pstore_post_reboot_tests
@@ -36,45 +36,46 @@ else
fi
cd ${mount_point}
+for backend in ${backends}; do
+prlog -n "Checking ${backend}-dmesg files exist in pstore filesystem ... "
+check_files_exist dmesg ${backend}
-prlog -n "Checking dmesg files exist in pstore filesystem ... "
-check_files_exist dmesg
+prlog -n "Checking ${backend}-console files exist in pstore filesystem ...
"
+check_files_exist console ${backend}
-prlog -n "Checking console files exist in pstore filesystem ... "
-check_files_exist console
+prlog -n "Checking ${backend}-pmsg files exist in pstore filesystem ... "
+check_files_exist pmsg ${backend}
-prlog -n "Checking pmsg files exist in pstore filesystem ... "
-check_files_exist pmsg
+prlog -n "Checking ${backend}-dmesg files contain oops end marker"
+grep_end_trace() {
+grep -q "\---\[ end trace" $1
+}
+files=`ls dmesg-${backend}-*`
+operate_files $? "$files" grep_end_trace
-prlog -n "Checking dmesg files contain oops end marker"
-grep_end_trace() {
-grep -q "\---\[ end trace" $1
-}
-files=`ls dmesg-${backend}-*`
-operate_files $? "$files" grep_end_trace
+prlog -n "Checking ${backend}-console file contains oops end marker ... "
+grep -q "\---\[ end trace" console-${backend}-0
+show_result $?
-prlog -n "Checking console file contains oops end marker ... "
-grep -q "\---\[ end trace" console-${backend}-0
-show_result $?
-
-prlog -n "Checking pmsg file properly keeps the content written before crash
... "
-prev_uuid=`cat $TOP_DIR/prev_uuid`
-if [ $? -eq 0 ]; then
-nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
-if [ $nr_matched -eq 1 ]; then
- grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
- show_result $?
+prlog -n "Checking ${backend}-pmsg file properly keeps the content written
before crash ... "
+prev_uuid=`cat $TOP_DIR/prev_uuid`
+if [ $? -eq 0 ]; then
+nr_matched=`grep -c "$TEST_STRING_PATTERN" pmsg-${backend}-0`
+if [ $nr_matched -eq 1 ]; then
+ grep -q "$TEST_STRING_PATTERN"$prev_uuid pmsg-${backend}-0
+ show_result $?
+else
+prlog "FAIL"
+rc=1
+fi
else
- prlog "FAIL"
- rc=1
+prlog "FAIL"
+rc=1
fi
-else
-prlog "FAIL"
-rc=1
-fi
-prlog -n "Removing all files in pstore filesystem "
-files=`ls *-${backend}-*`
-operate_files $? "$files" rm
+prlog -n "Removing all ${backend} files in pstore filesystem "
+files=`ls *-${backend}-*`
+operate_files $? "$files" rm
+done
exit $rc
diff --git a/tools/testing/selftests/pstore/pstore_tests
b/tools/testing/selftests/pstore/pstore_tests
index 2aa9a3852a84..f4665a8c77dc 100755
--- a/tools/testing/selftests/pstore/pstore_tests
+++ b/tools/testing/selftests/pstore/pstore_tests
@@ -10,7 +10,7 @@
. ./common_tests
prlog -n "Checking pstore console is registered ... "
-dmesg | grep -Eq "console \[(pstore|${backend})"
+dmesg | grep -Eq "console \[(pstore console)"
show_result $?
prlog -n "Checking /dev/pmsg0 exists ... "
--
2.39.3
[PATCH 1/3] pstore: add multi-backend support
Currently, pstore supports only one backend open at a time.
Specifically, due to the global variable "psinfo", pstore only accepts
the first registered backend. If a new backend wants to register later,
pstore will simply reject it and return an error. This design forced us
to close existing backend in order to use the new ones.
To enable pstore to support multiple backends, "psinfo" is replaced by
"psinfo_list", a list that holds multiple "psinfo". If multiple backends
are registered with the same frontend, the frontend is reused.
User can specify multiple backends that are allowed to be registered by
module parameter "pstore.backend=" separated by commas or "all" to
enable all available backends. If no pstore.backend was specified,
pstore would accept the first registered backend which is the same as
before.
Signed-off-by: Xingrui Yi
Signed-off-by: Yuanhe Shu
---
fs/pstore/ftrace.c | 29 +-
fs/pstore/inode.c | 19 ++--
fs/pstore/internal.h | 4 +-
fs/pstore/platform.c | 225 -
fs/pstore/pmsg.c | 24 -
include/linux/pstore.h | 29 ++
6 files changed, 248 insertions(+), 82 deletions(-)
diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c
index 776cae20af4e..2532a663aa2c 100644
--- a/fs/pstore/ftrace.c
+++ b/fs/pstore/ftrace.c
@@ -23,10 +23,11 @@
/* This doesn't need to be atomic: speed is chosen over correctness here. */
static u64 pstore_ftrace_stamp;
-static void notrace pstore_ftrace_call(unsigned long ip,
+static void notrace pstore_do_ftrace(unsigned long ip,
unsigned long parent_ip,
struct ftrace_ops *op,
- struct ftrace_regs *fregs)
+ struct ftrace_regs *fregs,
+ struct ftrace_info *psinfo)
{
int bit;
unsigned long flags;
@@ -57,6 +58,20 @@ static void notrace pstore_ftrace_call(unsigned long ip,
ftrace_test_recursion_unlock(bit);
}
+static void notrace pstore_ftrace_call(unsigned long ip,
+ unsigned long parent_ip,
+ struct ftrace_ops *op,
+ struct pt_regs *regs)
+{
+ struct pstore_info_list *entry;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, >list_entry, list)
+ if (entry->psi->flags & PSTORE_FLAGS_FTRACE)
+ pstore_do_ftrace(ip, parent_ip, op, regs, entry->psi);
+ rcu_read_unlock();
+}
+
static struct ftrace_ops pstore_ftrace_ops __read_mostly = {
.func = pstore_ftrace_call,
};
@@ -131,8 +146,14 @@ MODULE_PARM_DESC(record_ftrace,
void pstore_register_ftrace(void)
{
- if (!psinfo->write)
- return;
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, >list_entry, list)
+ if (entry->psi->flags & PSTORE_FLAGS_FTRACE)
+ if (!entry->psi->write) {
+ rcu_read_unlock();
+ return;
+ }
+ rcu_read_unlock();
pstore_ftrace_dir = debugfs_create_dir("pstore", NULL);
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index d0d9bfdad30c..bee71c7da995 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -285,7 +285,7 @@ static const struct super_operations pstore_ops = {
.show_options = pstore_show_options,
};
-static struct dentry *psinfo_lock_root(void)
+static struct dentry *psinfo_lock_root(struct pstore_info *psinfo)
{
struct dentry *root;
@@ -309,7 +309,7 @@ int pstore_put_backend_records(struct pstore_info *psi)
struct dentry *root;
int rc = 0;
- root = psinfo_lock_root();
+ root = psinfo_lock_root(psi);
if (!root)
return 0;
@@ -398,21 +398,22 @@ int pstore_mkfile(struct dentry *root, struct
pstore_record *record)
* when we are re-scanning the backing store looking to add new
* error records.
*/
-void pstore_get_records(int quiet)
+void pstore_get_records(struct pstore_info *psi, int quiet)
{
struct dentry *root;
- root = psinfo_lock_root();
+ root = psinfo_lock_root(psi);
if (!root)
return;
- pstore_get_backend_records(psinfo, root, quiet);
+ pstore_get_backend_records(psi, root, quiet);
inode_unlock(d_inode(root));
}
static int pstore_fill_super(struct super_block *sb, void *data, int silent)
{
struct inode *inode;
+ struct pstore_info_list *entry;
sb->s_maxbytes = MAX_LFS_FILESIZE;
sb->s_blocksize = PAGE_SIZE;
@@ -437,7 +438,13 @@ static int pstore_fill_super(struct super_block *sb, void
*data, int silent)
scoped_guard(mutex, _sb_lock)
pstore_sb = sb;
- pstore_get_records(0);
+ if (!psback)
+
[PATCH v2 0/3] pstore: add multi-backend support
I have been steadily working but struggled to find a seamlessly integrated way to implement tty frontend until Guilherme inspired me that multi-backend and tty frontend are actually two separate entities. This submission presents the second iteration of my efforts, listing notable changes form the v1: 1. pstore.backend no longer acts as "registered backend", but "backends eligible for registration". 2. drop subdir since it will break user space 3. drop tty frontend since I haven't yet devised a satisfactory implementation strategy A heartfelt thank you to Kees and Guilherme for your suggestions. I firmly believe that a tty frontend is crucial for kdump debugging, and I am still dedicating effort to develop one. Hope in the future I can accomplish it with deeper comprehension with tty driver :) Yuanhe Shu (3): pstore: add multi-backend support Documentation: adjust pstore backend related document tools/testing: adjust pstore backend related selftest Documentation/ABI/testing/pstore | 8 +- .../admin-guide/kernel-parameters.txt | 4 +- fs/pstore/ftrace.c| 29 ++- fs/pstore/inode.c | 19 +- fs/pstore/internal.h | 4 +- fs/pstore/platform.c | 225 -- fs/pstore/pmsg.c | 24 +- include/linux/pstore.h| 29 +++ tools/testing/selftests/pstore/common_tests | 8 +- .../selftests/pstore/pstore_post_reboot_tests | 65 ++--- tools/testing/selftests/pstore/pstore_tests | 2 +- 11 files changed, 293 insertions(+), 124 deletions(-) -- 2.39.3
[PATCH v4 5/5] selftests/resctrl: Add non-contiguous CBMs CAT test
Add tests for both L2 and L3 CAT to verify the return values
generated by writing non-contiguous CBMs don't contradict the
reported non-contiguous support information.
Use a logical XOR to confirm return value of write_schemata() and
non-contiguous CBMs support information match.
Signed-off-by: Maciej Wieczor-Retman
---
Changelog v4:
- Return failure instead of error on check of cpuid against sparse_masks
and on contiguous write_schemata fail. (Reinette)
Changelog v3:
- Roll back __cpuid_count part. (Reinette)
- Update function name to read sparse_masks file.
- Roll back get_cache_level() changes.
- Add ksft_print_msg() to contiguous schemata write error handling
(Reinette).
Changelog v2:
- Redo the patch message. (Ilpo)
- Tidy up __cpuid_count calls. (Ilpo)
- Remove redundant AND in noncont_mask calculations (Ilpo)
- Fix bit_center offset.
- Add newline before function return. (Ilpo)
- Group non-contiguous tests with CAT tests. (Ilpo)
- Use a helper for reading sparse_masks file. (Ilpo)
- Make get_cache_level() available in other source files. (Ilpo)
tools/testing/selftests/resctrl/cat_test.c| 81 +++
tools/testing/selftests/resctrl/resctrl.h | 2 +
.../testing/selftests/resctrl/resctrl_tests.c | 2 +
3 files changed, 85 insertions(+)
diff --git a/tools/testing/selftests/resctrl/cat_test.c
b/tools/testing/selftests/resctrl/cat_test.c
index 39fc9303b8e8..20eb978e624b 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -294,6 +294,71 @@ static int cat_run_test(const struct resctrl_test *test,
const struct user_param
return ret;
}
+static int noncont_cat_run_test(const struct resctrl_test *test,
+ const struct user_params *uparams)
+{
+ unsigned long full_cache_mask, cont_mask, noncont_mask;
+ unsigned int eax, ebx, ecx, edx, ret, sparse_masks;
+ char schemata[64];
+ int bit_center;
+
+ /* Check to compare sparse_masks content to CPUID output. */
+ ret = resource_info_unsigned_get(test->resource, "sparse_masks",
_masks);
+ if (ret)
+ return ret;
+
+ if (!strcmp(test->resource, "L3"))
+ __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
+ else if (!strcmp(test->resource, "L2"))
+ __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
+ else
+ return -EINVAL;
+
+ if (sparse_masks != ((ecx >> 3) & 1)) {
+ ksft_print_msg("CPUID output doesn't match 'sparse_masks' file
content!\n");
+ return 1;
+ }
+
+ /* Write checks initialization. */
+ ret = get_full_cbm(test->resource, _cache_mask);
+ if (ret < 0)
+ return ret;
+ bit_center = count_bits(full_cache_mask) / 2;
+ cont_mask = full_cache_mask >> bit_center;
+
+ /* Contiguous mask write check. */
+ snprintf(schemata, sizeof(schemata), "%lx", cont_mask);
+ ret = write_schemata("", schemata, uparams->cpu, test->resource);
+ if (ret) {
+ ksft_print_msg("Write of contiguous CBM failed\n");
+ return 1;
+ }
+
+ /*
+* Non-contiguous mask write check. CBM has a 0xf hole approximately in
the middle.
+* Output is compared with support information to catch any edge case
errors.
+*/
+ noncont_mask = ~(0xf << (bit_center - 2)) & full_cache_mask;
+ snprintf(schemata, sizeof(schemata), "%lx", noncont_mask);
+ ret = write_schemata("", schemata, uparams->cpu, test->resource);
+ if (ret && sparse_masks)
+ ksft_print_msg("Non-contiguous CBMs supported but write of
non-contiguous CBM failed\n");
+ else if (ret && !sparse_masks)
+ ksft_print_msg("Non-contiguous CBMs not supported and write of
non-contiguous CBM failed as expected\n");
+ else if (!ret && !sparse_masks)
+ ksft_print_msg("Non-contiguous CBMs not supported but write of
non-contiguous CBM succeeded\n");
+
+ return !ret == !sparse_masks;
+}
+
+static bool noncont_cat_feature_check(const struct resctrl_test *test)
+{
+ if (!resctrl_resource_exists(test->resource))
+ return false;
+
+ return resource_info_file_exists(test->resource, "sparse_masks");
+}
+
struct resctrl_test l3_cat_test = {
.name = "L3_CAT",
.group = "CAT",
@@ -301,3 +366,19 @@ struct resctrl_test l3_cat_test = {
.feature_check = test_resource_feature_check,
.run_test = cat_run_test,
};
+
+struct resctrl_test l3_noncont_cat_test = {
+ .name = "L3_NONCONT_CAT",
+ .group = "CAT",
+ .resource = "L3",
+ .feature_check = noncont_cat_feature_check,
+ .run_test = noncont_cat_run_test,
+};
+
+struct resctrl_test l2_noncont_cat_test = {
+ .name = "L2_NONCONT_CAT",
+ .group = "CAT",
+ .resource = "L2",
+ .feature_check = noncont_cat_feature_check,
+ .run_test =
[PATCH v4 4/5] selftests/resctrl: Add resource_info_file_exists()
Feature checking done by resctrl_mon_feature_exists() covers features
represented by the feature name presence inside the 'mon_features' file
in /sys/fs/resctrl/info/L3_MON directory. There exists a different way
to represent feature support and that is by the presence of 0 or 1 in a
single file in the info/resource directory. In this case the filename
represents what feature support is being indicated.
Add a generic function to check file presence in the
/sys/fs/resctrl/info/ directory.
Signed-off-by: Maciej Wieczor-Retman
Reviewed-by: Ilpo Järvinen
---
Changelog v4:
- Remove unnecessary new lines.
- Change 'feature' -> 'file' to keep things generic. (Reinette)
- Add Ilpo's reviewed-by tag.
Changelog v3:
- Split off the new function into this patch. (Reinette)
Changelog v2:
- Add this patch.
tools/testing/selftests/resctrl/resctrl.h | 1 +
tools/testing/selftests/resctrl/resctrlfs.c | 25 +
2 files changed, 26 insertions(+)
diff --git a/tools/testing/selftests/resctrl/resctrl.h
b/tools/testing/selftests/resctrl/resctrl.h
index 4603b215b97e..2b9a3d0570c7 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -138,6 +138,7 @@ int umount_resctrlfs(void);
int validate_bw_report_request(char *bw_report);
bool resctrl_resource_exists(const char *resource);
bool resctrl_mon_feature_exists(const char *feature);
+bool resource_info_file_exists(const char *resource, const char *feature);
bool test_resource_feature_check(const struct resctrl_test *test);
char *fgrep(FILE *inf, const char *str);
int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
b/tools/testing/selftests/resctrl/resctrlfs.c
index 0cfec8bb23fd..6a3082ca58b5 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -760,6 +760,31 @@ bool resctrl_mon_feature_exists(const char *feature)
return !!res;
}
+/*
+ * resource_info_file_exists - Check if a file is present inside
+ * /sys/fs/resctrl/info/RESOURCE.
+ * @resource: Required resource (Eg: MB, L3, L2, etc.)
+ * @file: Required file.
+ *
+ * Return: True if the file exists, else false.
+ */
+bool resource_info_file_exists(const char *resource, const char *file)
+{
+ char res_path[PATH_MAX];
+ struct stat statbuf;
+
+ if (!file || !resource)
+ return false;
+
+ snprintf(res_path, sizeof(res_path), "%s/%s/%s", INFO_PATH, resource,
+file);
+
+ if (stat(res_path, ))
+ return false;
+
+ return true;
+}
+
bool test_resource_feature_check(const struct resctrl_test *test)
{
return resctrl_resource_exists(test->resource);
--
2.43.0
[PATCH v4 3/5] selftests/resctrl: Split validate_resctrl_feature_request()
validate_resctrl_feature_request() is used to test both if a resource is
present in the info directory, and if a passed monitoring feature is
present in the mon_features file.
Refactor validate_resctrl_feature_request() into two smaller functions
that each accomplish one check to give feature checking more
granularity:
- Resource directory presence in the /sys/fs/resctrl/info directory.
- Feature name presence in the /sys/fs/resctrl/info/L3_MON/mon_features
file.
Signed-off-by: Maciej Wieczor-Retman
---
Changelog v4:
- Roll back to using test_resource_feature_check() for CMT and MBA.
(Ilpo).
Changelog v3:
- Move new function to a separate patch. (Reinette)
- Rewrite resctrl_mon_feature_exists() only for L3_MON.
Changelog v2:
- Add this patch.
tools/testing/selftests/resctrl/cmt_test.c | 2 +-
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 6 ++--
tools/testing/selftests/resctrl/resctrl.h | 3 +-
tools/testing/selftests/resctrl/resctrlfs.c | 33 +
5 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cmt_test.c
b/tools/testing/selftests/resctrl/cmt_test.c
index dd5ca343c469..c1157917a814 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -170,7 +170,7 @@ static int cmt_run_test(const struct resctrl_test *test,
const struct user_param
static bool cmt_feature_check(const struct resctrl_test *test)
{
return test_resource_feature_check(test) &&
- validate_resctrl_feature_request("L3_MON", "llc_occupancy");
+ resctrl_resource_exists("L3");
}
struct resctrl_test cmt_test = {
diff --git a/tools/testing/selftests/resctrl/mba_test.c
b/tools/testing/selftests/resctrl/mba_test.c
index da256d2dbe5c..fa99a91c8ab7 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -171,7 +171,7 @@ static int mba_run_test(const struct resctrl_test *test,
const struct user_param
static bool mba_feature_check(const struct resctrl_test *test)
{
return test_resource_feature_check(test) &&
- validate_resctrl_feature_request("L3_MON", "mbm_local_bytes");
+ resctrl_mon_feature_exists("mbm_local_bytes");
}
struct resctrl_test mba_test = {
diff --git a/tools/testing/selftests/resctrl/mbm_test.c
b/tools/testing/selftests/resctrl/mbm_test.c
index 34879e7b71a0..9c885bc427ca 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -97,7 +97,7 @@ static int mbm_setup(const struct resctrl_test *test,
return END_OF_TESTS;
/* Set up shemata with 100% allocation on the first run. */
- if (p->num_of_runs == 0 && validate_resctrl_feature_request("MB", NULL))
+ if (p->num_of_runs == 0 && resctrl_resource_exists("MB"))
ret = write_schemata(p->ctrlgrp, "100", uparams->cpu,
test->resource);
p->num_of_runs++;
@@ -140,8 +140,8 @@ static int mbm_run_test(const struct resctrl_test *test,
const struct user_param
static bool mbm_feature_check(const struct resctrl_test *test)
{
- return validate_resctrl_feature_request("L3_MON", "mbm_total_bytes") &&
- validate_resctrl_feature_request("L3_MON", "mbm_local_bytes");
+ return resctrl_mon_feature_exists("mbm_total_bytes") &&
+ resctrl_mon_feature_exists("mbm_local_bytes");
}
struct resctrl_test mbm_test = {
diff --git a/tools/testing/selftests/resctrl/resctrl.h
b/tools/testing/selftests/resctrl/resctrl.h
index 5116ea082d03..4603b215b97e 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -136,7 +136,8 @@ int get_domain_id(const char *resource, int cpu_no, int
*domain_id);
int mount_resctrlfs(void);
int umount_resctrlfs(void);
int validate_bw_report_request(char *bw_report);
-bool validate_resctrl_feature_request(const char *resource, const char
*feature);
+bool resctrl_resource_exists(const char *resource);
+bool resctrl_mon_feature_exists(const char *feature);
bool test_resource_feature_check(const struct resctrl_test *test);
char *fgrep(FILE *inf, const char *str);
int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
b/tools/testing/selftests/resctrl/resctrlfs.c
index e0fbc46a917a..0cfec8bb23fd 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -708,20 +708,16 @@ char *fgrep(FILE *inf, const char *str)
}
/*
- * validate_resctrl_feature_request - Check if requested feature is valid.
- * @resource: Required resource (e.g., MB, L3, L2, L3_MON, etc.)
- * @feature: Required monitor feature (in mon_features file). Can only be
- * set for L3_MON. Must be NULL for all other resources.
+ * resctrl_resource_exists - Check if a
[PATCH v4 2/5] selftests/resctrl: Add helpers for the non-contiguous test
The CAT non-contiguous selftests have to read the file responsible for
reporting support of non-contiguous CBMs in kernel (resctrl). Then the
test compares if that information matches what is reported by CPUID
output.
Add a generic helper function to read an unsigned number from a file in
/sys/fs/resctrl/info//.
Signed-off-by: Maciej Wieczor-Retman
---
Changelog v4:
- Rewrite function comment.
- Redo ksft_perror() as ksft_print_msg(). (Reinette)
Changelog v3:
- Rewrite patch message.
- Add documentation and rewrote the function. (Reinette)
Changelog v2:
- Add this patch.
tools/testing/selftests/resctrl/resctrl.h | 1 +
tools/testing/selftests/resctrl/resctrlfs.c | 36 +
2 files changed, 37 insertions(+)
diff --git a/tools/testing/selftests/resctrl/resctrl.h
b/tools/testing/selftests/resctrl/resctrl.h
index a1462029998e..5116ea082d03 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val,
unsigned int *start);
int get_full_cbm(const char *cache_type, unsigned long *mask);
int get_mask_no_shareable(const char *cache_type, unsigned long *mask);
int get_cache_size(int cpu_no, const char *cache_type, unsigned long
*cache_size);
+int resource_info_unsigned_get(const char *resource, const char *filename,
unsigned int *val);
void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
int signal_handler_register(void);
void signal_handler_unregister(void);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
b/tools/testing/selftests/resctrl/resctrlfs.c
index 5750662cce57..e0fbc46a917a 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned
long *mask)
return 0;
}
+/**
+ * resource_info_unsigned_get - Read an unsigned value from
+ * /sys/fs/resctrl/info/RESOURCE/FILENAME
+ * @resource: Resource name that matches directory name in
+ * /sys/fs/resctrl/info
+ * @filename: File in /sys/fs/resctrl/info/@resource
+ * @val: Contains read value on success.
+ *
+ * Return: = 0 on success, < 0 on failure. On success the read
+ * value is saved into the @val.
+ */
+int resource_info_unsigned_get(const char *resource, const char *filename,
+ unsigned int *val)
+{
+ char file_path[PATH_MAX];
+ FILE *fp;
+
+ snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource,
+filename);
+
+ fp = fopen(file_path, "r");
+ if (!fp) {
+ ksft_print_msg("Error in opening %s\n: %m\n", file_path);
+ return -1;
+ }
+
+ if (fscanf(fp, "%u", val) <= 0) {
+ ksft_print_msg("Could not get contents of %s\n: %m\n",
file_path);
+ fclose(fp);
+ return -1;
+ }
+
+ fclose(fp);
+ return 0;
+}
+
/*
* create_bit_mask- Create bit mask from start, len pair
* @start: LSB of the mask
--
2.43.0
[PATCH v4 1/5] selftests/resctrl: Add test groups and name L3 CAT test L3_CAT
From: Ilpo Järvinen
To select test to run -t parameter can be used. However, -t cat
currently maps to L3 CAT test which will be confusing after more CAT
related tests will be added.
Allow selecting tests as groups and call L3 CAT test "L3_CAT", "CAT"
group will enable all CAT related tests.
Signed-off-by: Ilpo Järvinen
Signed-off-by: Maciej Wieczor-Retman
---
Changelog v3:
- Expand group description in struct comment (Reinette).
Changelog v2:
- Move this patch from Ilpo's series here (Ilpo).
tools/testing/selftests/resctrl/cat_test.c | 3 ++-
tools/testing/selftests/resctrl/resctrl.h | 3 +++
tools/testing/selftests/resctrl/resctrl_tests.c | 16 +++-
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c
b/tools/testing/selftests/resctrl/cat_test.c
index 24af8310288a..39fc9303b8e8 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -295,7 +295,8 @@ static int cat_run_test(const struct resctrl_test *test,
const struct user_param
}
struct resctrl_test l3_cat_test = {
- .name = "CAT",
+ .name = "L3_CAT",
+ .group = "CAT",
.resource = "L3",
.feature_check = test_resource_feature_check,
.run_test = cat_run_test,
diff --git a/tools/testing/selftests/resctrl/resctrl.h
b/tools/testing/selftests/resctrl/resctrl.h
index c52eaf46f24d..a1462029998e 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -65,6 +65,8 @@ struct user_params {
/*
* resctrl_test: resctrl test definition
* @name: Test name
+ * @group: Test group - a common name for tests that share some
characteristic
+ * (e.g., L3 CAT test belongs to the CAT group). Can be
NULL
* @resource: Resource to test (e.g., MB, L3, L2, etc.)
* @vendor_specific: Bitmask for vendor-specific tests (can be 0 for
universal tests)
* @disabled: Test is disabled
@@ -73,6 +75,7 @@ struct user_params {
*/
struct resctrl_test {
const char *name;
+ const char *group;
const char *resource;
unsigned intvendor_specific;
booldisabled;
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c
b/tools/testing/selftests/resctrl/resctrl_tests.c
index 75fc49ba3efb..3044179ee6e9 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -65,11 +65,15 @@ static void cmd_help(void)
printf("usage: resctrl_tests [-h] [-t test list] [-n no_of_bits] [-b
benchmark_cmd [option]...]\n");
printf("\t-b benchmark_cmd [option]...: run specified benchmark for
MBM, MBA and CMT\n");
printf("\t default benchmark is builtin fill_buf\n");
- printf("\t-t test list: run tests specified in the test list, ");
+ printf("\t-t test list: run tests/groups specified by the list, ");
printf("e.g. -t mbm,mba,cmt,cat\n");
- printf("\t\tSupported tests:\n");
- for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++)
- printf("\t\t\t%s\n", resctrl_tests[i]->name);
+ printf("\t\tSupported tests (group):\n");
+ for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) {
+ if (resctrl_tests[i]->group)
+ printf("\t\t\t%s (%s)\n", resctrl_tests[i]->name,
resctrl_tests[i]->group);
+ else
+ printf("\t\t\t%s\n", resctrl_tests[i]->name);
+ }
printf("\t-n no_of_bits: run cache tests using specified no of bits in
cache bit mask\n");
printf("\t-p cpu_no: specify CPU number to run the test. 1 is
default\n");
printf("\t-h: help\n");
@@ -199,7 +203,9 @@ int main(int argc, char **argv)
bool found = false;
for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++)
{
- if (!strcasecmp(token,
resctrl_tests[i]->name)) {
+ if (!strcasecmp(token,
resctrl_tests[i]->name) ||
+ (resctrl_tests[i]->group &&
+!strcasecmp(token,
resctrl_tests[i]->group))) {
if (resctrl_tests[i]->disabled)
tests++;
resctrl_tests[i]->disabled =
false;
--
2.43.0
[PATCH v4 0/5] selftests/resctrl: Add non-contiguous CBMs in Intel CAT selftest
Non-contiguous CBM support for Intel CAT has been merged into the kernel
with Commit 0e3cd31f6e90 ("x86/resctrl: Enable non-contiguous CBMs in
Intel CAT") but there is no selftest that would validate if this feature
works correctly.
The selftest needs to verify if writing non-contiguous CBMs to the
schemata file behaves as expected in comparison to the information about
non-contiguous CBMs support.
The patch series is based on a rework of resctrl selftests that's
currently in review [1]. The patch also implements a similar
functionality presented in the bash script included in the cover letter
of the original non-contiguous CBMs in Intel CAT series [3].
Changelog v4:
- Changes to error failure return values in non-contiguous test.
- Some minor text refactoring without functional changes.
Changelog v3:
- Rebase onto v4 of Ilpo's series [1].
- Split old patch 3/4 into two parts. One doing refactoring and one
adding a new function.
- Some changes to all the patches after Reinette's review.
Changelog v2:
- Rebase onto v4 of Ilpo's series [2].
- Add two patches that prepare helpers for the new test.
- Move Ilpo's patch that adds test grouping to this series.
- Apply Ilpo's suggestion to the patch that adds a new test.
[1]
https://lore.kernel.org/all/20231215150515.36983-1-ilpo.jarvi...@linux.intel.com/
[2]
https://lore.kernel.org/all/20231211121826.14392-1-ilpo.jarvi...@linux.intel.com/
[3]
https://lore.kernel.org/all/cover.1696934091.git.maciej.wieczor-ret...@intel.com/
Older versions of this series:
[v1]
https://lore.kernel.org/all/20231109112847.432687-1-maciej.wieczor-ret...@intel.com/
[v2]
https://lore.kernel.org/all/cover.1702392177.git.maciej.wieczor-ret...@intel.com/
Ilpo Järvinen (1):
selftests/resctrl: Add test groups and name L3 CAT test L3_CAT
Maciej Wieczor-Retman (4):
selftests/resctrl: Add helpers for the non-contiguous test
selftests/resctrl: Split validate_resctrl_feature_request()
selftests/resctrl: Add resource_info_file_exists()
selftests/resctrl: Add non-contiguous CBMs CAT test
tools/testing/selftests/resctrl/cat_test.c| 84 -
tools/testing/selftests/resctrl/cmt_test.c| 2 +-
tools/testing/selftests/resctrl/mba_test.c| 2 +-
tools/testing/selftests/resctrl/mbm_test.c| 6 +-
tools/testing/selftests/resctrl/resctrl.h | 10 +-
.../testing/selftests/resctrl/resctrl_tests.c | 18 +++-
tools/testing/selftests/resctrl/resctrlfs.c | 94 ---
7 files changed, 192 insertions(+), 24 deletions(-)
--
2.43.0
Re: [PATCH v3] KVM: selftests: Fix the dirty_log_test semaphore imbalance
On Mon, Feb 05, 2024 at 06:05:02PM +0800, Peter Xu wrote:
> Shaoqin, Sean,
>
> Apologies for a late comment. I'm trying to remember what I wrote..
>
> On Fri, Feb 02, 2024 at 01:43:32AM -0500, Shaoqin Huang wrote:
> > Why sem_vcpu_cont and sem_vcpu_stop can be non-zero value? It's because
> > the dirty_ring_before_vcpu_join() execute the sem_post(_vcpu_cont)
> > at the end of each dirty-ring test. It can cause two cases:
>
> As a possible alternative, would it work if we simply reset all the sems
> for each run? Then we don't care about the leftovers. E.g. sem_destroy()
> at the end of run_test(), then always init to 0 at entry.
One more thing when I was reading the code again: I had a feeling that I
missed one call to vcpu_handle_sync_stop() for the dirty ring case:
==
@@ -395,8 +395,7 @@ static void dirty_ring_after_vcpu_run(struct kvm_vcpu
*vcpu, int ret, int err)
/* A ucall-sync or ring-full event is allowed */
if (get_ucall(vcpu, NULL) == UCALL_SYNC) {
- /* We should allow this to continue */
- ;
+ vcpu_handle_sync_stop();
} else if (run->exit_reason == KVM_EXIT_DIRTY_RING_FULL ||
(ret == -1 && err == EINTR)) {
/* Update the flag first before pause */
==
Otherwise it'll be meaningless for run_test() to set
vcpu_sync_stop_requested for the ring test, if the ring test never reads
it..
Without about change, the test will still work (and I assume that's why
nobody noticed including myself..), but IIUC the vcpu can stop later,
e.g. until the ring fulls, or there's some leftover SIGUSR1 around.
With this change, the vcpu can stop earlier, as soon as the main thread
requested a stop, which should be what the code wanted to do.
Shaoqin, feel free to have a look there too if you're working on the test.
Thanks,
--
Peter Xu
Re: [PATCH v3] KVM: selftests: Fix the dirty_log_test semaphore imbalance
Shaoqin, Sean, Apologies for a late comment. I'm trying to remember what I wrote.. On Fri, Feb 02, 2024 at 01:43:32AM -0500, Shaoqin Huang wrote: > Why sem_vcpu_cont and sem_vcpu_stop can be non-zero value? It's because > the dirty_ring_before_vcpu_join() execute the sem_post(_vcpu_cont) > at the end of each dirty-ring test. It can cause two cases: As a possible alternative, would it work if we simply reset all the sems for each run? Then we don't care about the leftovers. E.g. sem_destroy() at the end of run_test(), then always init to 0 at entry. -- Peter Xu
Re: [PATCH net-next v2] net: ctnetlink: support filtering by zone
> > >
> > > Hi, Felix and Pablo.
> > >
> > > I was looking through the code and the following part is bothering me:
> > >
> > > diff --git a/net/netfilter/nf_conntrack_netlink.c
> > > b/net/netfilter/nf_conntrack_netlink.c
> > > index fb0ae15e96df..4e9133f61251 100644
> > > --- a/net/netfilter/nf_conntrack_netlink.c
> > > +++ b/net/netfilter/nf_conntrack_netlink.c
> > > @@ -1148,6 +1149,10 @@ static int ctnetlink_filter_match(struct nf_conn
> > > *ct, void *data)
> > > if (filter->family && nf_ct_l3num(ct) != filter->family)
> > > goto ignore_entry;
> > >
> > > + if (filter->zone.id != NF_CT_DEFAULT_ZONE_ID &&
> > > + !nf_ct_zone_equal_any(ct, >zone))
> > > + goto ignore_entry;
> > > +
> > > if (filter->orig_flags) {
> > > tuple = nf_ct_tuple(ct, IP_CT_DIR_ORIGINAL);
> > > if (!ctnetlink_filter_match_tuple(>orig, tuple,
> > >
> > > If I'm reading that right, the default zone is always flushed, even if the
> > > user requested to flush a different zone. I.e. the entry is never ignored
> > > for a default zone. Is that correct or am I reading that wrong?
> > >
> > > If my observation is correct, then I don't think this functionality can
> > > actually be used by applications as it does something unexpected.
> >
> > This needs a fix, the NF_CT_DEFAULT_ZONE_ID is used as a marker to
> > indicate if the filtering by zone needs to happen or not.
> >
> > I'd suggest to add a boolean flag that specifies that zone filtering
> > is set on.
Hi everyone,
i build a patch along with tests for the mentioned issue. However the issue
was rather that the filter would be skipped if we wanted to flush zone 0,
which caused all zones to be flushed.
The fix is available here:
https://lore.kernel.org/netdev/zccxn9hdsb8du...@felix.runs.onstackit.cloud/T/#u
[PATCH net] net: ctnetlink: fix filtering for zone 0
previously filtering for the default zone would actually skip the zone
filter and flush all zones.
Fixes: eff3c558bb7e ("netfilter: ctnetlink: support filtering by zone")
Reported-by: Ilya Maximets
Closes:
https://lore.kernel.org/netdev/2032238f-31ac-4106-8f22-522e76df5...@ovn.org/
Signed-off-by: Felix Huettner
---
net/netfilter/nf_conntrack_netlink.c | 12 --
.../netfilter/conntrack_dump_flush.c | 43 ++-
2 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c
b/net/netfilter/nf_conntrack_netlink.c
index 0c22a02c2035..3b846cbdc050 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -876,6 +876,7 @@ struct ctnetlink_filter_u32 {
struct ctnetlink_filter {
u8 family;
+ bool zone_filter;
u_int32_t orig_flags;
u_int32_t reply_flags;
@@ -992,9 +993,12 @@ ctnetlink_alloc_filter(const struct nlattr * const cda[],
u8 family)
if (err)
goto err_filter;
- err = ctnetlink_parse_zone(cda[CTA_ZONE], >zone);
- if (err < 0)
- goto err_filter;
+ if (cda[CTA_ZONE]) {
+ err = ctnetlink_parse_zone(cda[CTA_ZONE], >zone);
+ if (err < 0)
+ goto err_filter;
+ filter->zone_filter = true;
+ }
if (!cda[CTA_FILTER])
return filter;
@@ -1148,7 +1152,7 @@ static int ctnetlink_filter_match(struct nf_conn *ct,
void *data)
if (filter->family && nf_ct_l3num(ct) != filter->family)
goto ignore_entry;
- if (filter->zone.id != NF_CT_DEFAULT_ZONE_ID &&
+ if (filter->zone_filter &&
!nf_ct_zone_equal_any(ct, >zone))
goto ignore_entry;
diff --git a/tools/testing/selftests/netfilter/conntrack_dump_flush.c
b/tools/testing/selftests/netfilter/conntrack_dump_flush.c
index f18c6db13bbf..b11ea8ee6719 100644
--- a/tools/testing/selftests/netfilter/conntrack_dump_flush.c
+++ b/tools/testing/selftests/netfilter/conntrack_dump_flush.c
@@ -13,7 +13,7 @@
#include "../kselftest_harness.h"
#define TEST_ZONE_ID 123
-#define CTA_FILTER_F_CTA_TUPLE_ZONE (1 << 2)
+#define NF_CT_DEFAULT_ZONE_ID 0
static int reply_counter;
@@ -336,6 +336,9 @@ FIXTURE_SETUP(conntrack_dump_flush)
ret = conntrack_data_generate_v4(self->sock, 0xf4f4f4f4, 0xf5f5f5f5,
TEST_ZONE_ID + 2);
EXPECT_EQ(ret, 0);
+ ret = conntrack_data_generate_v4(self->sock, 0xf6f6f6f6, 0xf7f7f7f7,
+NF_CT_DEFAULT_ZONE_ID);
+ EXPECT_EQ(ret, 0);
src = (struct in6_addr) {{
.__u6_addr32 = {
@@ -395,6 +398,26 @@ FIXTURE_SETUP(conntrack_dump_flush)
TEST_ZONE_ID + 2);
EXPECT_EQ(ret, 0);
+ src = (struct in6_addr) {{
+ .__u6_addr32 = {
+ 0xb80d0120,
+ 0x,
+ 0x,
+ 0x0700
+ }
+ }};
+ dst = (struct in6_addr) {{
+ .__u6_addr32 = {
+ 0xb80d0120,
+ 0x,
+ 0x,
+ 0x0800
+ }
+ }};
+ ret = conntrack_data_generate_v6(self->sock, src, dst,
+NF_CT_DEFAULT_ZONE_ID);
+ EXPECT_EQ(ret, 0);
+
ret = conntracK_count_zone(self->sock, TEST_ZONE_ID);
EXPECT_GE(ret, 2);
if (ret > 2)
@@ -425,6 +448,24 @@ TEST_F(conntrack_dump_flush, test_flush_by_zone)
EXPECT_EQ(ret, 2);
ret = conntracK_count_zone(self->sock, TEST_ZONE_ID + 2);
EXPECT_EQ(ret, 2);
+ ret = conntracK_count_zone(self->sock, NF_CT_DEFAULT_ZONE_ID);
+ EXPECT_EQ(ret, 2);
+}
+
+TEST_F(conntrack_dump_flush, test_flush_by_zone_default)
+{
+ int ret;
+
+ ret = conntrack_flush_zone(self->sock, NF_CT_DEFAULT_ZONE_ID);
+ EXPECT_EQ(ret, 0);
+ ret = conntracK_count_zone(self->sock, TEST_ZONE_ID);
+ EXPECT_EQ(ret, 2);
+ ret = conntracK_count_zone(self->sock, TEST_ZONE_ID + 1);
+ EXPECT_EQ(ret, 2);
+ ret = conntracK_count_zone(self->sock, TEST_ZONE_ID + 2);
+ EXPECT_EQ(ret, 2);
+ ret = conntracK_count_zone(self->sock, NF_CT_DEFAULT_ZONE_ID);
+ EXPECT_EQ(ret, 0);
}
TEST_HARNESS_MAIN
base-commit: eef00a82c568944f113f2de738156ac591bbd5cd
--
2.43.0
Re: [PATCH v8 13/38] KVM: arm64: Manage GCS registers for guests
On Sat, 03 Feb 2024 12:25:39 +,
Mark Brown wrote:
>
> GCS introduces a number of system registers for EL1 and EL0, on systems
and EL2.
> with GCS we need to context switch them and expose them to VMMs to allow
> guests to use GCS, as well as describe their fine grained traps to
> nested virtualisation. Traps are already disabled.
The latter is not true with NV, since the guest is in control of the
FGT registers.
>
> Signed-off-by: Mark Brown
> ---
> arch/arm64/include/asm/kvm_host.h | 12
> arch/arm64/kvm/emulate-nested.c| 4
> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 17 +
> arch/arm64/kvm/sys_regs.c | 22 ++
> 4 files changed, 55 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h
> b/arch/arm64/include/asm/kvm_host.h
> index 21c57b812569..6c7ea7f9cd92 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -388,6 +388,12 @@ enum vcpu_sysreg {
> GCR_EL1,/* Tag Control Register */
> TFSRE0_EL1, /* Tag Fault Status Register (EL0) */
>
> + /* Guarded Control Stack registers */
> + GCSCRE0_EL1,/* Guarded Control Stack Control (EL0) */
> + GCSCR_EL1, /* Guarded Control Stack Control (EL1) */
This is subjected to VNCR (0x8D0).
> + GCSPR_EL0, /* Guarded Control Stack Pointer (EL0) */
> + GCSPR_EL1, /* Guarded Control Stack Pointer (EL1) */
So is this one (0x8C0). And how about the *_EL2 versions?
> +
> /* 32bit specific registers. */
> DACR32_EL2, /* Domain Access Control Register */
> IFSR32_EL2, /* Instruction Fault Status Register */
> @@ -1221,6 +1227,12 @@ static inline bool __vcpu_has_feature(const struct
> kvm_arch *ka, int feature)
>
> #define vcpu_has_feature(v, f) __vcpu_has_feature(&(v)->kvm->arch, (f))
>
> +static inline bool has_gcs(void)
> +{
> + return IS_ENABLED(CONFIG_ARM64_GCS) &&
> + cpus_have_final_cap(ARM64_HAS_GCS);
> +}
> +
> int kvm_trng_call(struct kvm_vcpu *vcpu);
> #ifdef CONFIG_KVM
> extern phys_addr_t hyp_mem_base;
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 431fd429932d..24eb7eccbae4 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -1098,8 +1098,12 @@ static const struct encoding_to_trap_config
> encoding_to_fgt[] __initconst = {
> SR_FGT(SYS_ESR_EL1, HFGxTR, ESR_EL1, 1),
> SR_FGT(SYS_DCZID_EL0, HFGxTR, DCZID_EL0, 1),
> SR_FGT(SYS_CTR_EL0, HFGxTR, CTR_EL0, 1),
> + SR_FGT(SYS_GCSPR_EL0, HFGxTR, nGCS_EL0, 1),
> SR_FGT(SYS_CSSELR_EL1, HFGxTR, CSSELR_EL1, 1),
> SR_FGT(SYS_CPACR_EL1, HFGxTR, CPACR_EL1, 1),
> + SR_FGT(SYS_GCSCR_EL1, HFGxTR, nGCS_EL1, 1),
> + SR_FGT(SYS_GCSPR_EL1, HFGxTR, nGCS_EL1, 1),
> + SR_FGT(SYS_GCSCRE0_EL1, HFGxTR, nGCS_EL0, 1),
This is clearly wrong on all 4 counts (the n prefix gives it away...).
> SR_FGT(SYS_CONTEXTIDR_EL1, HFGxTR, CONTEXTIDR_EL1, 1),
> SR_FGT(SYS_CLIDR_EL1, HFGxTR, CLIDR_EL1, 1),
> SR_FGT(SYS_CCSIDR_EL1, HFGxTR, CCSIDR_EL1, 1),
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index bb6b571ec627..ec34d4a90717 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -25,6 +25,8 @@ 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 (has_gcs())
> + ctxt_sys_reg(ctxt, GCSPR_EL0) = read_sysreg_s(SYS_GCSPR_EL0);
We have had this discussion in the past. This must be based on the
VM's configuration. Guarding the check with the host capability is a
valuable optimisation, but that's nowhere near enough. See the series
that I have posted on this very subject (you're on Cc), but you are
welcome to invent your own mechanism in the meantime.
> }
>
> static inline bool ctxt_has_mte(struct kvm_cpu_context *ctxt)
> @@ -62,6 +64,12 @@ static inline void __sysreg_save_el1_state(struct
> kvm_cpu_context *ctxt)
> ctxt_sys_reg(ctxt, PAR_EL1) = read_sysreg_par();
> ctxt_sys_reg(ctxt, TPIDR_EL1) = read_sysreg(tpidr_el1);
>
> + if (has_gcs()) {
> + ctxt_sys_reg(ctxt, GCSPR_EL1) = read_sysreg_el1(SYS_GCSPR);
> + ctxt_sys_reg(ctxt, GCSCR_EL1) = read_sysreg_el1(SYS_GCSCR);
> + ctxt_sys_reg(ctxt, GCSCRE0_EL1) =
> read_sysreg_s(SYS_GCSCRE0_EL1);
> + }
> +
Same thing.
> if (ctxt_has_mte(ctxt)) {
> ctxt_sys_reg(ctxt, TFSR_EL1) = read_sysreg_el1(SYS_TFSR);
> ctxt_sys_reg(ctxt, TFSRE0_EL1) =
