Received-SPF: pass (google.com: domain of linux-kernel+bounces-209012-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <eb36980a-2a81-426b-82fb-7f598f7a0037@intel.com>
Date: Tue, 11 Jun 2024 10:39:53 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v14 14/14] selftests/sgx: Add scripts for EPC cgroup
 testing
To: Haitao Huang <haitao.huang@linux.intel.com>, "dave.hansen@linux.intel.com"
	<dave.hansen@linux.intel.com>, "tj@kernel.org" <tj@kernel.org>,
	"mkoutny@suse.com" <mkoutny@suse.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "linux-sgx@vger.kernel.org"
	<linux-sgx@vger.kernel.org>, "x86@kernel.org" <x86@kernel.org>,
	"cgroups@vger.kernel.org" <cgroups@vger.kernel.org>, "tglx@linutronix.de"
	<tglx@linutronix.de>, "mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de"
	<bp@alien8.de>, "hpa@zytor.com" <hpa@zytor.com>, "Mehta, Sohil"
	<sohil.mehta@intel.com>, "tim.c.chen@linux.intel.com"
	<tim.c.chen@linux.intel.com>, Jarkko Sakkinen <jarkko@kernel.org>
CC: "Li, Zhiquan1" <zhiquan1.li@intel.com>, "kristen@linux.intel.com"
	<kristen@linux.intel.com>, "seanjc@google.com" <seanjc@google.com>, "Zhang,
 Bo" <zhanb@microsoft.com>, "anakrish@microsoft.com" <anakrish@microsoft.com>,
	"mikko.ylinen@linux.intel.com" <mikko.ylinen@linux.intel.com>,
	"yangjie@microsoft.com" <yangjie@microsoft.com>, "chrisyan@microsoft.com"
	<chrisyan@microsoft.com>
References: <20240531222630.4634-1-haitao.huang@linux.intel.com>
 <20240531222630.4634-15-haitao.huang@linux.intel.com>
 <D1RKK8CENNXI.1KMNDADV9C1YM@kernel.org>
 <op.2owf5xiwwjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <D1SOT40TEXMI.A5J72PR5IWSP@kernel.org>
 <op.2ox4ccz7wjvjmi@hhuan26-mobl.amr.corp.intel.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <op.2ox4ccz7wjvjmi@hhuan26-mobl.amr.corp.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V1BHWDhGcFdPMjZoeG5BRVJmOWVhQkdlM2F3VXFGNHhHSjdkbXlJWElhdmN0?=
 =?utf-8?B?M0ZqeS9PQmlhZmNHb1ovOHgwQWdnanlNWUlzUFBMMjErU2I0eThrTVYrWCtP?=
 =?utf-8?B?UFpIRTNYVUU0TTU3R0dTWlRQRmltUmRBVzBrUGcxdFpjdUE3dEY1Z1VBNTFX?=
 =?utf-8?B?UU9OUGpJeEZpZitFMUdqT3hhWFlHbmJrb2tXK0dZanlPZHNHdmd4YU92VWhX?=
 =?utf-8?B?d3V1WU9mTlpDMElQc0U0L3ZUZm00Y1VZSmIrNFdaM3EzR2d3VE1lMFdDZW1q?=
 =?utf-8?B?RDJTNkxKZFY1T3pibFRxY21LWmN0YUNiVFBMNFdzN2YxL0ovNWFHVGZRRzVD?=
 =?utf-8?B?MjFTeUgyUnU2V2VQVStqNi9Ua1NEc1BTWXRteFcydkhFRGNLRk5rUC91cGVa?=
 =?utf-8?B?MlVIM08zdXYrZktPc0I2RXR5NjRLK2N0dThmcHZacS9sWmpFcE5KTFRVanJL?=
 =?utf-8?B?c2xROEVLUVZnS0JCOW5vRlhZOUo2SHphdTBNYjJjTy9PUzBIOS9RV2J1eXp0?=
 =?utf-8?B?RWVJMWNyNm8vUVYzSVRxMTVJL2NSU2cwVENmVkY3eWs2dHliU1VzM3l0NWVa?=
 =?utf-8?B?d2h0Z0loK3Y4WkNNenVCZzRkQzI4NVdsdXdOcS8rY2NGV3VKcXFvdUROYkkv?=
 =?utf-8?B?ME9PVmk5L3pWbzVTZitKRkNpek5rOTRzdmdkZk9WRVZtT0ZTNG5laGlyaXlD?=
 =?utf-8?B?YWF0TXU4MHpuVk1CRFI2anRqdjBQY3BzT2lmSjMxK3IvWEYrQ3ViUTViSHI5?=
 =?utf-8?B?OGQzckRHSXNrTG14UkwzZCtRNEpFeEtXM0lGaXhyUHAvVDNzVzlmY0pKeGpi?=
 =?utf-8?B?TFFVWU1kVnpsb1JnVlhWUlRrb3d5d3FTSnVDOEdjY2dBNmN5ODhyUTNERjg3?=
 =?utf-8?B?bi9Vc1ROOXVDVlM4R2hIN2txSEptaFJ3bXpmbTRsdHNBeG1SNW9Fbnh6dStB?=
 =?utf-8?B?M2VHVWh6NTRnQW9FTTU1aHJUaHdHeXVnQi9Ndm9MdzhMVVYxQXg5eThaVnFr?=
 =?utf-8?B?YUhiYzRVVlhoeFltWmtXZHlVemIrU01INGszeDFDM0YvRFc4Slh6K2E4NlRV?=
 =?utf-8?B?bkpSdTY1TzZodlhkMnlLY2lzQldHZ25TV0ZYQllaYUgxZDFCeWQ2N1d0ZVVw?=
 =?utf-8?B?UnFOaTNBZlkya2IvTDNYNWo1cWZCR2pDYXFuV1g3YTRpOUFFb1FXRTNuM3hk?=
 =?utf-8?B?anRtOFhMajFVMGpmWWxadXA1YzkrNnhXbW91bGo5WWFzUEVkSHQ3ODdPZ3Fk?=
 =?utf-8?B?eFY0bnF0RU1SRzh3bmNhVm1NQVA4ZjVvMzF5NFY4UTdEa2pha0FVa3FaNUhT?=
 =?utf-8?B?SWwwVUNQK2k4MXNBcloyVi9PT25jL3VOOEVRcGQ5WXNtRzlldGhsc3hGOGUv?=
 =?utf-8?B?eUpQWGUybEFQV1ZYQzFKRGQ0TFR1Tjhyd0g0RmhGVHBXMi8vYlRPYlNxMHpO?=
 =?utf-8?B?RVhuU1V1bytVZjJ0SDlaOTEzeXhvL0I5b2ppblFKTnBYMnMvZjZjQUEzRDRM?=
 =?utf-8?B?SXBSRENKMTNoWTZJbXd3dVRwK0tqOVl6b0dZK1VjQjJqUUx0cExHMGpQZVZ6?=
 =?utf-8?B?RFAwUnhyNUNuZ3Z5YkQ3SjBLTStINjhlSS9tb0RtVS9Rd3dyWWNSbjg1ck5Y?=
 =?utf-8?B?MHJrczZEQi9WZWp1ejZrYnNrd3F2clpmaUJlckl1S2hjWE83V0pCMGZmOUE1?=
 =?utf-8?B?Znc3ZWRxMHJZaloyZmF6MHNZUDdtZDA1UXFGVmhmMVJxc2xoL09iOHhNbnk4?=
 =?utf-8?B?T2V4T3dONk1QUkR6eFp6R2dVSThwRDJhVXFoNElTVW92ckJwbFhuditTb1FE?=
 =?utf-8?B?cVNyWTBhWDZybzJwaHViOUc5WWpnTXpMMmRteFFZZzV3L1V2STk2c1RzdHlJ?=
 =?utf-8?B?ZUU0SlB2b1pLUk8xck5hVUNTM09PTG54MkZNS0ZvMm5peHE0N1A5S0pLQkdk?=
 =?utf-8?B?TGdNbjI5WFhuakw5Uzc2SWpzRno2OE42dWxIbEk5YTlWRnVEVzU1bFVoZmp5?=
 =?utf-8?B?R3VqRXRvTWhNODdBTkJTZVljWGZVdXNBTjJHcUE5OWlpa2FRaXpIYnRUZlpU?=
 =?utf-8?B?Zno1OTNpWHRMK2Zwakx3VXJySnk3TzNHSTRVYmJucEVRM3ZmUXVoc2QrV0lU?=
 =?utf-8?Q?TXGfluJu2DW+Y6Y6sy+umtw4t?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e23735e3-4a9e-4b26-4ba3-08dc899e45fe
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jun 2024 22:40:07.1762
 (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: F1drV8yHTKfeti6Stu4LLkpK32dFDaaDuMfrzZxH721k19co/jlvxymxFZHGp9FyK8sns4mUxvOS9mEZ6NiSfQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6692
X-OriginatorOrg: intel.com


> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -1045,7 +1045,7 @@ static int __init sgx_init(void)
>        if (!sgx_page_cache_init())
>            return -ENOMEM;
> 
> -    if (!sgx_page_reclaimer_init()) {
> +    if (!sgx_page_reclaimer_init() || !sgx_cgroup_init()) {
>            ret = -ENOMEM;
>            goto err_page_cache;
>        }

Does it make more sense to move the sgx_cgroup_init() to the 
sgx_drv_init()?  The SGX cgroup only works for the driver side anyway. 
In this case, if something went wrong in sgx_cgroup_init(), the 
sgx_vepc_init() could still have a chance to work.

And IIUC we need to reset the "capacity" to 0 if sgx_cgroup_init() 
fails, no matter it is called inside sgx_drv_init() or sgx_init(), 
otherwise the "epc" would appear in the cgroup hierarchy as a misc 
cgroup resource.

Another option is to defer setting the capacity to the point where we 
have made sure sgx_drv_init() and sgx_cgroup_init() cannot fail.

Btw, I plan to review the rest from late of this week or next week 
because this week I have some other staff needs to be finished first.