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: <5d19be91-3aef-5cbe-6063-3ff3dbd5572b@intel.com>
Date:   Tue, 30 Aug 2022 15:56:29 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.2.0
Subject: Re: [PATCH 5/6] selftests/sgx: retry the ioctls returned with EAGAIN
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: <20220830031206.13449-1-jarkko@kernel.org>
 <20220830031206.13449-6-jarkko@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20220830031206.13449-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?TG10WDlXTkdUTGhxN2N6aWM5OUpESm5BN1RsakdoeG5vNDJJSTFZcGpZaHBx?=
 =?utf-8?B?Um5GMno0cE9KYjY0UVpKS0gzY0NWRmpRU0tyREQ2M0pEeVk1cWtHd2Vjcy9h?=
 =?utf-8?B?b0RoV0V3NDltYjBtczJ3UUZsQUg3N1NsOW5FTFdyYnYvOWVoTkdzQ0E2YUQ0?=
 =?utf-8?B?NHZ0QklQMmJQRzc5Y3F1ZExiZG00VHpxa1M1SnVOc1RTN29Qd3d1cU12aWVC?=
 =?utf-8?B?ZTZVMUFwZURVUHVpZ29KMWFIZ2Vld3R0MmJ5ck8rbHNpRHNrMDdIQnoySFFI?=
 =?utf-8?B?UG5EN2FRbm5FYkV3WEV1OENUNGFJT2hRaUFneUk5Q1Y3Z2FHUTFTMjlNbFRU?=
 =?utf-8?B?ZXcxalJYZlpyWlN3S3hxRTAybkZ5S0FNNFJTYUhEVUNERnBRbStPN2dkdWx1?=
 =?utf-8?B?d3VSUXN5bmQ1QjVjMCs3NDNQYndaNjQvTUNOT0NlRHpxKysxNUlrdzBOaTlS?=
 =?utf-8?B?WlFvbkNJdG00ak9iQjRFZ2xmVW15Y0dqVWwzeHhpWk9IU2E1dUFJZ09FSU9j?=
 =?utf-8?B?OTlmbDBWMy9qV25rRE5oSjNDczZrY3A3UG1NeExDMGFUNWI0TW5LaitVTkNU?=
 =?utf-8?B?SGU1UDJ3ZmVFMzZ6czNNclV2YW1vRVdQZ3VzbS9uWlFIR082QmdZeVAxZXJX?=
 =?utf-8?B?aUdzdGxsZXhOb0ZHSnpHak85S0l4VS9NVFRDOWtiVkw1Q3Rjd3JUVHhTYTl3?=
 =?utf-8?B?ZTI2T2orTCs5SXZHNTd3Y2UwOVZpbmIzd2VQNFZyQndtWDBmSFpLZUltSFNn?=
 =?utf-8?B?ZDNRZm00b3RwaEhCREx4d1NiWUkxeDVuSyttOVMrSitKZm9OWmJEUjM5ak0x?=
 =?utf-8?B?VXRRQTdNVDdoWE5oZnJLUmY3STZnUWpXN2xLd2Mrb2N1OHg3SU5WVzVYZUhr?=
 =?utf-8?B?SXI0ZVRJODRzWjE5VEtSbnlsMVRTbG1abzRueE8wamlCYlh2R2QvZW1PNzYv?=
 =?utf-8?B?blZNb2dYclF0SE9od3JZQ1FnRVRuUUZpY1ZycU9oQW81QXNsakFGd0ZqeXI2?=
 =?utf-8?B?R2xmSldWbzVoeUdpMDJOZWhGWnR3K1h1NEllREwxd2NKaGo0NWFVa0NMNFFH?=
 =?utf-8?B?ZE1PNE9LN0VZekZrNFZ5UHQ5bUVJMGZ4emNtM29FbEFWbkFUanQ2cDZkT0dK?=
 =?utf-8?B?VXlsaU52NnBndUlmdVAvSytpbGpmK3Y2aFZ0MVNTNEdCQXVVZ2U5TkpqT1NY?=
 =?utf-8?B?Q0VWZVN4ZUl1dnZvMGROdmlDcit5YlZqQVpuZ0FkME9HRWEvaUFVT1UzRXRD?=
 =?utf-8?B?YkQzK0V6anUvTkZCbWpQT2NwMEdBL2liaEZVRXlNTm00TGZGWVJpZVpVNnkr?=
 =?utf-8?B?TkNNbEs2YVlIQkFxRzBXbWc3V1BHK2NTaXlML0Yra2hvcEJHczIyNml5L3Ir?=
 =?utf-8?B?b3VWNGNJenRqTk5TdzRETk1kMTNXUWZtSzBrV2o4UDVnNDlRMDdJd29IbVVi?=
 =?utf-8?B?OElhRHNuU1lvOWZ2aG9wNTd4Nm53UU9SNjlMN21jVVU0QmVuK3JJRWFZQ1RO?=
 =?utf-8?B?cVZCTktGd2x6dzM5cUd2WmtvenlDSU9IT1dRZFUzSXFPays2bk84OFVpdXRM?=
 =?utf-8?B?U1ZmQ0t3VUJVKzBkQ1hqeUZaVmFYQVUrTW5TeEZXNlhORlhwTzd1ZFQwRmVI?=
 =?utf-8?B?emRrNllYNmp0azVUWXNPMm81aHcvZGRNVFdWU00wZzJxRjF1SkhlZDd3YVd2?=
 =?utf-8?B?OGVTUmwzRURQZFVSRW14bkRoVGM2UzdDM0JBaDV0a0dIN0pMOTdOZE4vWEFY?=
 =?utf-8?B?a3hIVXZFL0hPWTZ1bUpmYzlEdVVGSWFwTnNURWloZnd0cUNJNnJPYVRTR2sw?=
 =?utf-8?B?REp2YWt4N0Rnems2NjZOR2h0QXlFcDJqMFhMVlFuOG9yWVhDS0VhbGFXSk1I?=
 =?utf-8?B?VHBZbmFvdVNxZDJwMHVCNnNQSzhIeE5CRytmZ0RQS0pKYVdnVjdJRm4vd1ZM?=
 =?utf-8?B?bndoM3NxbU53d0hRZUZLSTdjbXZIeUM4eHFZTURJV1hLQ0xzcm5GMjlPS2NU?=
 =?utf-8?B?TWJYa1VBcTZxNUMwMHN1T1R6MmZ2WEd4VzdGcVRSL21sWW9iQWc3VUlpVW1D?=
 =?utf-8?B?MWFlZjMwT1JDNEZ3ZFVVanJDVXJDUEl2WUdxQm9WcEpwTlpUZDZPOW11ZzhB?=
 =?utf-8?B?UHFzaXlFY3VJYTE1c3FKVkREalRqYnZBY1JZRjdwaUdHV2NwQVJlVUZNdDR1?=
 =?utf-8?B?ZWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f3a132de-25da-4944-b17a-08da8adae130
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Aug 2022 22:56:31.3008
 (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: UTxPRluEwvs7dbdk99rEo9WFvCF5n79jFa4OM2LbLM0QTPWOfJqxXMiCkmnxQTxAzvt47hx42u/2Rnz4+ADLDlzS6i+PqIVH6ZOBceFnAzk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB3487
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 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_MED,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 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;
> +		} else
> +			break;

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;
> +		} 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)

Reinette