Received-SPF: pass (google.com: domain of linux-kernel+bounces-146958-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Tue, 16 Apr 2024 17:10:12 +0300
Message-Id: <D0LLVE07V8O0.S8XF3CY2DQ9A@kernel.org>
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Jarkko Sakkinen" <jarkko@kernel.org>, "Haitao Huang"
 <haitao.huang@linux.intel.com>, <dave.hansen@linux.intel.com>,
 <kai.huang@intel.com>, <tj@kernel.org>, <mkoutny@suse.com>,
 <linux-kernel@vger.kernel.org>, <linux-sgx@vger.kernel.org>,
 <x86@kernel.org>, <cgroups@vger.kernel.org>, <tglx@linutronix.de>,
 <mingo@redhat.com>, <bp@alien8.de>, <hpa@zytor.com>,
 <sohil.mehta@intel.com>, <tim.c.chen@linux.intel.com>
Cc: <zhiquan1.li@intel.com>, <kristen@linux.intel.com>, <seanjc@google.com>,
 <zhanb@microsoft.com>, <anakrish@microsoft.com>,
 <mikko.ylinen@linux.intel.com>, <yangjie@microsoft.com>,
 <chrisyan@microsoft.com>
Subject: Re: [PATCH v12 14/14] selftests/sgx: Add scripts for EPC cgroup
 testing
References: <20240416032011.58578-1-haitao.huang@linux.intel.com>
 <20240416032011.58578-15-haitao.huang@linux.intel.com>
 <D0LLS28WEXYA.G15BAG7WOJGR@kernel.org>
In-Reply-To: <D0LLS28WEXYA.G15BAG7WOJGR@kernel.org>

On Tue Apr 16, 2024 at 5:05 PM EEST, Jarkko Sakkinen wrote:
> On Tue Apr 16, 2024 at 6:20 AM EEST, Haitao Huang wrote:
> > With different cgroups, the script starts one or multiple concurrent SG=
X
> > selftests (test_sgx), each to run the unclobbered_vdso_oversubscribed
> > test case, which loads an enclave of EPC size equal to the EPC capacity
> > available on the platform. The script checks results against the
> > expectation set for each cgroup and reports success or failure.
> >
> > The script creates 3 different cgroups at the beginning with following
> > expectations:
> >
> > 1) SMALL - intentionally small enough to fail the test loading an
> > enclave of size equal to the capacity.
> > 2) LARGE - large enough to run up to 4 concurrent tests but fail some i=
f
> > more than 4 concurrent tests are run. The script starts 4 expecting at
> > least one test to pass, and then starts 5 expecting at least one test
> > to fail.
> > 3) LARGER - limit is the same as the capacity, large enough to run lots=
 of
> > concurrent tests. The script starts 8 of them and expects all pass.
> > Then it reruns the same test with one process randomly killed and
> > usage checked to be zero after all processes exit.
> >
> > The script also includes a test with low mem_cg limit and LARGE sgx_epc
> > limit to verify that the RAM used for per-cgroup reclamation is charged
> > to a proper mem_cg. For this test, it turns off swapping before start,
> > and turns swapping back on afterwards.
> >
> > Add README to document how to run the tests.
> >
> > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com>
>
> jarkko@mustatorvisieni:~/linux-tpmdd> sudo make -C tools/testing/selftest=
s/sgx run_tests
> make: Entering directory '/home/jarkko/linux-tpmdd/tools/testing/selftest=
s/sgx'
> gcc -Wall -Werror -g -I/home/jarkko/linux-tpmdd/tools/testing/selftests/.=
/../../tools/include -fPIC -c main.c -o /home/jarkko/linux-tpmdd/tools/tes=
ting/selftests/sgx/main.o
> gcc -Wall -Werror -g -I/home/jarkko/linux-tpmdd/tools/testing/selftests/.=
/../../tools/include -fPIC -c load.c -o /home/jarkko/linux-tpmdd/tools/tes=
ting/selftests/sgx/load.o
> gcc -Wall -Werror -g -I/home/jarkko/linux-tpmdd/tools/testing/selftests/.=
/../../tools/include -fPIC -c sigstruct.c -o /home/jarkko/linux-tpmdd/tool=
s/testing/selftests/sgx/sigstruct.o
> gcc -Wall -Werror -g -I/home/jarkko/linux-tpmdd/tools/testing/selftests/.=
/../../tools/include -fPIC -c call.S -o /home/jarkko/linux-tpmdd/tools/tes=
ting/selftests/sgx/call.o
> gcc -Wall -Werror -g -I/home/jarkko/linux-tpmdd/tools/testing/selftests/.=
/../../tools/include -fPIC -c sign_key.S -o /home/jarkko/linux-tpmdd/tools=
/testing/selftests/sgx/sign_key.o
> gcc -Wall -Werror -g -I/home/jarkko/linux-tpmdd/tools/testing/selftests/.=
/../../tools/include -fPIC -o /home/jarkko/linux-tpmdd/tools/testing/selft=
ests/sgx/test_sgx /home/jarkko/linux-tpmdd/tools/testing/selftests/sgx/main=
o /home/jarkko/linux-tpmdd/tools/testing/selftests/sgx/load.o /home/jarkko=
/linux-tpmdd/tools/testing/selftests/sgx/sigstruct.o /home/jarkko/linux-tpm=
dd/tools/testing/selftests/sgx/call.o /home/jarkko/linux-tpmdd/tools/testin=
g/selftests/sgx/sign_key.o -z noexecstack -lcrypto
> gcc -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE -fno-stack-p=
rotector -mrdrnd -I/home/jarkko/linux-tpmdd/tools/testing/selftests/../../.=
/tools/include test_encl.c test_encl_bootstrap.S -o /home/jarkko/linux-tpm=
dd/tools/testing/selftests/sgx/test_encl.elf -Wl,-T,test_encl.lds,--build-i=
d=3Dnone
> /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld:=
 warning: /tmp/ccqvDJVg.o: missing .note.GNU-stack section implies executab=
le stack
> /usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld:=
 NOTE: This behaviour is deprecated and will be removed in a future version=
 of the linker
> TAP version 13
> 1..2
> # timeout set to 45
> # selftests: sgx: test_sgx
> # TAP version 13
> # 1..16
> # # Starting 16 tests from 1 test cases.
> # #  RUN           enclave.unclobbered_vdso ...
> # #            OK  enclave.unclobbered_vdso
> # ok 1 enclave.unclobbered_vdso
> # #  RUN           enclave.unclobbered_vdso_oversubscribed ...
> # #            OK  enclave.unclobbered_vdso_oversubscribed
> # ok 2 enclave.unclobbered_vdso_oversubscribed
> # #  RUN           enclave.unclobbered_vdso_oversubscribed_remove ...
> # # main.c:402:unclobbered_vdso_oversubscribed_remove:Creating an enclave=
 with 98566144 bytes heap may take a while ...
> # # main.c:457:unclobbered_vdso_oversubscribed_remove:Changing type of 98=
566144 bytes to trimmed may take a while ...
> # # main.c:473:unclobbered_vdso_oversubscribed_remove:Entering enclave to=
 run EACCEPT for each page of 98566144 bytes may take a while ...
> # # main.c:494:unclobbered_vdso_oversubscribed_remove:Removing 98566144 b=
ytes from enclave may take a while ...
> # #            OK  enclave.unclobbered_vdso_oversubscribed_remove
> # ok 3 enclave.unclobbered_vdso_oversubscribed_remove
> # #  RUN           enclave.clobbered_vdso ...
> # #            OK  enclave.clobbered_vdso
> # ok 4 enclave.clobbered_vdso
> # #  RUN           enclave.clobbered_vdso_and_user_function ...
> # #            OK  enclave.clobbered_vdso_and_user_function
> # ok 5 enclave.clobbered_vdso_and_user_function
> # #  RUN           enclave.tcs_entry ...
> # #            OK  enclave.tcs_entry
> # ok 6 enclave.tcs_entry
> # #  RUN           enclave.pte_permissions ...
> # #            OK  enclave.pte_permissions
> # ok 7 enclave.pte_permissions
> # #  RUN           enclave.tcs_permissions ...
> # #            OK  enclave.tcs_permissions
> # ok 8 enclave.tcs_permissions
> # #  RUN           enclave.epcm_permissions ...
> # #            OK  enclave.epcm_permissions
> # ok 9 enclave.epcm_permissions
> # #  RUN           enclave.augment ...
> # #            OK  enclave.augment
> # ok 10 enclave.augment
> # #  RUN           enclave.augment_via_eaccept ...
> # #            OK  enclave.augment_via_eaccept
> # ok 11 enclave.augment_via_eaccept
> # #  RUN           enclave.tcs_create ...
> # #            OK  enclave.tcs_create
> # ok 12 enclave.tcs_create
> # #  RUN           enclave.remove_added_page_no_eaccept ...
> # #            OK  enclave.remove_added_page_no_eaccept
> # ok 13 enclave.remove_added_page_no_eaccept
> # #  RUN           enclave.remove_added_page_invalid_access ...
> # #            OK  enclave.remove_added_page_invalid_access
> # ok 14 enclave.remove_added_page_invalid_access
> # #  RUN           enclave.remove_added_page_invalid_access_after_eaccept=
 ...
> # #            OK  enclave.remove_added_page_invalid_access_after_eaccept
> # ok 15 enclave.remove_added_page_invalid_access_after_eaccept
> # #  RUN           enclave.remove_untouched_page ...
> # #            OK  enclave.remove_untouched_page
> # ok 16 enclave.remove_untouched_page
> # # PASSED: 16 / 16 tests passed.
> # # Totals: pass:16 fail:0 xfail:0 xpass:0 skip:0 error:0
> ok 1 selftests: sgx: test_sgx
> # timeout set to 45
> # selftests: sgx: run_epc_cg_selftests.sh
> # # Setting up limits.
> # ./run_epc_cg_selftests.sh: line 50: echo: write error: Invalid argument
> # # Failed setting up misc limits.
> not ok 2 selftests: sgx: run_epc_cg_selftests.sh # exit=3D1
> make: Leaving directory '/home/jarkko/linux-tpmdd/tools/testing/selftests=
/sgx'
>
> This is what happens now.
>
> BTW, I noticed a file that should not exist, i.e. README. Only thing
> that should exist is the tests for kselftest and anything else should
> not exist at all, so this file by definiton should not exist.

I'd suggest to sanity-check the kselftest with a person from Intel who
has worked with kselftest before the next version so that it will be
nailed next time. Or better internal review this single patch with a=20
person with expertise on kernel QA.

I did not check this but I have also suspicion that it might have some
checks whetehr it is run as root or not. If there are any, those should
be removed too. Let people set their environment however want...

BR, Jarkko