From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel may not finish in one shot and return EAGAIN error code and count of bytes of EPC pages on that operations are finished successfully.
Change the unclobbered_vdso_oversubscribed_remove test to rerun the ioctls in a loop, updating offset and length using the byte count returned in each iteration.
Signed-off-by: Haitao Huang haitao.huang@linux.intel.com Tested-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org --- tools/testing/selftests/sgx/main.c | 39 +++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 867e98502120..3268d8b01b0b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -399,7 +399,7 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) unsigned long total_mem; int ret, errno_save; unsigned long addr; - unsigned long i; + unsigned long i, count;
/* * Create enclave with additional heap that is as big as all @@ -461,16 +461,27 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) modt_ioc.offset = heap->offset; modt_ioc.length = heap->size; modt_ioc.page_type = SGX_PAGE_TYPE_TRIM; - + count = 0; TH_LOG("Changing type of %zd bytes to trimmed may take a while ...", heap->size); - ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc); - errno_save = ret == -1 ? errno : 0; + do { + ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc); + errno_save = ret == -1 ? errno : 0; + if (errno_save == EAGAIN) { + count += modt_ioc.count; + modt_ioc.offset += modt_ioc.count; + modt_ioc.length -= modt_ioc.count; + modt_ioc.result = 0; + modt_ioc.count = 0; + } else + break; + } while (modt_ioc.length != 0);
EXPECT_EQ(ret, 0); EXPECT_EQ(errno_save, 0); EXPECT_EQ(modt_ioc.result, 0); - EXPECT_EQ(modt_ioc.count, heap->size); + count += modt_ioc.count; + EXPECT_EQ(count, heap->size);
/* EACCEPT all removed pages. */ addr = self->encl.encl_base + heap->offset; @@ -498,15 +509,25 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900)
remove_ioc.offset = heap->offset; remove_ioc.length = heap->size; - + count = 0; TH_LOG("Removing %zd bytes from enclave may take a while ...", heap->size); - ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc); - errno_save = ret == -1 ? errno : 0; + do { + ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc); + errno_save = ret == -1 ? errno : 0; + if (errno_save == EAGAIN) { + count += remove_ioc.count; + remove_ioc.offset += remove_ioc.count; + remove_ioc.length -= remove_ioc.count; + remove_ioc.count = 0; + } else + break; + } while (remove_ioc.length != 0);
EXPECT_EQ(ret, 0); EXPECT_EQ(errno_save, 0); - EXPECT_EQ(remove_ioc.count, heap->size); + count += remove_ioc.count; + EXPECT_EQ(count, heap->size); }
TEST_F(enclave, clobbered_vdso)
Hi Haitao and Jarkko,
selftests/sgx: Retry the ioctl()s returned with EAGAIN
On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel
ioctl()s?
may not finish in one shot and return EAGAIN error code and count of bytes of EPC pages on that operations are finished successfully.
Change the unclobbered_vdso_oversubscribed_remove test to rerun the ioctls in a loop, updating offset and length
ioctl()s?
using the byte count returned in each iteration.
This is a valuable addition, thank you very much.
Signed-off-by: Haitao Huang haitao.huang@linux.intel.com Tested-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org
tools/testing/selftests/sgx/main.c | 39 +++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 867e98502120..3268d8b01b0b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -399,7 +399,7 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) unsigned long total_mem; int ret, errno_save; unsigned long addr;
- unsigned long i;
- unsigned long i, count;
Reverse fir tree?
/* * Create enclave with additional heap that is as big as all @@ -461,16 +461,27 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) modt_ioc.offset = heap->offset; modt_ioc.length = heap->size; modt_ioc.page_type = SGX_PAGE_TYPE_TRIM;
- count = 0; TH_LOG("Changing type of %zd bytes to trimmed may take a while ...", heap->size);
- ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
- errno_save = ret == -1 ? errno : 0;
- do {
ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);errno_save = ret == -1 ? errno : 0;if (errno_save == EAGAIN) {count += modt_ioc.count;modt_ioc.offset += modt_ioc.count;modt_ioc.length -= modt_ioc.count;modt_ioc.result = 0;
As part of the test I think it would be helpful to know if there was a result code in here. What do you think of failing the test if it is not zero?
modt_ioc.count = 0;} elsebreak;
Watch out for unbalanced braces (also later in patch). This causes checkpatch.pl noise.
- } while (modt_ioc.length != 0);
EXPECT_EQ(ret, 0); EXPECT_EQ(errno_save, 0); EXPECT_EQ(modt_ioc.result, 0);
- EXPECT_EQ(modt_ioc.count, heap->size);
- count += modt_ioc.count;
- EXPECT_EQ(count, heap->size);
/* EACCEPT all removed pages. */ addr = self->encl.encl_base + heap->offset; @@ -498,15 +509,25 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) remove_ioc.offset = heap->offset; remove_ioc.length = heap->size;
- count = 0; TH_LOG("Removing %zd bytes from enclave may take a while ...", heap->size);
- ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
- errno_save = ret == -1 ? errno : 0;
- do {
ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);errno_save = ret == -1 ? errno : 0;if (errno_save == EAGAIN) {count += remove_ioc.count;remove_ioc.offset += remove_ioc.count;remove_ioc.length -= remove_ioc.count;remove_ioc.count = 0;} elsebreak;- } while (remove_ioc.length != 0);
EXPECT_EQ(ret, 0); EXPECT_EQ(errno_save, 0);
- EXPECT_EQ(remove_ioc.count, heap->size);
- count += remove_ioc.count;
- EXPECT_EQ(count, heap->size);
} TEST_F(enclave, clobbered_vdso)
Reinette
On Tue, Aug 30, 2022 at 03:56:29PM -0700, Reinette Chatre wrote:
Hi Haitao and Jarkko,
selftests/sgx: Retry the ioctl()s returned with EAGAIN
On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel
ioctl()s?
Ioctl is common enough to be considered as noun and is widely phrased like that in commit messages. I don't see any added clarity.
may not finish in one shot and return EAGAIN error code and count of bytes of EPC pages on that operations are finished successfully.
Change the unclobbered_vdso_oversubscribed_remove test to rerun the ioctls in a loop, updating offset and length
ioctl()s?
using the byte count returned in each iteration.
This is a valuable addition, thank you very much.
Signed-off-by: Haitao Huang haitao.huang@linux.intel.com Tested-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org
tools/testing/selftests/sgx/main.c | 39 +++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 867e98502120..3268d8b01b0b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -399,7 +399,7 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) unsigned long total_mem; int ret, errno_save; unsigned long addr;
- unsigned long i;
- unsigned long i, count;
Reverse fir tree?
+1
/* * Create enclave with additional heap that is as big as all @@ -461,16 +461,27 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) modt_ioc.offset = heap->offset; modt_ioc.length = heap->size; modt_ioc.page_type = SGX_PAGE_TYPE_TRIM;
- count = 0; TH_LOG("Changing type of %zd bytes to trimmed may take a while ...", heap->size);
- ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
- errno_save = ret == -1 ? errno : 0;
- do {
ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);errno_save = ret == -1 ? errno : 0;if (errno_save == EAGAIN) {count += modt_ioc.count;modt_ioc.offset += modt_ioc.count;modt_ioc.length -= modt_ioc.count;modt_ioc.result = 0;As part of the test I think it would be helpful to know if there was a result code in here. What do you think of failing the test if it is not zero?
I would not mind doing that.
modt_ioc.count = 0;} elsebreak;Watch out for unbalanced braces (also later in patch). This causes checkpatch.pl noise.
Again. I did run checkpatch to all of these. Will revisit.
- } while (modt_ioc.length != 0);
EXPECT_EQ(ret, 0); EXPECT_EQ(errno_save, 0); EXPECT_EQ(modt_ioc.result, 0);
- EXPECT_EQ(modt_ioc.count, heap->size);
- count += modt_ioc.count;
- EXPECT_EQ(count, heap->size);
/* EACCEPT all removed pages. */ addr = self->encl.encl_base + heap->offset; @@ -498,15 +509,25 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) remove_ioc.offset = heap->offset; remove_ioc.length = heap->size;
- count = 0; TH_LOG("Removing %zd bytes from enclave may take a while ...", heap->size);
- ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
- errno_save = ret == -1 ? errno : 0;
- do {
ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);errno_save = ret == -1 ? errno : 0;if (errno_save == EAGAIN) {count += remove_ioc.count;remove_ioc.offset += remove_ioc.count;remove_ioc.length -= remove_ioc.count;remove_ioc.count = 0;} elsebreak;- } while (remove_ioc.length != 0);
EXPECT_EQ(ret, 0); EXPECT_EQ(errno_save, 0);
- EXPECT_EQ(remove_ioc.count, heap->size);
- count += remove_ioc.count;
- EXPECT_EQ(count, heap->size);
} TEST_F(enclave, clobbered_vdso)
Reinette
BR, Jarkko
Hi Jarkko,
On 8/30/2022 7:31 PM, Jarkko Sakkinen wrote:
On Tue, Aug 30, 2022 at 03:56:29PM -0700, Reinette Chatre wrote:
Hi Haitao and Jarkko,
selftests/sgx: Retry the ioctl()s returned with EAGAIN
On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel
ioctl()s?
Ioctl is common enough to be considered as noun and is widely phrased like that in commit messages. I don't see any added clarity.
ok. I was asked to make this change in the SGX2 patches and thought that I should propagate this advice :)
modt_ioc.count = 0;} elsebreak;Watch out for unbalanced braces (also later in patch). This causes checkpatch.pl noise.
Again. I did run checkpatch to all of these. Will revisit.
It looks like I see it because I use "checkpatch.pl --strict".
Reinette
On Wed, Aug 31, 2022 at 11:09:21AM -0700, Reinette Chatre wrote:
Hi Jarkko,
On 8/30/2022 7:31 PM, Jarkko Sakkinen wrote:
On Tue, Aug 30, 2022 at 03:56:29PM -0700, Reinette Chatre wrote:
Hi Haitao and Jarkko,
selftests/sgx: Retry the ioctl()s returned with EAGAIN
On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel
ioctl()s?
Ioctl is common enough to be considered as noun and is widely phrased like that in commit messages. I don't see any added clarity.
ok. I was asked to make this change in the SGX2 patches and thought that I should propagate this advice :)
I can use the other form too, np.
modt_ioc.count = 0;} elsebreak;Watch out for unbalanced braces (also later in patch). This causes checkpatch.pl noise.
Again. I did run checkpatch to all of these. Will revisit.
It looks like I see it because I use "checkpatch.pl --strict".
Thanks BTW for pointing this out :-)
Reinette
BR, Jarkko
On 8/30/22 19:31, Jarkko Sakkinen wrote:
On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel
ioctl()s?
Ioctl is common enough to be considered as noun and is widely phrased like that in commit messages. I don't see any added clarity.
I definitely prefer ioctl().
On Wed, Aug 31, 2022 at 11:14:02AM -0700, Dave Hansen wrote:
On 8/30/22 19:31, Jarkko Sakkinen wrote:
On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
From: Haitao Huang haitao.huang@linux.intel.com
For EMODT and EREMOVE ioctls with a large range, kernel
ioctl()s?
Ioctl is common enough to be considered as noun and is widely phrased like that in commit messages. I don't see any added clarity.
I definitely prefer ioctl().
I can use it.
BR, Jarkko
linux-kselftest-mirror@lists.linaro.org
-
Dave Hansen -
Jarkko Sakkinen -
Reinette Chatre