Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long

[Reinette Chatre]

Hi Jarkko,

On 9/1/2022 3:22 PM, Jarkko Sakkinen wrote:
> On Wed, Aug 31, 2022 at 01:07:35PM -0700, Reinette Chatre wrote:
>> On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:

...

>>>  tools/testing/selftests/sgx/load.c |   5 +-
>>>  tools/testing/selftests/sgx/main.c | 143 +++++++++++++++++++++++++----
>>>  tools/testing/selftests/sgx/main.h |   3 +-
>>
>> Is this test passing on your system? This version is missing the change to
>> mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
> 
> I *did* get a pass in my test machine. Hmm... I'll check if
> the kernel tree was out-of-sync, which could be the reason.
> 
> I do not compile kernel on that machine but have the kernel
> tree for running selftests. So there is a possiblity for
> a human error. Thanks for pointing this out.

On my system I encounter the failure below (V1 of this series
did not have this problem):

[SNIP]
ok 11 enclave.augment_via_eaccept
#  RUN           enclave.augment_via_eaccept_long ...
SGX_IOC_ENCLAVE_INIT failed: Operation not permitted
# main.c:236:augment_via_eaccept_long:0x0000000000000000 0x0000000000002000 0x03
# main.c:236:augment_via_eaccept_long:0x0000000000002000 0x0000000000001000 0x05
# main.c:236:augment_via_eaccept_long:0x0000000000003000 0x0000000000006000 0x03
# main.c:236:augment_via_eaccept_long:0x0000000000009000 0x0000000000001000 0x03
# main.c:251:augment_via_eaccept_long:Failed to initialize the test enclave.
# main.c:1260:augment_via_eaccept_long:Expected 0 (0) != setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, EDMM_SIZE_LONG) (0)
# augment_via_eaccept_long: Test terminated by assertion
#          FAIL  enclave.augment_via_eaccept_long
not ok 12 enclave.augment_via_eaccept_long
[SNIP]

...

>>>  
>>>  static const uint64_t MAGIC = 0x1122334455667788ULL;
>>>  static const uint64_t MAGIC2 = 0x8877665544332211ULL;
>>> +/* Message-ID: <DM8PR11MB55912A7F47A84EC9913A6352F6999@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> */
>>> +static const uint64_t EDMM_SIZE_LONG = 8L * 1024L * 1024L * 1024L;
>>> +static const uint64_t TIMEOUT_LONG = 900; /* seconds */
>>> +
>>
>> Apologies if my feedback was vague - I actually think that the comments in V1 added
>> valuable information, it was just the variation in formatting that was distracting.
> 
> IMHO message ID is pretty good reference. Can you
> propose how would you redo it to minimize the number
> of iterations in the series?

The message ID is a good reference but it points to an email thread
and as used here it is unclear what part of that thread is referred to.
What you had in V1 was very helpful so it could be:

/*
 * The size was chosen based on a bug report:
 * Message-ID: <DM8PR11MB55912A7F47A84EC9913A6352F6999@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
 */

I am not sure about Message-ID vs url. The latter may be more
convenient since the user needs to first search which inbox the message-ID belongs
to before the message can be accessed. Not a big deal though so I think
either works.

Reinette