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: <7be76cf8-28c4-edcf-aa67-703b5c18bef1@intel.com>
Date:   Tue, 30 Aug 2022 15:54:54 -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 2/6] x86/sgx: Handle VA page allocation failure for EAUG
 on PF.
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>,
        "Thomas Gleixner" <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "Borislav Petkov" <bp@alien8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
References: <20220830031206.13449-1-jarkko@kernel.org>
 <20220830031206.13449-3-jarkko@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20220830031206.13449-3-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?STczUjlHZzdpVlViMkFVWmZqUmI0MUpMRUxxMHNQaDhnclFyNDJnaEFRVGlR?=
 =?utf-8?B?WkpLT09tcFNZV21zRkxwcFVzbFN0U2RBbUxINXFiUkZkempyd2NiN1dOU0VK?=
 =?utf-8?B?VmEzRGprM01XZDlMR3dmcllmMmkvVXlZNlpYcGpzWkhUb1BSRmtNR0tPdEpu?=
 =?utf-8?B?czdodTMxNFpwWmUrdWZsS29FZFNKclVEUUMwVnFQZXJOODk0amV6OVd4RXlX?=
 =?utf-8?B?YjZIMHhza3lqc21WT3NDd3dOOXY5cW9JQ280dFFqU1pUZElOMW9RS3IxREJS?=
 =?utf-8?B?OS91Y3JTRVZBWjk3eDFmTnp0SkJGdXQ0TFM3R0pCaDhMRWl4YkI2VGdHM2JL?=
 =?utf-8?B?MGVEaXQxWEtHZkRmMTZlbGpxZUowL3JreFExVk5vNWs1Mi9PcEpLWEllQS9K?=
 =?utf-8?B?WlczUDhSdVgvRXJMZzcwam90ajNINWN6dDBHQzhHcG5UOTIzT3EwL2h1MUU2?=
 =?utf-8?B?Q3A3RVZnSDhpSWtONUd4TStadnJyWGRnTVdTVDFOWVFwZlc3RUVLMkhmMWx1?=
 =?utf-8?B?MnVhWXhlWVpwRDgrMk5zeDJWTzViSEZNZVRmQzNJSWkvMVRTS2E3L0hJZVYw?=
 =?utf-8?B?Q20zNVVnU0pTdDlYZ255c3FNaWVocmhoK2xGeVlqSXA1Snh5Ui9xUVY3ek5W?=
 =?utf-8?B?T2tHZXA2ZE9KTk1WdmgyRGxkRFJXNkMwd2NSRHpSU2J3ckk3TE0yeGJVaVFW?=
 =?utf-8?B?eE1hQ2RWR280Sm1tVjhJUEllMy94VDVTdjRYTkRUdkpGZEpYangwM3BpbEpS?=
 =?utf-8?B?RFBTMmpzM1lTYm9KS2YyYWtJYytVZFBQQm9DeC9aQlIwcy9uYmkrN1ozUjVZ?=
 =?utf-8?B?djdKTTlLT0kvYlVrcFlxTkZSN0dXOEJZakQwd0tKME1GU3Q4My9TWjBxZEJU?=
 =?utf-8?B?b0tMTjlnb0p5WmZubkhnSTdma0ZYNXVPeHpjdzd3TjN0Y1ZwSy93a3oxQTZI?=
 =?utf-8?B?THlxQXdaZzdqcFpOWTJVc3lhcVlISG5GY1IvZTVjVnBzTWFSOFJWTjFxWStw?=
 =?utf-8?B?QnoyeW5CZm1nK3NmNzRESCtUa1BiaHhadlZFZkMxem5OaFFmVE9vZjRQYjdj?=
 =?utf-8?B?d3V0RTErZzdYY3I0ODViNGxFaFNyVTRMdE8zYlVGeDhaYThsZEJUaFdObEVU?=
 =?utf-8?B?RWlpRlZjeU1OWk80MGxFOGxOb2huWmVOZTFMcHRwWVEyWDBmWlRZZy94SVY4?=
 =?utf-8?B?QnpETjVMQnZyUU1BU1JISVNMMU4yTU9jaTdNMFVlQm0ydUY5cUpicE1aUzZO?=
 =?utf-8?B?aXNIYk1Dc3VkbjZNQjNKQTFHZUx2OWVNQURNSXFLNDcvRExzS0ErVGs0U1Zu?=
 =?utf-8?B?cms5WWJ2STA1K1JXeGZJNTVnak9USzlWS3o2aXZJM29pRzk1dGVleXRVZFF1?=
 =?utf-8?B?Y3h2aWsxZmNpRHdtUTh6TCtKSVI2cjBxcE9UellsUWN2bUZVOUJmcU9tMzFB?=
 =?utf-8?B?NlB5bGpOMDRPVEMyaGY5c1hWSFIrZGVxTlp4aFFiQldNMzRtYStqK0VXbW1r?=
 =?utf-8?B?cmZlZUcxaFhQN05mZlNuWGU4RS9pVTBVVW5MZnBUTzBNWENTVmI1MTF0ejhz?=
 =?utf-8?B?anllcUMrTWdRalo2Y21EancycmRpeXQyOW1lQmk0QVB0bGZ6WWtCdEh4U2VM?=
 =?utf-8?B?L3BpZmMxWEVNbjI0OWlMTnhPakluSUJtdm5jMm8vK0RyZ2RRM0JGWFg5SFgw?=
 =?utf-8?B?YjFaODNNZ0NwamtJcGlBNlNNNGdORFZxcC9LcGcvamFFc0U2b2F4d1FVWEpP?=
 =?utf-8?B?MDdQbjBYT294RkpqSEV3NjhCbllSZkNYc3I1aERjRklYSHJwUUJTTjhGQ3lL?=
 =?utf-8?B?OGVWa0NianlHUDlJT0JUSUR6NUM5bXJuSDBsQzlQOExHdTJJZTdXSDlBT1N3?=
 =?utf-8?B?NHVNYlFZQjFPMEQ0L2dWTC9LbHFkbjVsRFIwUVE2SHZLZXZ0a2RmVGhuNmFo?=
 =?utf-8?B?bVdvRzFZMC94OGFBT2p2K21YdjlJR2ZXekRPdXcxOVU5WUo1UTIzRkVqN0tR?=
 =?utf-8?B?VSszYm9vVGpIWWd3MndRRTBaNVZ6T3Q2MFFwWFFUYlhwT1ZmU1dvK2toWFNy?=
 =?utf-8?B?M0Q1cThaaW5tNW44b0JkakpyUzYyWEVDbWc4aWFsdDFIaDZqSnNYMGZLUnlv?=
 =?utf-8?B?ZVhHL1dMeUlxUWlvYVphUkhVZUpvMHFlM3JRdi84bEtaeDdGVTlta2dETFJh?=
 =?utf-8?B?bGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e590abd2-ef4d-4feb-5aea-08da8adaa92f
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:54:57.3552
 (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: TH2JR3/bbpFTcMpdx/mxlyweFlnv6U+/SI2givzVwlXKh7cdDS5cHNYw2VWpXfkE8aGL5RyS73QK7Wqq7ndaKKePRLMTR35nrycZtcSqYXU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6523
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 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,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 Haitao,

On 8/29/2022 8:12 PM, Jarkko Sakkinen wrote:
> From: Haitao Huang <haitao.huang@linux.intel.com>
> 
> VM_FAULT_NOPAGE is expected behaviour for -EBUSY failure path, when
> augmenting a page, as this means that the reclaimer thread has been
> triggered, and the intention is just to round-trip in ring-3, and
> retry with a new page fault.

Thank you very much for finding and fixing this issue.

> 
> Fixes: 5a90d2c3f5ef ("x86/sgx: Support adding of pages to an initialized enclave")
> Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com>
> Tested-by: Vijay Dhanraj <vijay.dhanraj@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v2:
> * Removed reviewed-by, no other changes.
> ---
>  arch/x86/kernel/cpu/sgx/encl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index 24c1bb8eb196..de92c1c8b79d 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -344,8 +344,11 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma,
>  	}
>  
>  	va_page = sgx_encl_grow(encl, false);
> -	if (IS_ERR(va_page))
> +	if (IS_ERR(va_page)) {
> +		if (PTR_ERR(va_page) == -EBUSY)
> +			vmret =  VM_FAULT_NOPAGE;

Some unnecessary white space remained after the copy of this snippet. No
need to propagate that mistake.

>  		goto err_out_epc;
> +	}
>  
>  	if (va_page)
>  		list_add(&va_page->list, &encl->va_pages);


Apart from the white space nitpick this looks good.

Thank you very much.

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette