[PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()
Currently, VA exhaustion is being checked by passing a hint to mmap() and
expecting it to fail. This patch makes a stricter test by successful write()
calls from /proc/self/maps to a dump file, confirming that a free chunk is
indeed not available.
Signed-off-by: Dev Jain
---
Merge dependency:
https://lore.kernel.org/all/20240314122250.68534-1-dev.j...@arm.com/
.../selftests/mm/virtual_address_range.c | 69 +++
1 file changed, 69 insertions(+)
diff --git a/tools/testing/selftests/mm/virtual_address_range.c
b/tools/testing/selftests/mm/virtual_address_range.c
index 7bcf8d48256a..31063613dfd9 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -12,6 +12,8 @@
#include
#include
#include
+#include
+
#include "../kselftest.h"
/*
@@ -93,6 +95,69 @@ static int validate_lower_address_hint(void)
return 1;
}
+static int validate_complete_va_space(void)
+{
+ unsigned long start_addr, end_addr, prev_end_addr;
+ char line[400];
+ char prot[6];
+ FILE *file;
+ int fd;
+
+ fd = open("va_dump", O_CREAT | O_WRONLY, 0600);
+ unlink("va_dump");
+ if (fd < 0) {
+ ksft_test_result_skip("cannot create or open dump file\n");
+ ksft_finished();
+ }
+
+ file = fopen("/proc/self/maps", "r");
+ if (file == NULL)
+ ksft_exit_fail_msg("cannot open /proc/self/maps\n");
+
+ prev_end_addr = 0;
+ while (fgets(line, sizeof(line), file)) {
+ unsigned long hop;
+ int ret;
+
+ ret = sscanf(line, "%lx-%lx %s[rwxp-]",
+&start_addr, &end_addr, prot);
+ if (ret != 3)
+ ksft_exit_fail_msg("sscanf failed, cannot parse\n");
+
+ /* end of userspace mappings; ignore vsyscall mapping */
+ if (start_addr & (1UL << 63))
+ return 0;
+
+ /* /proc/self/maps must have gaps less than 1GB only */
+ if (start_addr - prev_end_addr >= SZ_1GB)
+ return 1;
+
+ prev_end_addr = end_addr;
+
+ if (prot[0] != 'r')
+ continue;
+
+ /*
+* Confirm whether MAP_CHUNK_SIZE chunk can be found or not.
+* If write succeeds, no need to check MAP_CHUNK_SIZE - 1
+* addresses after that. If the address was not held by this
+* process, write would fail with errno set to EFAULT.
+* Anyways, if write returns anything apart from 1, exit the
+* program since that would mean a bug in /proc/self/maps.
+*/
+ hop = 0;
+ while (start_addr + hop < end_addr) {
+ if (write(fd, (void *)(start_addr + hop), 1) != 1)
+ return 1;
+ else
+ lseek(fd, 0, SEEK_SET);
+
+ hop += MAP_CHUNK_SIZE;
+ }
+ }
+ return 0;
+}
+
int main(int argc, char *argv[])
{
char *ptr[NR_CHUNKS_LOW];
@@ -135,6 +200,10 @@ int main(int argc, char *argv[])
validate_addr(hptr[i], 1);
}
hchunks = i;
+ if (validate_complete_va_space()) {
+ ksft_test_result_fail("BUG in mmap() or /proc/self/maps\n");
+ ksft_finished();
+ }
for (i = 0; i < lchunks; i++)
munmap(ptr[i], MAP_CHUNK_SIZE);
--
2.34.1
Re: [PATCH 1/8] iommu: Introduce a replace API for device pasid
On 2024/3/21 14:16, Yi Liu wrote:
On 2024/3/20 20:38, Jason Gunthorpe wrote:
On Tue, Mar 19, 2024 at 03:29:39PM +0800, Yi Liu wrote:
On 2024/3/19 00:52, Jason Gunthorpe wrote:
On Wed, Mar 13, 2024 at 04:11:41PM +0800, Yi Liu wrote:
yes. how about your opinion? @Jason. I noticed the set_dev_pasid callback
and pasid_array update is under the group->lock, so update it should be
fine to adjust the order to update pasid_array after set_dev_pasid
returns.
Yes, it makes some sense
But, also I would like it very much if we just have the core pass in
the actual old domain as a an addition function argument.
ok, this works too. For normal attach, just pass in a NULL old domain.
I think we have some small mistakes in multi-device group error
unwinding for remove because the global xarray can't isn't actually
going to be correct in all scenarios.
do you mean the __iommu_remove_group_pasid() call in the below function?
Currently, it is called when __iommu_set_group_pasid() failed. However,
__iommu_set_group_pasid() may need to do remove itself when error happens,
so the helper can be more self-contained. Or you mean something else?
Yes..
int iommu_attach_device_pasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid)
{
/* Caller must be a probed driver on dev */
struct iommu_group *group = dev->iommu_group;
void *curr;
int ret;
if (!domain->ops->set_dev_pasid)
return -EOPNOTSUPP;
if (!group)
return -ENODEV;
if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain->owner)
return -EINVAL;
mutex_lock(&group->mutex);
curr = xa_cmpxchg(&group->pasid_array, pasid, NULL, domain,
GFP_KERNEL);
if (curr) {
ret = xa_err(curr) ? : -EBUSY;
goto out_unlock;
}
ret = __iommu_set_group_pasid(domain, group, pasid);
So here we have the xa set to the new domain
aha, yes. The underlying driver won't be able to get the correct domain
in the remove_dev_pasid callback.
if (ret) {
__iommu_remove_group_pasid(group, pasid);
And here we still have it set to the new domain even though some of
the devices within the group failed to attach. The logic needs to be
more like the main domain attach path where iterate and then undo only
what we did
yes, the correct way is to undo what have been done before the fail
device. However, I somehow remember that pasid capability is only
available when the group is singleton. So iterate all devices of the
devices just means one device in fact. If this is true, then the
current code is fine although a bit confusing.
And the whole thing is easier to reason about if an input argument
specifies the current attached domain instead of having the driver
read it from the xarray.
yep, will correct it as a fix patch.
Hi Jason,
It appears there are two solutions here.
First, only undo the devices that have set_dev_pasid successfully in
the __iommu_set_group_pasid(), so the problematic
__iommu_remove_group_pasid() call at line 3378 [1] would go away.
This also makes the helper more self-contained. Draft patch in [2]
Second, pass in the domain to remove_dev_pasid(). Draft patch in [3]
Either of the above two should be able to solve the mistake you mentioned.
BTW. They are orthogonal, so it's also possible to apply both of them.
Which one is your preference then?
[1]
https://github.com/torvalds/linux/blob/b3603fcb79b1036acae10602bffc4855a4b9af80/drivers/iommu/iommu.c#L3378
[2]
https://github.com/yiliu1765/iommufd/commit/bb83270767ab460978a3452750f5e032b43e59bf
[3]
https://github.com/yiliu1765/iommufd/commit/bd270b7b6619b8ba35dfaf9870df8728e370163f
Regards,
Yi Liu
Re: [PATCH 1/8] iommu: Introduce a replace API for device pasid
On Thu, Mar 21, 2024 at 07:26:41PM +0800, Yi Liu wrote: > > yes, the correct way is to undo what have been done before the fail > > device. However, I somehow remember that pasid capability is only > > available when the group is singleton. So iterate all devices of the > > devices just means one device in fact. If this is true, then the > > current code is fine although a bit confusing. Platform devicse don't have that limitation.. It is PCI only. > > > And the whole thing is easier to reason about if an input argument > > > specifies the current attached domain instead of having the driver > > > read it from the xarray. > > > > yep, will correct it as a fix patch. > > Hi Jason, > > It appears there are two solutions here. > > First, only undo the devices that have set_dev_pasid successfully in > the __iommu_set_group_pasid(), so the problematic > __iommu_remove_group_pasid() call at line 3378 [1] would go away. > This also makes the helper more self-contained. Draft patch in [2] > > Second, pass in the domain to remove_dev_pasid(). Draft patch in [3] > > Either of the above two should be able to solve the mistake you mentioned. > BTW. They are orthogonal, so it's also possible to apply both of them. > Which one is your preference then? I would do both because I also think it is not nice that the drivers always have to have the boiler plate to read the xarray in their remove.. Jason
Re: [PATCH 1/8] iommu: Introduce a replace API for device pasid
On 2024/3/21 20:20, Jason Gunthorpe wrote: On Thu, Mar 21, 2024 at 07:26:41PM +0800, Yi Liu wrote: yes, the correct way is to undo what have been done before the fail device. However, I somehow remember that pasid capability is only available when the group is singleton. So iterate all devices of the devices just means one device in fact. If this is true, then the current code is fine although a bit confusing. Platform devicse don't have that limitation.. It is PCI only. ok. And the whole thing is easier to reason about if an input argument specifies the current attached domain instead of having the driver read it from the xarray. yep, will correct it as a fix patch. Hi Jason, It appears there are two solutions here. First, only undo the devices that have set_dev_pasid successfully in the __iommu_set_group_pasid(), so the problematic __iommu_remove_group_pasid() call at line 3378 [1] would go away. This also makes the helper more self-contained. Draft patch in [2] Second, pass in the domain to remove_dev_pasid(). Draft patch in [3] Either of the above two should be able to solve the mistake you mentioned. BTW. They are orthogonal, so it's also possible to apply both of them. Which one is your preference then? I would do both because I also think it is not nice that the drivers always have to have the boiler plate to read the xarray in their remove.. sure. I'll send the two patches as Fix series. -- Regards, Yi Liu
Re: [PATCH] selftests: livepatch: Test atomic replace against multiple modules
On 3/12/24 08:12, Marcos Paulo de Souza wrote: > This new test checks if a livepatch with replace attribute set replaces > all previously applied livepatches. > > Signed-off-by: Marcos Paulo de Souza > --- > tools/testing/selftests/livepatch/Makefile | 3 +- > .../selftests/livepatch/test-atomic-replace.sh | 71 > ++ > 2 files changed, 73 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/livepatch/Makefile > b/tools/testing/selftests/livepatch/Makefile > index 35418a4790be..e92f61208d35 100644 > --- a/tools/testing/selftests/livepatch/Makefile > +++ b/tools/testing/selftests/livepatch/Makefile > @@ -10,7 +10,8 @@ TEST_PROGS := \ > test-state.sh \ > test-ftrace.sh \ > test-sysfs.sh \ > - test-syscall.sh > + test-syscall.sh \ > + test-atomic-replace.sh > > TEST_FILES := settings > > diff --git a/tools/testing/selftests/livepatch/test-atomic-replace.sh > b/tools/testing/selftests/livepatch/test-atomic-replace.sh > new file mode 100755 > index ..09a3dcdcb8de > --- /dev/null > +++ b/tools/testing/selftests/livepatch/test-atomic-replace.sh > @@ -0,0 +1,71 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Copyright (C) 2024 SUSE > +# Author: Marcos Paulo de Souza > + > +. $(dirname $0)/functions.sh > + > +MOD_REPLACE=test_klp_atomic_replace > + > +setup_config > + > +# - Load three livepatch modules. > +# - Load one more livepatch with replace being set, and check that only one > +# livepatch module is being listed. > + > +start_test "apply one liveptach to replace multiple livepatches" > + > +for mod in test_klp_livepatch test_klp_syscall test_klp_callbacks_demo; do > + load_lp $mod > +done > + > +nmods=$(ls /sys/kernel/livepatch | wc -l) > +if [ $nmods -ne 3 ]; then > + die "Expecting three modules listed, found $nmods" > +fi > + > +load_lp $MOD_REPLACE replace=1 > + > +nmods=$(ls /sys/kernel/livepatch | wc -l) > +if [ $nmods -ne 1 ]; then > + die "Expecting only one moduled listed, found $nmods" > +fi > + > +disable_lp $MOD_REPLACE > +unload_lp $MOD_REPLACE > + > +check_result "% insmod test_modules/test_klp_livepatch.ko > +livepatch: enabling patch 'test_klp_livepatch' > +livepatch: 'test_klp_livepatch': initializing patching transition > +livepatch: 'test_klp_livepatch': starting patching transition > +livepatch: 'test_klp_livepatch': completing patching transition > +livepatch: 'test_klp_livepatch': patching complete > +% insmod test_modules/test_klp_syscall.ko > +livepatch: enabling patch 'test_klp_syscall' > +livepatch: 'test_klp_syscall': initializing patching transition > +livepatch: 'test_klp_syscall': starting patching transition > +livepatch: 'test_klp_syscall': completing patching transition > +livepatch: 'test_klp_syscall': patching complete > +% insmod test_modules/test_klp_callbacks_demo.ko > +livepatch: enabling patch 'test_klp_callbacks_demo' > +livepatch: 'test_klp_callbacks_demo': initializing patching transition > +test_klp_callbacks_demo: pre_patch_callback: vmlinux > +livepatch: 'test_klp_callbacks_demo': starting patching transition > +livepatch: 'test_klp_callbacks_demo': completing patching transition > +test_klp_callbacks_demo: post_patch_callback: vmlinux > +livepatch: 'test_klp_callbacks_demo': patching complete > +% insmod test_modules/test_klp_atomic_replace.ko replace=1 > +livepatch: enabling patch 'test_klp_atomic_replace' > +livepatch: 'test_klp_atomic_replace': initializing patching transition > +livepatch: 'test_klp_atomic_replace': starting patching transition > +livepatch: 'test_klp_atomic_replace': completing patching transition > +livepatch: 'test_klp_atomic_replace': patching complete > +% echo 0 > /sys/kernel/livepatch/test_klp_atomic_replace/enabled > +livepatch: 'test_klp_atomic_replace': initializing unpatching transition > +livepatch: 'test_klp_atomic_replace': starting unpatching transition > +livepatch: 'test_klp_atomic_replace': completing unpatching transition > +livepatch: 'test_klp_atomic_replace': unpatching complete > +% rmmod test_klp_atomic_replace" > + > +exit 0 > Hi Marcos, I'm not against adding a specific atomic replace test, but for a quick tl/dr what is the difference between this new test and test-livepatch.sh's "atomic replace livepatch" test? If this one provides better coverage, should we follow up with removing the existing one? -- Joe
[PATCH] kunit: bail out early in __kunit_test_suites_init() if there are no suites to test
Commit c72a870926c2 added a mutex to prevent kunit tests from running
concurrently. Unfortunately that mutex gets locked during module load
regardless of whether the module actually has any kunit tests. This
causes a problem for kunit tests that might need to load other kernel
modules (e.g. gss_krb5_test loading the camellia module).
So check to see if there are actually any tests to run before locking
the kunit_run_lock mutex.
Fixes: c72a870926c2 ("kunit: add ability to run tests after boot using debugfs")
Reported-by: Nico Pache
Signed-off-by: Scott Mayhew
---
lib/kunit/test.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 1d1475578515..b8514dbb337c 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -712,6 +712,9 @@ int __kunit_test_suites_init(struct kunit_suite * const *
const suites, int num_
{
unsigned int i;
+ if (num_suites == 0)
+ return 0;
+
if (!kunit_enabled() && num_suites > 0) {
pr_info("kunit: disabled\n");
return 0;
--
2.43.0
Re: [PATCH bpf-next v4 3/6] bpf/helpers: introduce bpf_timer_set_sleepable_cb() kfunc
On Mon, Mar 18, 2024 at 11:52 PM Eduard Zingerman wrote:
>
> On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote:
>
> This patch looks good to me, please see two nitpicks below.
> Acked-by: Eduard Zingerman
Thanks!
>
> [...]
>
> > @@ -1350,6 +1358,11 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *,
> > timer, u64, nsecs, u64, fla
> > goto out;
> > }
> >
> > + if (t->is_sleepable && !(flags & BPF_F_TIMER_SLEEPABLE)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
>
> Nit:
> the BPF_F_TIMER_ABS and BPF_F_TIMER_CPU_PIN don't affect
> sleepable timers, should this check be changed to:
> '(t->is_sleepable && flags != BPF_F_TIMER_SLEEPABLE)' ?
Sounds fair enough. Scheduled this for v5
>
> [...]
>
> > @@ -12151,6 +12175,16 @@ static int check_kfunc_call(struct
> > bpf_verifier_env *env, struct bpf_insn *insn,
> > }
> > }
> >
> > + if (is_async_callback_calling_kfunc(meta.func_id)) {
> > + err = push_callback_call(env, insn, insn_idx, meta.subprogno,
> > + set_timer_callback_state);
>
> Nit: still think that this fragment would be better as:
>
> if (is_bpf_timer_set_sleepable_cb_impl_kfunc(meta.func_id)) {
> err = push_callback_call(env, insn, insn_idx, meta.subprogno,
> set_timer_callback_state);
>
> Because of the 'set_timer_callback_state' passed to push_callback_call().
Yeah, sorry I missed that part from the previous reviews.
Fixed in v5
Cheers,
Benjamin
>
> > + if (err) {
> > + verbose(env, "kfunc %s#%d failed callback
> > verification\n",
> > + func_name, meta.func_id);
> > + return err;
> > + }
> > + }
> > +
> > rcu_lock = is_kfunc_bpf_rcu_read_lock(&meta);
> > rcu_unlock = is_kfunc_bpf_rcu_read_unlock(&meta);
> >
>
Re: [PATCH bpf-next v4 6/6] selftests/bpf: add sleepable timer tests
On Tue, Mar 19, 2024 at 1:20 AM Eduard Zingerman wrote: > > On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote: > > bpf_experimental.h and ../bpf_testmod/bpf_testmod_kfunc.h are both > > including vmlinux.h, which is not compatible with including time.h > > or bpf_tcp_helpers.h. > > > > So prevent vmlinux.h to be included, and override the few missing > > types. > > > > Signed-off-by: Benjamin Tissoires > > [...] > > > @@ -6,6 +6,14 @@ > > #include > > #include "bpf_tcp_helpers.h" > > > > +#define __VMLINUX_H__ > > +#define u32 __u32 > > +#define u64 __u64 > > +#include "bpf_experimental.h" > > +struct prog_test_member1; > > +#include "../bpf_testmod/bpf_testmod_kfunc.h" > > +#undef __VMLINUX_H__ > > Tbh, this looks very ugly. heh :) > Would it be possible to create a new tests file sleepable_timer.c > and include bpf_experimental.h there, skipping time.h? Sounds like an option, yeah :) > It appears that for the new tests the only necessary definition from > time.h is CLOCK_MONOTONIC. > Yeah, that #define should be the only one missing. I'll try to come up with better tests in v5 :) Cheers, Benjamin
Re: [PATCH bpf-next v4 2/6] bpf/verifier: add bpf_timer as a kfunc capable type
On Mon, Mar 18, 2024 at 10:59 PM Eduard Zingerman wrote:
>
> On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote:
> [...]
>
> > @@ -12021,6 +12034,12 @@ static int check_kfunc_args(struct
> > bpf_verifier_env *env, struct bpf_kfunc_call_
> > if (ret)
> > return ret;
> > break;
> > + case KF_ARG_PTR_TO_TIMER:
> > + if (reg->type != PTR_TO_MAP_VALUE) {
> > + verbose(env, "arg#%d doesn't point to a map
> > value\n", i);
> > + return -EINVAL;
> > + }
> > + break;
>
> I think that pointer offset has to be checked as well,
> otherwise the following program verifies w/o error:
I initially thought it would be harder than it actually was.
Fixed (with the new test below) in v5.
Cheers,
Benjamin
>
> --- 8<
>
> #include
> #include
> #include
> #include
> #include "bpf_tcp_helpers.h"
>
> extern int bpf_timer_set_sleepable_cb_impl(struct bpf_timer *timer,
> int (callback_fn)(void *map, int *key, struct bpf_timer
> *timer), void *aux__ign) __ksym;
>
> #define bpf_timer_set_sleepable_cb(timer, cb) \
> bpf_timer_set_sleepable_cb_impl(timer, cb, NULL)
>
> struct elem {
> struct bpf_timer t;
> };
>
> struct {
> __uint(type, BPF_MAP_TYPE_ARRAY);
> __uint(max_entries, 2);
> __type(key, int);
> __type(value, struct elem);
> } array SEC(".maps");
>
> static int cb_sleepable(void *map, int *key, struct bpf_timer *timer)
> {
> return 0;
> }
>
> SEC("fentry/bpf_fentry_test5")
> int BPF_PROG2(test_sleepable, int, a)
> {
> struct bpf_timer *arr_timer;
> int array_key = 1;
>
> arr_timer = bpf_map_lookup_elem(&array, &array_key);
> if (!arr_timer)
> return 0;
> bpf_timer_init(arr_timer, &array, CLOCK_MONOTONIC);
> bpf_timer_set_sleepable_cb((void *)arr_timer + 1, // <-- note
> incorrect offset
>cb_sleepable);
> bpf_timer_start(arr_timer, 0, 0);
> return 0;
> }
>
> char _license[] SEC("license") = "GPL";
>
> >8 ---
>
Re: [PATCH bpf-next v4 4/6] bpf/helpers: mark the callback of bpf_timer_set_sleepable_cb() as sleepable
On Tue, Mar 19, 2024 at 12:54 AM Eduard Zingerman wrote:
>
> On Fri, 2024-03-15 at 15:29 +0100, Benjamin Tissoires wrote:
> [...]
>
> > @@ -5279,7 +5281,8 @@ static int map_kptr_match_type(struct
> > bpf_verifier_env *env,
> >
> > static bool in_sleepable(struct bpf_verifier_env *env)
> > {
> > - return env->prog->sleepable;
> > + return env->prog->sleepable ||
> > +(env->cur_state && env->cur_state->in_sleepable);
> > }
>
> I was curious why 'env->cur_state &&' check was needed and found that
> removing it caused an error in the following fragment:
>
> static int do_misc_fixups(struct bpf_verifier_env *env)
> {
> ...
> if (is_storage_get_function(insn->imm)) {
> if (!in_sleepable(env) ||
> env->insn_aux_data[i +
> delta].storage_get_func_atomic)
> insn_buf[0] = BPF_MOV64_IMM(BPF_REG_5,
> (__force __s32)GFP_ATOMIC);
> else
> insn_buf[0] = BPF_MOV64_IMM(BPF_REG_5,
> (__force __s32)GFP_KERNEL);
> ...
> }
> ...
> }
>
> When do_misc_fixups() is done env->cur_state is NULL.
> Current implementation would use GFP_ATOMIC allocation even for
> sleepable callbacks, where GFP_KERNEL is sufficient.
> Is this is something we want to address?
I honestly have no idea of the impact there.
AFAICT, if env->cur_state is not set, we don't even know if the
callback will be sleepable or not, so if there is a small penalty,
then it's the safest option, no?
Cheers,
Benjamin
>
> >
> > /* The non-sleepable programs and sleepable programs with explicit
> > bpf_rcu_read_lock()
>
[PATCH v1] uffd-unit-tests: Fix ARM related issue with fork after pthread_create
Following issue was observed while running the uffd-unit-tests selftest
on ARM devices. On x86_64 no issues were detected:
pthread_create followed by fork caused deadlock in certain cases
wherein fork required some work to be completed by the created thread.
Used synchronization to ensure that created thread's start function has
started before invoking fork.
Signed-off-by: Lokesh Gidra
[edliaw: Refactored to use atomic_bool]
Signed-off-by: Edward Liaw
---
tools/testing/selftests/mm/uffd-common.c | 4 +++-
tools/testing/selftests/mm/uffd-common.h | 2 ++
tools/testing/selftests/mm/uffd-unit-tests.c | 10 ++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/uffd-common.c
b/tools/testing/selftests/mm/uffd-common.c
index b0ac0ec2356d..14ed98c3a389 100644
--- a/tools/testing/selftests/mm/uffd-common.c
+++ b/tools/testing/selftests/mm/uffd-common.c
@@ -17,7 +17,7 @@ bool map_shared;
bool test_uffdio_wp = true;
unsigned long long *count_verify;
uffd_test_ops_t *uffd_test_ops;
-uffd_test_case_ops_t *uffd_test_case_ops;
+atomic_bool ready_for_fork;
static int uffd_mem_fd_create(off_t mem_size, bool hugetlb)
{
@@ -518,6 +518,8 @@ void *uffd_poll_thread(void *arg)
pollfd[1].fd = pipefd[cpu*2];
pollfd[1].events = POLLIN;
+ ready_for_fork = true;
+
for (;;) {
ret = poll(pollfd, 2, -1);
if (ret <= 0) {
diff --git a/tools/testing/selftests/mm/uffd-common.h
b/tools/testing/selftests/mm/uffd-common.h
index cb055282c89c..cc5629c3d2aa 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include "../kselftest.h"
#include "vm_util.h"
@@ -103,6 +104,7 @@ extern bool map_shared;
extern bool test_uffdio_wp;
extern unsigned long long *count_verify;
extern volatile bool test_uffdio_copy_eexist;
+extern atomic_bool ready_for_fork;
extern uffd_test_ops_t anon_uffd_test_ops;
extern uffd_test_ops_t shmem_uffd_test_ops;
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c
b/tools/testing/selftests/mm/uffd-unit-tests.c
index 2b9f8cc52639..4a48dc617c6b 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -775,6 +775,8 @@ static void uffd_sigbus_test_common(bool wp)
char c;
struct uffd_args args = { 0 };
+ ready_for_fork = false;
+
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
if (uffd_register(uffd, area_dst, nr_pages * page_size,
@@ -790,6 +792,9 @@ static void uffd_sigbus_test_common(bool wp)
if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
err("uffd_poll_thread create");
+ while (!ready_for_fork)
+ ; /* Wait for the poll_thread to start executing before forking
*/
+
pid = fork();
if (pid < 0)
err("fork");
@@ -829,6 +834,8 @@ static void uffd_events_test_common(bool wp)
char c;
struct uffd_args args = { 0 };
+ ready_for_fork = false;
+
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
if (uffd_register(uffd, area_dst, nr_pages * page_size,
true, wp, false))
@@ -838,6 +845,9 @@ static void uffd_events_test_common(bool wp)
if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
err("uffd_poll_thread create");
+ while (!ready_for_fork)
+ ; /* Wait for the poll_thread to start executing before forking
*/
+
pid = fork();
if (pid < 0)
err("fork");
--
2.44.0.396.g6e790dbe36-goog
Re: [BUG] selftests/net: test_vxlan_mdb.sh: 84 out of 642 tests [FAIL]
On 3/20/24 12:01, Ido Schimmel wrote: On Wed, Mar 20, 2024 at 01:47:36AM +0100, Mirsad Todorovac wrote: On 3/19/24 15:25, Ido Schimmel wrote: Will look into it today or later this week. Thank you for considering this. Can you please try the following patch? https://github.com/idosch/linux/commit/58f25dd8766dbe9ac50c76b44f9ba92350ebb5c6.patch Congratulations, apparently, your patch had fixed them all: # TEST: Torture test [ OK ] # # Data path: MDB torture test - IPv6 overlay / IPv6 underlay # -- # TEST: Torture test [ OK ] # # Tests passed: 642 # Tests failed: 0 ok 90 selftests: net: test_vxlan_mdb.sh Please consider adding: Tested-by: Mirsad Todorovac at your convenience. Shalom, and have a great evening! Best regards, Mirsad Todorovac
Re: [BUG] selftests/net: test_vxlan_mdb.sh: 84 out of 642 tests [FAIL]
On 3/20/24 12:01, Ido Schimmel wrote: On Wed, Mar 20, 2024 at 01:47:36AM +0100, Mirsad Todorovac wrote: On 3/19/24 15:25, Ido Schimmel wrote: Will look into it today or later this week. Thank you for considering this. Can you please try the following patch? https://github.com/idosch/linux/commit/58f25dd8766dbe9ac50c76b44f9ba92350ebb5c6.patch Congratulations, apparently, your patch had fixed them all: # TEST: Torture test [ OK ] # # Data path: MDB torture test - IPv6 overlay / IPv6 underlay # -- # TEST: Torture test [ OK ] # # Tests passed: 642 # Tests failed: 0 ok 90 selftests: net: test_vxlan_mdb.sh Please consider adding: Tested-by: Mirsad Todorovac at your convenience. Shalom, and have a great evening! Best regards, Mirsad Todorovac
[PATCH v2] uffd-unit-tests: Fix ARM related issue with fork after pthread_create
Following issue was observed while running the uffd-unit-tests selftest
on ARM devices. On x86_64 no issues were detected:
pthread_create followed by fork caused deadlock in certain cases
wherein fork required some work to be completed by the created thread.
Used synchronization to ensure that created thread's start function has
started before invoking fork.
Signed-off-by: Lokesh Gidra
[edliaw: Refactored to use atomic_bool]
Signed-off-by: Edward Liaw
---
tools/testing/selftests/mm/uffd-common.c | 3 +++
tools/testing/selftests/mm/uffd-common.h | 2 ++
tools/testing/selftests/mm/uffd-unit-tests.c | 10 ++
3 files changed, 15 insertions(+)
diff --git a/tools/testing/selftests/mm/uffd-common.c
b/tools/testing/selftests/mm/uffd-common.c
index b0ac0ec2356d..7ad6ba660c7d 100644
--- a/tools/testing/selftests/mm/uffd-common.c
+++ b/tools/testing/selftests/mm/uffd-common.c
@@ -18,6 +18,7 @@ bool test_uffdio_wp = true;
unsigned long long *count_verify;
uffd_test_ops_t *uffd_test_ops;
uffd_test_case_ops_t *uffd_test_case_ops;
+atomic_bool ready_for_fork;
static int uffd_mem_fd_create(off_t mem_size, bool hugetlb)
{
@@ -518,6 +519,8 @@ void *uffd_poll_thread(void *arg)
pollfd[1].fd = pipefd[cpu*2];
pollfd[1].events = POLLIN;
+ ready_for_fork = true;
+
for (;;) {
ret = poll(pollfd, 2, -1);
if (ret <= 0) {
diff --git a/tools/testing/selftests/mm/uffd-common.h
b/tools/testing/selftests/mm/uffd-common.h
index cb055282c89c..cc5629c3d2aa 100644
--- a/tools/testing/selftests/mm/uffd-common.h
+++ b/tools/testing/selftests/mm/uffd-common.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include "../kselftest.h"
#include "vm_util.h"
@@ -103,6 +104,7 @@ extern bool map_shared;
extern bool test_uffdio_wp;
extern unsigned long long *count_verify;
extern volatile bool test_uffdio_copy_eexist;
+extern atomic_bool ready_for_fork;
extern uffd_test_ops_t anon_uffd_test_ops;
extern uffd_test_ops_t shmem_uffd_test_ops;
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c
b/tools/testing/selftests/mm/uffd-unit-tests.c
index 2b9f8cc52639..4a48dc617c6b 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -775,6 +775,8 @@ static void uffd_sigbus_test_common(bool wp)
char c;
struct uffd_args args = { 0 };
+ ready_for_fork = false;
+
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
if (uffd_register(uffd, area_dst, nr_pages * page_size,
@@ -790,6 +792,9 @@ static void uffd_sigbus_test_common(bool wp)
if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
err("uffd_poll_thread create");
+ while (!ready_for_fork)
+ ; /* Wait for the poll_thread to start executing before forking
*/
+
pid = fork();
if (pid < 0)
err("fork");
@@ -829,6 +834,8 @@ static void uffd_events_test_common(bool wp)
char c;
struct uffd_args args = { 0 };
+ ready_for_fork = false;
+
fcntl(uffd, F_SETFL, uffd_flags | O_NONBLOCK);
if (uffd_register(uffd, area_dst, nr_pages * page_size,
true, wp, false))
@@ -838,6 +845,9 @@ static void uffd_events_test_common(bool wp)
if (pthread_create(&uffd_mon, NULL, uffd_poll_thread, &args))
err("uffd_poll_thread create");
+ while (!ready_for_fork)
+ ; /* Wait for the poll_thread to start executing before forking
*/
+
pid = fork();
if (pid < 0)
err("fork");
--
2.44.0.396.g6e790dbe36-goog
Re: [PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()
On Thu, 21 Mar 2024 16:05:22 +0530 Dev Jain wrote: > Currently, VA exhaustion is being checked by passing a hint to mmap() and > expecting it to fail. This patch makes a stricter test by successful write() > calls from /proc/self/maps to a dump file, confirming that a free chunk is > indeed not available. What's wrong with the current approach?
Re: [RFC PATCH v5 01/29] KVM: selftests: Add function to allow one-to-one GVA to GPA mappings
On 12/12/2023 12:46 PM, Sagi Shahar wrote:
> From: Ackerley Tng
>
> One-to-one GVA to GPA mappings can be used in the guest to set up boot
> sequences during which paging is enabled, hence requiring a transition
> from using physical to virtual addresses in consecutive instructions.
>
> Signed-off-by: Ackerley Tng
> Signed-off-by: Ryan Afranji
> Signed-off-by: Sagi Shahar
> ---
> .../selftests/kvm/include/kvm_util_base.h | 2 +
> tools/testing/selftests/kvm/lib/kvm_util.c| 63 ---
> 2 files changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h
> b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 1426e88ebdc7..c2e5c5f25dfc 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -564,6 +564,8 @@ vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz,
> vm_vaddr_t vaddr_min);
> vm_vaddr_t __vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t
> vaddr_min,
> enum kvm_mem_region_type type);
> vm_vaddr_t vm_vaddr_alloc_shared(struct kvm_vm *vm, size_t sz, vm_vaddr_t
> vaddr_min);
> +vm_vaddr_t vm_vaddr_alloc_1to1(struct kvm_vm *vm, size_t sz,
> +vm_vaddr_t vaddr_min, uint32_t data_memslot);
> vm_vaddr_t vm_vaddr_alloc_pages(struct kvm_vm *vm, int nr_pages);
> vm_vaddr_t __vm_vaddr_alloc_page(struct kvm_vm *vm,
>enum kvm_mem_region_type type);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
> b/tools/testing/selftests/kvm/lib/kvm_util.c
> index febc63d7a46b..4f1ae0f1eef0 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1388,17 +1388,37 @@ vm_vaddr_t vm_vaddr_unused_gap(struct kvm_vm *vm,
> size_t sz,
> return pgidx_start * vm->page_size;
> }
>
> +/*
> + * VM Virtual Address Allocate Shared/Encrypted
> + *
> + * Input Args:
> + * vm - Virtual Machine
> + * sz - Size in bytes
> + * vaddr_min - Minimum starting virtual address
> + * paddr_min - Minimum starting physical address
> + * data_memslot - memslot number to allocate in
> + * encrypt - Whether the region should be handled as encrypted
> + *
> + * Output Args: None
> + *
> + * Return:
> + * Starting guest virtual address
> + *
> + * Allocates at least sz bytes within the virtual address space of the vm
> + * given by vm. The allocated bytes are mapped to a virtual address >=
> + * the address given by vaddr_min. Note that each allocation uses a
> + * a unique set of pages, with the minimum real allocation being at least
> + * a page.
> + */
> static vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz,
> - vm_vaddr_t vaddr_min,
> - enum kvm_mem_region_type type,
> - bool encrypt)
> + vm_vaddr_t vaddr_min, vm_paddr_t paddr_min,
> + uint32_t data_memslot, bool encrypt)
> {
> uint64_t pages = (sz >> vm->page_shift) + ((sz % vm->page_size) != 0);
>
> virt_pgd_alloc(vm);
> - vm_paddr_t paddr = _vm_phy_pages_alloc(vm, pages,
> - KVM_UTIL_MIN_PFN * vm->page_size,
> - vm->memslots[type], encrypt);
> + vm_paddr_t paddr = _vm_phy_pages_alloc(vm, pages, paddr_min,
> +data_memslot, encrypt);
>
> /*
>* Find an unused range of virtual page addresses of at least
> @@ -1408,8 +1428,7 @@ static vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm,
> size_t sz,
>
> /* Map the virtual pages. */
> for (vm_vaddr_t vaddr = vaddr_start; pages > 0;
> - pages--, vaddr += vm->page_size, paddr += vm->page_size) {
> -
> + pages--, vaddr += vm->page_size, paddr += vm->page_size) {
> virt_pg_map(vm, vaddr, paddr);
>
> sparsebit_set(vm->vpages_mapped, vaddr >> vm->page_shift);
> @@ -1421,12 +1440,16 @@ static vm_vaddr_t vm_vaddr_alloc(struct kvm_vm
> *vm, size_t sz,
> vm_vaddr_t __vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t
> vaddr_min,
> enum kvm_mem_region_type type)
> {
> - return vm_vaddr_alloc(vm, sz, vaddr_min, type, vm->protected);
> + return vm_vaddr_alloc(vm, sz, vaddr_min,
> + KVM_UTIL_MIN_PFN * vm->page_size,
> + vm->memslots[type], vm->protected);
> }
>
> vm_vaddr_t vm_vaddr_alloc_shared(struct kvm_vm *vm, size_t sz, vm_vaddr_t
> vaddr_min)
> {
> - return vm_vaddr_alloc(vm, sz, vaddr_min, MEM_REGION_TEST_DATA,
> false);
> + return vm_vaddr_alloc(vm, sz, vaddr_min,
> + KVM_UTIL_MIN_PFN * vm->page_size,
> + vm->m
Re: [RFC PATCH v5 05/29] KVM: selftests: Add helper functions to create TDX VMs
On 12/12/2023 12:46 PM, Sagi Shahar wrote:
> From: Erdem Aktas
>
> TDX requires additional IOCTLs to initialize VM and vCPUs to add
> private memory and to finalize the VM memory. Also additional utility
> functions are provided to manipulate a TD, similar to those that
> manipulate a VM in the current selftest framework.
>
> A TD's initial register state cannot be manipulated directly by
> setting the VM's memory, hence boot code is provided at the TD's reset
> vector. This boot code takes boot parameters loaded in the TD's memory
> and sets up the TD for the selftest.
>
> Signed-off-by: Erdem Aktas
> Signed-off-by: Ryan Afranji
> Signed-off-by: Sagi Shahar
> Co-developed-by: Ackerley Tng
> Signed-off-by: Ackerley Tng
> ---
> tools/testing/selftests/kvm/Makefile | 2 +
> .../kvm/include/x86_64/tdx/td_boot.h | 82
> .../kvm/include/x86_64/tdx/td_boot_asm.h | 16 +
> .../kvm/include/x86_64/tdx/tdx_util.h | 16 +
> .../selftests/kvm/lib/x86_64/tdx/td_boot.S| 101
> .../selftests/kvm/lib/x86_64/tdx/tdx_util.c | 434 ++
> 6 files changed, 651 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/tdx/td_boot.h
> create mode 100644
> tools/testing/selftests/kvm/include/x86_64/tdx/td_boot_asm.h
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/tdx/tdx_util.h
> create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx/td_boot.S
> create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx/tdx_util.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile
> b/tools/testing/selftests/kvm/Makefile
> index b11ac221aba4..a35150ab855f 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -50,6 +50,8 @@ LIBKVM_x86_64 += lib/x86_64/svm.c
> LIBKVM_x86_64 += lib/x86_64/ucall.c
> LIBKVM_x86_64 += lib/x86_64/vmx.c
> LIBKVM_x86_64 += lib/x86_64/sev.c
> +LIBKVM_x86_64 += lib/x86_64/tdx/tdx_util.c
> +LIBKVM_x86_64 += lib/x86_64/tdx/td_boot.S
>
> LIBKVM_aarch64 += lib/aarch64/gic.c
> LIBKVM_aarch64 += lib/aarch64/gic_v3.c
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/td_boot.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/td_boot.h
> new file mode 100644
> index ..148057e569d6
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/td_boot.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTEST_TDX_TD_BOOT_H
> +#define SELFTEST_TDX_TD_BOOT_H
> +
> +#include
> +#include "tdx/td_boot_asm.h"
> +
> +/*
> + * Layout for boot section (not to scale)
> + *
> + * GPA
> + * ┌─┬──0x1__ (4GB)
> + * │ Boot code trampoline │
> + * ├─┼──0x0__fff0: Reset vector (16B below
> 4GB)
> + * │ Boot code │
> + * ├─┼──td_boot will be copied here, so that the
> + * │ │ jmp to td_boot is exactly at the reset
> vector
> + * │ Empty space │
> + * │ │
> + * ├─┤
> + * │ │
> + * │ │
> + * │ Boot parameters │
> + * │ │
> + * │ │
> + * └─┴──0x0__: TD_BOOT_PARAMETERS_GPA
> + */
> +#define FOUR_GIGABYTES_GPA (4ULL << 30)
> +
> +/**
> + * The exact memory layout for LGDT or LIDT instructions.
> + */
> +struct __packed td_boot_parameters_dtr {
> + uint16_t limit;
> + uint32_t base;
> +};
> +
> +/**
> + * The exact layout in memory required for a ljmp, including the selector for
> + * changing code segment.
> + */
> +struct __packed td_boot_parameters_ljmp_target {
> + uint32_t eip_gva;
> + uint16_t code64_sel;
> +};
> +
> +/**
> + * Allows each vCPU to be initialized with different eip and esp.
> + */
> +struct __packed td_per_vcpu_parameters {
> + uint32_t esp_gva;
> + struct td_boot_parameters_ljmp_target ljmp_target;
> +};
> +
> +/**
> + * Boot parameters for the TD.
> + *
> + * Unlike a regular VM, we can't ask KVM to set registers such as esp, eip,
> etc
> + * before boot, so to run selftests, these registers' values have to be
> + * initialized by the TD.
> + *
> + * This struct is loaded in TD private memory at TD_BOOT_PARAMETERS_GPA.
> + *
> + * The TD boot code will read off parameters from this struct and set up the
> + * vcpu for executing selftests.
> + */
> +struct __packed td_boot_parameters {
> + uint32_t cr0;
> + uint32_t cr3;
> + uint32_t cr4;
> + struct td_boot_parameters_dtr gdtr;
> + struct td_boot_parameters_dtr idtr;
> + struct td_per_vcpu_parameters per_vcpu[];
> +};
> +
> +extern void td_boot(void);
> +extern void reset_vector(void);
> +extern void td_boot_code_end(void);
> +
> +#define TD_BOOT_CODE_SIZE (td_boot_code_end -
[PATCH v1] selftests/mm: sigbus-wp test requires UFFD_FEATURE_WP_HUGETLBFS_SHMEM
The sigbus-wp test requires the UFFD_FEATURE_WP_HUGETLBFS_SHMEM flag for
shmem and hugetlb targets. Otherwise it is not backwards compatible
with kernels <5.19 and fails with EINVAL.
Signed-off-by: Edward Liaw
---
tools/testing/selftests/mm/uffd-unit-tests.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/mm/uffd-unit-tests.c
b/tools/testing/selftests/mm/uffd-unit-tests.c
index 4a48dc617c6b..21ec23206ab4 100644
--- a/tools/testing/selftests/mm/uffd-unit-tests.c
+++ b/tools/testing/selftests/mm/uffd-unit-tests.c
@@ -1437,7 +1437,8 @@ uffd_test_case_t uffd_tests[] = {
.uffd_fn = uffd_sigbus_wp_test,
.mem_targets = MEM_ALL,
.uffd_feature_required = UFFD_FEATURE_SIGBUS |
- UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_PAGEFAULT_FLAG_WP,
+ UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_PAGEFAULT_FLAG_WP |
+ UFFD_FEATURE_WP_HUGETLBFS_SHMEM,
},
{
.name = "events",
--
2.44.0.396.g6e790dbe36-goog
Re: [RFC PATCH v5 08/29] KVM: selftests: TDX: Add TDX lifecycle test
On 12/12/2023 12:46 PM, Sagi Shahar wrote:
> From: Erdem Aktas
>
> Adding a test to verify TDX lifecycle by creating a TD and running a
> dummy TDG.VP.VMCALL inside it.
>
> Signed-off-by: Erdem Aktas
> Signed-off-by: Ryan Afranji
> Signed-off-by: Sagi Shahar
> Co-developed-by: Ackerley Tng
> Signed-off-by: Ackerley Tng
> ---
> tools/testing/selftests/kvm/Makefile | 4 +
> .../selftests/kvm/include/x86_64/tdx/tdcall.h | 35
> .../selftests/kvm/include/x86_64/tdx/tdx.h| 12 +++
> .../kvm/include/x86_64/tdx/test_util.h| 52 +++
> .../selftests/kvm/lib/x86_64/tdx/tdcall.S | 90 +++
> .../selftests/kvm/lib/x86_64/tdx/tdx.c| 27 ++
> .../selftests/kvm/lib/x86_64/tdx/tdx_util.c | 1 +
> .../selftests/kvm/lib/x86_64/tdx/test_util.c | 34 +++
> .../selftests/kvm/x86_64/tdx_vm_tests.c | 45 ++
> 9 files changed, 300 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h
> create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx/tdcall.S
> create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> create mode 100644 tools/testing/selftests/kvm/lib/x86_64/tdx/test_util.c
> create mode 100644 tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile
> b/tools/testing/selftests/kvm/Makefile
> index a35150ab855f..80d4a50eeb9f 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -52,6 +52,9 @@ LIBKVM_x86_64 += lib/x86_64/vmx.c
> LIBKVM_x86_64 += lib/x86_64/sev.c
> LIBKVM_x86_64 += lib/x86_64/tdx/tdx_util.c
> LIBKVM_x86_64 += lib/x86_64/tdx/td_boot.S
> +LIBKVM_x86_64 += lib/x86_64/tdx/tdcall.S
> +LIBKVM_x86_64 += lib/x86_64/tdx/tdx.c
> +LIBKVM_x86_64 += lib/x86_64/tdx/test_util.c
>
> LIBKVM_aarch64 += lib/aarch64/gic.c
> LIBKVM_aarch64 += lib/aarch64/gic_v3.c
> @@ -152,6 +155,7 @@ TEST_GEN_PROGS_x86_64 += set_memory_region_test
> TEST_GEN_PROGS_x86_64 += steal_time
> TEST_GEN_PROGS_x86_64 += kvm_binary_stats_test
> TEST_GEN_PROGS_x86_64 += system_counter_offset_test
> +TEST_GEN_PROGS_x86_64 += x86_64/tdx_vm_tests
>
> # Compiled outputs used by test targets
> TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> new file mode 100644
> index ..78001bfec9c8
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Adapted from arch/x86/include/asm/shared/tdx.h */
> +
> +#ifndef SELFTESTS_TDX_TDCALL_H
> +#define SELFTESTS_TDX_TDCALL_H
> +
> +#include
> +#include
> +
> +#define TDG_VP_VMCALL_INSTRUCTION_IO_READ 0
> +#define TDG_VP_VMCALL_INSTRUCTION_IO_WRITE 1
Nit:
Probably we can define the following instead in test_util.c?
/* Port I/O direction */
#define PORT_READ 0
#define PORT_WRITE 1
Then use them in place of
TDG_VP_VMCALL_INSTRUCTION_IO_READ/TDG_VP_VMCALL_INSTRUCTION_IO_WRITE?
which are too long
> +
> +#define TDX_HCALL_HAS_OUTPUT BIT(0)
> +
> +#define TDX_HYPERCALL_STANDARD 0
> +
> +/*
> + * Used in __tdx_hypercall() to pass down and get back registers' values of
> + * the TDCALL instruction when requesting services from the VMM.
> + *
> + * This is a software only structure and not part of the TDX module/VMM ABI.
> + */
> +struct tdx_hypercall_args {
> + u64 r10;
> + u64 r11;
> + u64 r12;
> + u64 r13;
> + u64 r14;
> + u64 r15;
> +};
> +
> +/* Used to request services from the VMM */
> +u64 __tdx_hypercall(struct tdx_hypercall_args *args, unsigned long flags);
> +
> +#endif // SELFTESTS_TDX_TDCALL_H
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> new file mode 100644
> index ..a7161efe4ee2
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTEST_TDX_TDX_H
> +#define SELFTEST_TDX_TDX_H
> +
> +#include
> +
> +#define TDG_VP_VMCALL_INSTRUCTION_IO 30
Nit:
arch/x86/include/uapi/asm/vmx.h already exports the following define:
#define EXIT_REASON_IO_INSTRUCTION 30
Linux kernel example (arch/x86/coco/tdx/tdx.c):
static bool handle_in(struct pt_regs *regs, int size, int port)
{
struct tdx_module_args args = {
.r10 = TDX_HYPERCALL_STANDARD,
.r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION),
.r12 = size,
.r13 = PORT_READ,
.r14 = port,
};
So just like the kernel, here we can also use EXIT_REASON_IO_INSTRUCTION in
place of TDG_VP_VMCAL
Re: [PATCH] kunit: bail out early in __kunit_test_suites_init() if there are no suites to test
On Thu, Mar 21, 2024 at 10:32 AM Scott Mayhew wrote:
>
> Commit c72a870926c2 added a mutex to prevent kunit tests from running
> concurrently. Unfortunately that mutex gets locked during module load
> regardless of whether the module actually has any kunit tests. This
> causes a problem for kunit tests that might need to load other kernel
> modules (e.g. gss_krb5_test loading the camellia module).
>
> So check to see if there are actually any tests to run before locking
> the kunit_run_lock mutex.
>
> Fixes: c72a870926c2 ("kunit: add ability to run tests after boot using
> debugfs")
> Reported-by: Nico Pache
> Signed-off-by: Scott Mayhew
Hi!
Sorry about this bug. Thanks for the patch! We should definitely add this check.
Reviewed-by: Rae Moar
Thanks!
-Rae
> ---
> lib/kunit/test.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 1d1475578515..b8514dbb337c 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -712,6 +712,9 @@ int __kunit_test_suites_init(struct kunit_suite * const *
> const suites, int num_
> {
> unsigned int i;
>
> + if (num_suites == 0)
> + return 0;
> +
> if (!kunit_enabled() && num_suites > 0) {
> pr_info("kunit: disabled\n");
> return 0;
> --
> 2.43.0
>
Re: [PATCH v1] selftests/mm: sigbus-wp test requires UFFD_FEATURE_WP_HUGETLBFS_SHMEM
On Thu, 21 Mar 2024 23:20:21 + Edward Liaw wrote:
> The sigbus-wp test requires the UFFD_FEATURE_WP_HUGETLBFS_SHMEM flag for
> shmem and hugetlb targets. Otherwise it is not backwards compatible
> with kernels <5.19 and fails with EINVAL.
>
> ...
>
> --- a/tools/testing/selftests/mm/uffd-unit-tests.c
> +++ b/tools/testing/selftests/mm/uffd-unit-tests.c
> @@ -1437,7 +1437,8 @@ uffd_test_case_t uffd_tests[] = {
> .uffd_fn = uffd_sigbus_wp_test,
> .mem_targets = MEM_ALL,
> .uffd_feature_required = UFFD_FEATURE_SIGBUS |
> - UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_PAGEFAULT_FLAG_WP,
> + UFFD_FEATURE_EVENT_FORK | UFFD_FEATURE_PAGEFAULT_FLAG_WP |
> + UFFD_FEATURE_WP_HUGETLBFS_SHMEM,
> },
> {
> .name = "events",
Thanks, I'll add
Fixes: 73c1ea939b65 ("selftests/mm: move uffd sig/events tests into uffd unit
tests")
Cc:
Re: [RFC PATCH v5 15/29] KVM: selftests: TDX: Add TDX MSR read/write tests
On 12/12/2023 12:46 PM, Sagi Shahar wrote:
> The test verifies reads and writes for MSR registers with different access
> level.
>
> Signed-off-by: Sagi Shahar
> Signed-off-by: Ackerley Tng
> Signed-off-by: Ryan Afranji
> ---
> .../selftests/kvm/include/x86_64/tdx/tdx.h| 5 +
> .../selftests/kvm/lib/x86_64/tdx/tdx.c| 27 +++
> .../selftests/kvm/x86_64/tdx_vm_tests.c | 209 ++
> 3 files changed, 241 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> index 63788012bf94..85ba6aab79a7 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> @@ -9,11 +9,16 @@
> #define TDG_VP_VMCALL_REPORT_FATAL_ERROR 0x10003
>
> #define TDG_VP_VMCALL_INSTRUCTION_IO 30
> +#define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31
> +#define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32
Nit:
"arch/x86/include/uapi/asm/vmx.h" already defined the following defs:
#define EXIT_REASON_IO_INSTRUCTION 30
#define EXIT_REASON_MSR_READ31
#define EXIT_REASON_MSR_WRITE 32
> +
> void handle_userspace_tdg_vp_vmcall_exit(struct kvm_vcpu *vcpu);
> uint64_t tdg_vp_vmcall_instruction_io(uint64_t port, uint64_t size,
> uint64_t write, uint64_t *data);
> void tdg_vp_vmcall_report_fatal_error(uint64_t error_code, uint64_t
> data_gpa);
> uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11, uint64_t *r12,
> uint64_t *r13, uint64_t *r14);
> +uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t
> *ret_value);
> +uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
>
> #endif // SELFTEST_TDX_TDX_H
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> index e5a9e13c62e2..88ea6f2a6469 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> @@ -87,3 +87,30 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11,
> uint64_t *r12,
>
> return ret;
> }
> +
> +uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t *ret_value)
> +{
> + uint64_t ret;
> + struct tdx_hypercall_args args = {
> + .r11 = TDG_VP_VMCALL_INSTRUCTION_RDMSR,
> + .r12 = index,
> + };
> +
> + ret = __tdx_hypercall(&args, TDX_HCALL_HAS_OUTPUT);
> +
> + if (ret_value)
> + *ret_value = args.r11;
> +
> + return ret;
> +}
> +
> +uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value)
> +{
> + struct tdx_hypercall_args args = {
> + .r11 = TDG_VP_VMCALL_INSTRUCTION_WRMSR,
> + .r12 = index,
> + .r13 = value,
> + };
> +
> + return __tdx_hypercall(&args, 0);
> +}
> diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> index 699cba36e9ce..5db3701cc6d9 100644
> --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> @@ -515,6 +515,213 @@ void verify_guest_reads(void)
> printf("\t ... PASSED\n");
> }
>
> +/*
> + * Define a filter which denies all MSR access except the following:
> + * MSR_X2APIC_APIC_ICR: Allow read/write access (allowed by default)
> + * MSR_IA32_MISC_ENABLE: Allow read access
> + * MSR_IA32_POWER_CTL: Allow write access
> + */
> +#define MSR_X2APIC_APIC_ICR 0x830
> +static u64 tdx_msr_test_allow_bits = 0x;
Nit:
0x is error prone to define? the following?
static u64 tdx_msr_test_allow_bits = ~0ULL;
> +struct kvm_msr_filter tdx_msr_test_filter = {
> + .flags = KVM_MSR_FILTER_DEFAULT_DENY,
> + .ranges = {
> + {
> + .flags = KVM_MSR_FILTER_READ,
> + .nmsrs = 1,
> + .base = MSR_IA32_MISC_ENABLE,
> + .bitmap = (uint8_t *)&tdx_msr_test_allow_bits,
> + }, {
> + .flags = KVM_MSR_FILTER_WRITE,
> + .nmsrs = 1,
> + .base = MSR_IA32_POWER_CTL,
> + .bitmap = (uint8_t *)&tdx_msr_test_allow_bits,
> + },
> + },
> +};
> +
> +/*
> + * Verifies MSR read functionality.
> + */
> +void guest_msr_read(void)
> +{
> + uint64_t data;
> + uint64_t ret;
> +
> + ret = tdg_vp_vmcall_instruction_rdmsr(MSR_X2APIC_APIC_ICR, &data);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + ret = tdx_test_report_64bit_to_user_space(data);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + ret = tdg_vp_vmcall_instruction_rdmsr(MSR_IA32_MISC_ENABLE, &data);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + ret = tdx_test_report_64bit_to_user_space(data);
> + if (ret)
> + tdx_t
Re: [RFC PATCH v5 17/29] KVM: selftests: TDX: Add TDX MMIO reads test
On 12/12/2023 12:46 PM, Sagi Shahar wrote:
> The test verifies MMIO reads of various sizes from the host to the guest.
>
> Signed-off-by: Sagi Shahar
> Signed-off-by: Ackerley Tng
> Signed-off-by: Ryan Afranji
> ---
> .../selftests/kvm/include/x86_64/tdx/tdcall.h | 2 +
> .../selftests/kvm/include/x86_64/tdx/tdx.h| 3 +
> .../kvm/include/x86_64/tdx/test_util.h| 23 +
> .../selftests/kvm/lib/x86_64/tdx/tdx.c| 19
> .../selftests/kvm/x86_64/tdx_vm_tests.c | 87 +++
> 5 files changed, 134 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> index b5e94b7c48fa..95fcdbd8404e 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdcall.h
> @@ -9,6 +9,8 @@
>
> #define TDG_VP_VMCALL_INSTRUCTION_IO_READ 0
> #define TDG_VP_VMCALL_INSTRUCTION_IO_WRITE 1
> +#define TDG_VP_VMCALL_VE_REQUEST_MMIO_READ 0
> +#define TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE 1
>
> #define TDG_VP_VMCALL_SUCCESS 0x
> #define TDG_VP_VMCALL_INVALID_OPERAND 0x8000
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> index b18e39d20498..13ce60df5684 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> @@ -12,6 +12,7 @@
> #define TDG_VP_VMCALL_INSTRUCTION_IO 30
> #define TDG_VP_VMCALL_INSTRUCTION_RDMSR 31
> #define TDG_VP_VMCALL_INSTRUCTION_WRMSR 32
> +#define TDG_VP_VMCALL_VE_REQUEST_MMIO 48
>
> void handle_userspace_tdg_vp_vmcall_exit(struct kvm_vcpu *vcpu);
> uint64_t tdg_vp_vmcall_instruction_io(uint64_t port, uint64_t size,
> @@ -22,5 +23,7 @@ uint64_t tdg_vp_vmcall_get_td_vmcall_info(uint64_t *r11,
> uint64_t *r12,
> uint64_t tdg_vp_vmcall_instruction_rdmsr(uint64_t index, uint64_t
> *ret_value);
> uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index, uint64_t value);
> uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
> +uint64_t tdg_vp_vmcall_ve_request_mmio_read(uint64_t address, uint64_t size,
> + uint64_t *data_out);
>
> #endif // SELFTEST_TDX_TDX_H
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h
> index 8a9b6a1bec3e..af412b764604 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/test_util.h
> @@ -35,6 +35,29 @@
> (VCPU)->run->io.direction); \
> } while (0)
>
> +
> +/**
> + * Assert that some MMIO operation involving TDG.VP.VMCALL <#VERequestMMIO>
> was
> + * called in the guest.
> + */
> +#define TDX_TEST_ASSERT_MMIO(VCPU, ADDR, SIZE, DIR) \
> + do {\
> + TEST_ASSERT((VCPU)->run->exit_reason == KVM_EXIT_MMIO, \
> + "Got exit_reason other than KVM_EXIT_MMIO: %u (%s)\n", \
> + (VCPU)->run->exit_reason, \
> + exit_reason_str((VCPU)->run->exit_reason)); \
> + \
> + TEST_ASSERT(((VCPU)->run->exit_reason == KVM_EXIT_MMIO) && \
> + ((VCPU)->run->mmio.phys_addr == (ADDR)) && \
> + ((VCPU)->run->mmio.len == (SIZE)) &&\
> + ((VCPU)->run->mmio.is_write == (DIR)), \
> + "Got an unexpected MMIO exit values: %u (%s) %llu %d
> %d\n", \
> + (VCPU)->run->exit_reason, \
> + exit_reason_str((VCPU)->run->exit_reason), \
> + (VCPU)->run->mmio.phys_addr, (VCPU)->run->mmio.len, \
> + (VCPU)->run->mmio.is_write);\
> + } while (0)
> +
> /**
> * Check and report if there was some failure in the guest, either an
> exception
> * like a triple fault, or if a tdx_test_fatal() was hit.
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> index 9485bafedc38..b19f07ebc0e7 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> @@ -124,3 +124,22 @@ uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t
> interrupt_blocked_flag)
>
> return __tdx_hypercall(&args, 0);
> }
> +
> +uint64_t tdg_vp_vmcall_ve_request_mmio_read(uint64_t address, uint64_t size,
> + uint64_t *data_out)
> +{
> + uint64_t ret;
> + struct tdx_hypercall_args args = {
> + .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
> +
Re: [RFC PATCH v5 18/29] KVM: selftests: TDX: Add TDX MMIO writes test
On 12/12/2023 12:46 PM, Sagi Shahar wrote:
> The test verifies MMIO writes of various sizes from the guest to the host.
>
> Signed-off-by: Sagi Shahar
> Signed-off-by: Ackerley Tng
> Signed-off-by: Ryan Afranji
> ---
> .../selftests/kvm/include/x86_64/tdx/tdx.h| 2 +
> .../selftests/kvm/lib/x86_64/tdx/tdx.c| 14 +++
> .../selftests/kvm/x86_64/tdx_vm_tests.c | 85 +++
> 3 files changed, 101 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> index 13ce60df5684..502b670ea699 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/tdx/tdx.h
> @@ -25,5 +25,7 @@ uint64_t tdg_vp_vmcall_instruction_wrmsr(uint64_t index,
> uint64_t value);
> uint64_t tdg_vp_vmcall_instruction_hlt(uint64_t interrupt_blocked_flag);
> uint64_t tdg_vp_vmcall_ve_request_mmio_read(uint64_t address, uint64_t size,
> uint64_t *data_out);
> +uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> + uint64_t data_in);
>
> #endif // SELFTEST_TDX_TDX_H
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> index b19f07ebc0e7..f4afa09f7e3d 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/tdx/tdx.c
> @@ -143,3 +143,17 @@ uint64_t tdg_vp_vmcall_ve_request_mmio_read(uint64_t
> address, uint64_t size,
>
> return ret;
> }
> +
> +uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> + uint64_t data_in)
> +{
> + struct tdx_hypercall_args args = {
> + .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
> + .r12 = size,
> + .r13 = TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE,
> + .r14 = address,
> + .r15 = data_in,
> + };
> +
> + return __tdx_hypercall(&args, 0);
> +}
> diff --git a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> index 48902b69d13e..5e28ba828a92 100644
> --- a/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> +++ b/tools/testing/selftests/kvm/x86_64/tdx_vm_tests.c
> @@ -885,6 +885,90 @@ void verify_mmio_reads(void)
> printf("\t ... PASSED\n");
> }
>
> +void guest_mmio_writes(void)
> +{
> + uint64_t ret;
> +
> + ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 1, 0x12);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 2,
> 0x1234);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 4,
> 0x12345678);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + ret = tdg_vp_vmcall_ve_request_mmio_write(TDX_MMIO_TEST_ADDR, 8,
> 0x1234567890ABCDEF);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + // Write across page boundary.
> + ret = tdg_vp_vmcall_ve_request_mmio_write(PAGE_SIZE - 1, 8, 0);
> + if (ret)
> + tdx_test_fatal(ret);
> +
> + tdx_test_success();
> +}
> +
> +/*
> + * Varifies guest MMIO writes.
> + */
Nit: typo? Varifies ==> Verifies
> +void verify_mmio_writes(void)
> +{
> + struct kvm_vm *vm;
> + struct kvm_vcpu *vcpu;
> +
> + uint8_t byte_1;
> + uint16_t byte_2;
> + uint32_t byte_4;
> + uint64_t byte_8;
> +
> + vm = td_create();
> + td_initialize(vm, VM_MEM_SRC_ANONYMOUS, 0);
> + vcpu = td_vcpu_add(vm, 0, guest_mmio_writes);
> + td_finalize(vm);
> +
> + printf("Verifying TD MMIO writes:\n");
> +
> + td_vcpu_run(vcpu);
> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> + TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 1,
> TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> + byte_1 = *(uint8_t *)(vcpu->run->mmio.data);
> +
> + td_vcpu_run(vcpu);
> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> + TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 2,
> TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> + byte_2 = *(uint16_t *)(vcpu->run->mmio.data);
> +
> + td_vcpu_run(vcpu);
> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> + TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 4,
> TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> + byte_4 = *(uint32_t *)(vcpu->run->mmio.data);
> +
> + td_vcpu_run(vcpu);
> + TDX_TEST_CHECK_GUEST_FAILURE(vcpu);
> + TDX_TEST_ASSERT_MMIO(vcpu, TDX_MMIO_TEST_ADDR, 8,
> TDG_VP_VMCALL_VE_REQUEST_MMIO_WRITE);
> + byte_8 = *(uint64_t *)(vcpu->run->mmio.data);
> +
> + TEST_ASSERT_EQ(byte_1, 0x12);
> + TEST_ASSERT_EQ(byte_2, 0x1234);
> + TEST_ASSERT_EQ(byte_4, 0x12345678);
> + TEST_ASSERT_EQ(byte_8, 0x1234567890ABCDEF);
> +
> + td_vcpu_run(vcpu);
> + TEST_ASSERT_EQ(vcpu->run->exit_reason, KVM_EXIT_SYST
Re: [PATCH] Documentation: kunit: correct KUNIT_VERY_SLOW to KUNIT_SPEED_VERY_SLOW
On Wed, 20 Mar 2024 at 16:18, Kemeng Shi wrote:
>
> There is no KUNIT_VERY_SLOW, I guess we mean KUNIT_SPEED_VERY_SLOW.
>
> Signed-off-by: Kemeng Shi
> ---
Nice catch, thanks!
Reviewed-by: David Gow
Cheers,
-- David
> Documentation/dev-tools/kunit/running_tips.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/running_tips.rst
> b/Documentation/dev-tools/kunit/running_tips.rst
> index bd689db6fdd2..482f598d141c 100644
> --- a/Documentation/dev-tools/kunit/running_tips.rst
> +++ b/Documentation/dev-tools/kunit/running_tips.rst
> @@ -294,7 +294,7 @@ macro to define the test case instead of
> ``KUNIT_CASE(test_name)``.
> .. code-block:: c
>
> static const struct kunit_attributes example_attr = {
> - .speed = KUNIT_VERY_SLOW,
> + .speed = KUNIT_SPEED_VERY_SLOW,
> };
>
> static struct kunit_case example_test_cases[] = {
> @@ -311,7 +311,7 @@ suite definition.
> .. code-block:: c
>
> static const struct kunit_attributes example_attr = {
> - .speed = KUNIT_VERY_SLOW,
> + .speed = KUNIT_SPEED_VERY_SLOW,
> };
>
> static struct kunit_suite example_test_suite = {
> --
> 2.30.0
>
> --
> You received this message because you are subscribed to the Google Groups
> "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kunit-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kunit-dev/20240320171424.6536-1-shikemeng%40huaweicloud.com.
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()
On 3/22/24 03:21, Andrew Morton wrote: On Thu, 21 Mar 2024 16:05:22 +0530 Dev Jain wrote: Currently, VA exhaustion is being checked by passing a hint to mmap() and expecting it to fail. This patch makes a stricter test by successful write() calls from /proc/self/maps to a dump file, confirming that a free chunk is indeed not available. What's wrong with the current approach? While populating the lower VA space, mmap() fails because we have exhausted the space. Then, in validate_lower_address_hint(), because mmap() fails, we confirm that we have indeed exhausted the space. There is a circular logic involved here. Assume that there is a bug in mmap(), also assume that it exists independent of whether you pass a hint address or not; that for some reason it is not able to find a 1GB chunk. My idea is to assert the exhaustion against some other method. Also, in the following line in validate_complete_va_space(): if (start_addr - prev_end_addr >= SZ_1GB) I made a small error, I forgot to use MAP_CHUNK_SIZE instead of SZ_1GB.
