PEK 850: Adding part of the higher priority tests #165
Conversation
tests/tests/test_guest_cpu_off.py
Outdated
| # turn off arbitrary cpus | ||
| cpu_on_off('/sys/devices/system/cpu/cpu17/online', 0) | ||
| cpu_on_off('/sys/devices/system/cpu/cpu60/online', 0) | ||
| cpu_on_off('/sys/devices/system/cpu/cpu109/online', 0) |
There was a problem hiding this comment.
the cpu number can depend on the platform we are testing; using static numbers can be a problem
tests/tests/test_guest_cpu_off.py
Outdated
| m = Qemu.QemuSSH(qm) | ||
|
|
||
| # turn off arbitrary cpus | ||
| cpu_on_off('/sys/devices/system/cpu/cpu17/online', 0) |
There was a problem hiding this comment.
maybe it is good to turn on those cores before running the guest to make sure turning of them here might be impactful for the guest
tests/tests/test_guest_cpu_off.py
Outdated
| qm.stop() | ||
|
|
||
| # turn back on cpus | ||
| cpu_on_off('/sys/devices/system/cpu/cpu17/online', 1) |
There was a problem hiding this comment.
i would bring on the cores as soon as possible when i m done to avoid situations where there is an exception in the test (here it will be at the guest stop) and we do not bring them back on
tests/tests/test_guest_cpu_off.py
Outdated
| qm = Qemu.QemuMachine() | ||
| qm.run() | ||
|
|
||
| # get pid of Qemu machine (not written immediately sometimes) |
There was a problem hiding this comment.
I would abstract this feature in the Qemu.py as a service, we can have a check_run function that makes sure that the qemu process gets started and returns the pid of the qemu process; this will allow to simplify the tests reading. The idea is to make the test case as "human" friendly as possible
tests/tests/test_guest_cpu_off.py
Outdated
| m = Qemu.QemuSSH(qm) | ||
|
|
||
| # pin pid to a particular cpu | ||
| cs = subprocess.run(['sudo', 'taskset', '-pc', '18', f'{pid}'], capture_output=True) |
There was a problem hiding this comment.
again; using the static cpu number here can be a problem, if we do not want a complex test; we can skip the test if the number of core is less than 64 and we will use static numbers less than 64 in our tests and we can be sure that those numbers are valid
tests/tests/test_guest_fail.py
Outdated
| assert cs.returncode == 0, 'Failed getting dmesg' | ||
| dmesg_start = str(cs.stdout) | ||
|
|
||
| # Setup ept=0 in driver |
There was a problem hiding this comment.
I would put this as a function in util.py; load_kvm_intel() for example
tests/tests/test_guest_fail.py
Outdated
| dmesg_start = str(cs.stdout) | ||
|
|
||
| # Setup ept=0 in driver | ||
| cs = subprocess.run(['sudo', 'rmmod', 'kvm_intel'], check=True) |
There was a problem hiding this comment.
the rmmod will fail if there is any VM running, rmmod: ERROR: Module kvm_intel is in use ; i dont know how we can handle this situation
hector-cao
force-pushed
the
pek_850_partial
branch
from
bd31fc9
to
799991c
Compare
tests/tests/setup-env-tox.sh
Outdated
| python3-cpuinfo &> /dev/null | ||
| # Make sure kvm_intel is installed properly | ||
| # (one of the tests installed it with NOEPT) | ||
| sudo rmmod kvm_intel |
There was a problem hiding this comment.
@spmcmillan what is the reason to reload kvm_intel here ? i think kvm_intel is part of the prerequisite of the testing (like kernel, qemu installation). Moreover, modprobe kvm_intel might not be enough, we should provide the argument tdx=1 : modprobe kvm_intel tdx=1
There was a problem hiding this comment.
It says in the comment, but we load it differently at times on the different tests. So, this is to make sure we load it the same way at the beginning of each test.
There was a problem hiding this comment.
ideally, each test should reload the kernel module properly if it previously unloaded it, but in practice, if there is any reason you cannot have this certainty, loading it in the tox global setup will not be enough, we have to make sure to do it at the beginning of each test. So i think, for now, i will go for the ideal situation where each test should unload/reload the kernel module properly
There was a problem hiding this comment.
true and agreed
hector-cao
force-pushed
the
pek_850_partial
branch
from
799991c
to
839dbba
Compare
have a better cpu selection mechanism
add also kvm_intel module manager (as context manager for better robustness)
use the kvm_module context manager fix the qemu failure check
+ improve test check
hector-cao
force-pushed
the
pek_850_partial
branch
from
839dbba
to
5109e86
Compare
Adding tests to tdx set. Still need to add to checkbox tests in a separate PR with the rest of the tests specified in PEK850 in next pulse.
Fixes PEK-850