Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ecd03fbb-8019-07ae-e27d-44b1dd20bfb1@intel.com>
Date:   Thu, 8 Sep 2022 15:44:29 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.2.1
Subject: Re: [PATCH v2 5/5] selftests/sgx: Add SGX selftest
 augment_via_eaccept_long
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>, <linux-sgx@vger.kernel.org>
CC:     Haitao Huang <haitao.huang@linux.intel.com>,
        Vijay Dhanraj <vijay.dhanraj@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "Shuah Khan" <shuah@kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
References: <20220905020411.17290-1-jarkko@kernel.org>
 <20220905020411.17290-6-jarkko@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20220905020411.17290-6-jarkko@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZEE5eXhOZ0FCdTdzR2crVnhYcElLck91c3hCQkMvdTNJYkYvQkllbzdnaU1E?=
 =?utf-8?B?SlNTYUxsSitGWmdhTXdUZ0JKcDY4cWpYQ3ZFNHVpOGw3WVNiTURiUlQ5ejdm?=
 =?utf-8?B?L2oxNHZoTThodFlja0pRank4NkpGcFFoditwUVhsUmlLc3ZnMEpreENseWZW?=
 =?utf-8?B?SlgyaUFXZy9zRmlCNElLU0ZKOWtWOGhnVXRVRnpXTFc4SjZQZlI1TnUrU0NY?=
 =?utf-8?B?cW9PbXoxeUtaYUFEd3E5NU9sWE1aaUZkelQ2YjVFcFpjOXplUm0yaEFPRGc3?=
 =?utf-8?B?d04vT1ZUODlFYU0yOEFsRzJPWlpFL3B5RnpPSWhqOHFyMFJDNGxlUmxhMVVW?=
 =?utf-8?B?cXp6eVh1NGJZOGU5NVhxMDhjSUI1TUVSeEtmdzVYOFBXL1EycGRaV1RDNjFu?=
 =?utf-8?B?c3F4M2h0Mk5ZTUE0MEw1Qk9GS0xmYVY5OStoUmZUTXJiblpEdlRhMCtRZHNm?=
 =?utf-8?B?OUpvK2xhaXlyeFBJcEVZTms5RUYyRHRPWDZjNE1rZ2pUMXgrRlBCWmRCaHZ3?=
 =?utf-8?B?OXMrWHFTVzRCRUYwS01Ia2tWc3FLdnl5SXlURnFxaHVJTUlZM2dUVkp3dUxw?=
 =?utf-8?B?aUZ3ckxFcml3NzB5ak1pVk5BalUwbko0OVRMRzI1c0I4Q0pHUnJndmNFK1Bq?=
 =?utf-8?B?bUJ3Y3hiWmR4MDRNMHRPczE0ZU1CcGNBL3Q0ZXQyWWpHWmVBK2NQbW53UXNX?=
 =?utf-8?B?TWludGZaWWo1ZXFaYlQ1V2h0UGRJUzhTM25paTM1dEx4MU12WXBSV2djZWE0?=
 =?utf-8?B?ZzJBeEV1b2UveThzc3JTSVlZLzY4VmZHekhicjQ4RGkveG12R0l3enBlbkVT?=
 =?utf-8?B?RHY5YWZxd1lpTGxwaFBMb1Y4aXJJYTRMME1aQ0hxS1dBMW56NEJINDgxTzNO?=
 =?utf-8?B?bGU4d0lUSElkeFowUFFiUFlaZCtwT1kyRzU1Y0k0WlNrVlVvMFJ6emZWREtO?=
 =?utf-8?B?aEZldjJReDkzQjQwekdpYnR2c3lXVjM5SEVkcGlRaS8wWWZqK1M4OTRKSkFr?=
 =?utf-8?B?N3ZYTkJKd09jbDFrNFk5VnFtUFRtbHI5UU5Rd2ZvMnVvK0NIMHQzMG4zREZ0?=
 =?utf-8?B?cWJFQmpGaG9PVENERzh2Y3d3VmU4MWFweEpMOWVXSjFFemNWLzc3dzQ3Mkw5?=
 =?utf-8?B?aHphQjdtVHdrSEJxRFhlcS9QR0lKdGlrVHA5V01PTnBKdUo2RERueVJUbExz?=
 =?utf-8?B?aFl0NERVQm1BZ3RJZ3NQVmVtcTlsdXU0QUlMbmVjWVFrc0dicXhDWVZ6aDdT?=
 =?utf-8?B?N1U4bnZiMTRNdEJsbXVXeVlocFlIRlpodWgzVExDY2JYOUJiUUVvRVZwN2RW?=
 =?utf-8?B?Z216UXduWmZBOXJiRFJlZU51TGhaMVp1SGIvM3MxbW96MGQ0NVI3SlBkZERm?=
 =?utf-8?B?Q1FTV1EyOGVSZXpGaXlLUG93ZjdjYytxSVdJTFdRYVYrTnRWYWQxRHkyU2tm?=
 =?utf-8?B?UW96VWNUb0RtdExyazRVOEhFeWtxV0JnSi8xUWVMT21jN2JuUXk0bzI1bVNx?=
 =?utf-8?B?V0FWMmU0c1BQb2F5YW9LT1BuV08reExVd3NTWmY4ZVJvVk5zaFV2eFIyM3VQ?=
 =?utf-8?B?NURha3hZSDZGU2RKNTFLQmdaT1k0SU1LTjRSQlcrRityc1FoS3c3NU54YWhI?=
 =?utf-8?B?amlTY2F3UCtMczcvYXM1a3pLZlBuSDcwcGFqbXgyZzhab1c5VjdMSUNlcjBU?=
 =?utf-8?B?SnRSL2VKVVdwaFdZeXJHZG9QOUJ5WnpFLzA0YWd4SGhpZWZRc0QvZjlIMnRQ?=
 =?utf-8?B?a2ZhOGpLOGlIVUx1blNDUkhTMnRiRFdQQy91dVNSaVV5eFNISkQ4ZEkvWm5w?=
 =?utf-8?B?SG1IU0hwWlNoVE5zdTNzV2tjMjJFTDdUZXc1ZmR0eEVOeHk2UnNBS1dzZVRH?=
 =?utf-8?B?TC9aMzZubnl2ZmRaNUM4RDYySE5aSXF6YUd2bkxyQVRPZ3BzWlpPNW8wVDA1?=
 =?utf-8?B?aUJpQURoTm5aN2FSMHFxSHFSU2FZWmJpVzF1Yjd1VUxVa1kyd3RaV08xVFVK?=
 =?utf-8?B?RzFNczNxWS9kWWRCRzJweE1iS01STlB0TzR4cGt4ZW1KMWk1c1RucUc2L0VR?=
 =?utf-8?B?aHVvWXBQTHZhSnFUT3lyT3BlZjVKSVliOVJwT2lPWE5HVWE1TzJlSE1DdDhr?=
 =?utf-8?B?ek0vdHI3bUhFNVhNMEFpRFY0V0xsZ1JFRkJNaVRJY2xVTTZtMzA2bXlWYndJ?=
 =?utf-8?B?R0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 162b33eb-4609-4abc-21fb-08da91ebb1d8
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Sep 2022 22:44:31.5138
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cHUZUKodG8CSDZosdJL2IWEF0YIYGIUlxXbOLgwxJZHJPJy30rWCv3IpPWHLgIHTlxUnkd2I4im/VmKbxhfJGFtAegUkbO5CHVXtA5vpZkM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4075
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_HI,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jarkko and Vijay,

On 9/4/2022 7:04 PM, Jarkko Sakkinen wrote:
> From: Vijay Dhanraj <vijay.dhanraj@intel.com>
> 
> Add a new test case which is same as augment_via_eaccept but adds a
> larger number of EPC pages to stress test EAUG via EACCEPT.
> 
> Signed-off-by: Vijay Dhanraj <vijay.dhanraj@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---

...

> diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
> index 78c3b913ce10..e596b45bc5f8 100644
> --- a/tools/testing/selftests/sgx/main.c
> +++ b/tools/testing/selftests/sgx/main.c
> @@ -22,8 +22,10 @@
>  #include "main.h"
>  
>  static const size_t ENCL_HEAP_SIZE_DEFAULT = PAGE_SIZE;
> +static const unsigned long TIMEOUT_DEFAULT = 900;

I am not sure about the naming here ... it is _very_ close to
(and thus appears to be in the same namespace as)
TEST_TIMEOUT_DEFAULT from the included kselftest_harness.h.

This is surely a nitpick but how about SGX_TEST_TIMEOUT_DEFAULT?

>  static const uint64_t MAGIC = 0x1122334455667788ULL;
>  static const uint64_t MAGIC2 = 0x8877665544332211ULL;
> +

There is an extra empty line here ... but it looks like intended code
organization?

>  vdso_sgx_enter_enclave_t vdso_sgx_enter_enclave;
>  
>  /*


Apart from the naming comment this addition looks good. 

This is a valuable addition to the SGX tests.

Reinette