Received-SPF: pass (google.com: domain of linux-kernel+bounces-156818-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Content-Type: text/plain;
	charset=utf-8
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.100.2.1.4\))
Subject: Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's
 ioctl() to avoid softlockup
From: "Bojun Zhu" <zhubojun.zbj@antgroup.com>
In-Reply-To: <D0SAGP6DPKXF.2EHTXI9UH0HQ9@kernel.org>
Date: Wed, 24 Apr 2024 19:50:49 +0800
Cc: "=?UTF-8?B?SHVhbmcsIEthaQ==?=" <kai.huang@intel.com>,
  "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
  "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
  "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
  "=?UTF-8?B?5YiY5Y+MKOi9qeWxuSk=?=" <ls123674@antgroup.com>,
  "=?UTF-8?B?Q2hhdHJlLCBSZWluZXR0ZQ==?=" <reinette.chatre@intel.com>,
  "Bojun Zhu" <zhubojun.zbj@antgroup.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <D428BE51-20DA-42C5-A2B0-B264D7564700@antgroup.com>
References: <20240423092550.59297-1-zhubojun.zbj@antgroup.com>
 <20240423092550.59297-2-zhubojun.zbj@antgroup.com>
 <02fe00c3b10e4476d500ad7a34024b7eae5e3c97.camel@intel.com>
 <85dfbebc-a2d5-4828-b3b9-f929cd6e30cf@antgroup.com>
 <D0S5S2NL1N5P.3Q0SM01VIZBJX@kernel.org>
 <D0SAGP6DPKXF.2EHTXI9UH0HQ9@kernel.org>
To: Jarkko Sakkinen <jarkko@kernel.org>

Hi Jarkko,

> On Apr 24, 2024, at 18:42, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>=20
> On Wed Apr 24, 2024 at 10:02 AM EEST, Jarkko Sakkinen wrote:
>> On Wed Apr 24, 2024 at 9:46 AM EEST, Bojun Zhu wrote:
>>> Based on the the discussion among you, Jarkko and Reinette,
>>> I will keep the need_resched() and wrap the logic in using =
sgx_resched(),
>>> as suggested by Jarkko.
>>=20
>> Sounds like a plan :-)
>=20
> In sgx_ioc_enclave_add_pages() "if (!c)" check might cause possibly
> some  confusion.
>=20
> Reason for it is that in "transaction sense" the operation can
> be only meaningfully restarted when no pages have not been added
> as MRENCLAVE checksum cannot be reset.
>=20
> BR, Jarkko
>=20


Thanks for your reminder.

According to the SGX hardware specification, similar to "ADD PAGES=E2=80=9D=
,
the operations in  "MODT PAGEs=E2=80=9D and  "REMOVE PAGEs(at =
runtime)=E2=80=9D =20
can be only meaningfully restarted when no page has been handled.

For the following code in sgx_ioc_enclave_add_pages():

```c
if (signal_pending(current)) {
	if (!c)
		ret =3D -ERESTARTSYS;

	break;
}
```
I still have some questions:

It seems that the variable "ret" is set to 0 if there is **some** EPC =
pages have been=20
added when interrupted by signal(Supposed that sgx_encl_add_page()=20
always returns successfully).

Shall we set the return value as "-EINTR" if the context is interrupted =
by=20
signal when some EPC pages have been added?
(BTW, return values contain -EINTR as shown in
=
https://elixir.bootlin.com/linux/latest/source/arch/x86/kernel/cpu/sgx/ioc=
tl.c#L402)

Please correct me if misunderstood it.

Regards,
Bojun=