Message-ID: <66151f3a-0e32-fc57-cb54-5b714588389b@intel.com>
Date:   Mon, 6 Dec 2021 13:30:57 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.3.2
Subject: Re: [PATCH 07/25] x86/sgx: Move PTE zap code to separate function
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>
CC:     <dave.hansen@linux.intel.com>, <tglx@linutronix.de>,
        <bp@alien8.de>, <luto@kernel.org>, <mingo@redhat.com>,
        <linux-sgx@vger.kernel.org>, <x86@kernel.org>, <seanjc@google.com>,
        <kai.huang@intel.com>, <cathy.zhang@intel.com>,
        <cedric.xing@intel.com>, <haitao.huang@intel.com>,
        <mark.shanahan@intel.com>, <hpa@zytor.com>,
        <linux-kernel@vger.kernel.org>
References: <cover.1638381245.git.reinette.chatre@intel.com>
 <bd228c90c139437bb4fcc4b7b99063bfd3eb1439.1638381245.git.reinette.chatre@intel.com>
 <Yavy5JTYAkdZjnK2@iki.fi>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <Yavy5JTYAkdZjnK2@iki.fi>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a3QrVWR5WERoWmlLeldRM215d0ZielNxYXhhK3R3TTVjREhIMFBmbVZBTDFT?=
 =?utf-8?B?L1crcmlDdU5MQjNPUEVLUi9KeXJBbStvQzVBTnZMaE43aGlaQ2wvNjZ1Nk12?=
 =?utf-8?B?TGxWRnVzS1BQOHMyNGhkSnNEcnVDK3dhdEkxRmdMVUR0V29aNm5BRTk3UWpK?=
 =?utf-8?B?YnpZaFp6VjBXM0IzVzFyekhJcjNrb0VmRE1rZFdQZHdOMXZlN1BpcjV1UzBO?=
 =?utf-8?B?eG04VFZ5RlZqV2hscEF5VkJFNVlhcEtWaWY1TXdHMVZNUWMxcE1hRGNPVXBI?=
 =?utf-8?B?TTRzNTA3KzVaRm03cmNEU3N5NUIvMW9DbU9sR0FWZVJiUk1KeHVldWx0S1Br?=
 =?utf-8?B?YXlkSmJpeGI5TVpXUjNHQlRFd2JhQVdYMXlyNGhlM0Q1RGhvTEc3MFJlRSsy?=
 =?utf-8?B?aENyZVBTejVhRnBETE1oYU5ORlovWU5oNWxQd3FFSjV5aXlwTC92a2M1ajlm?=
 =?utf-8?B?UDJVRkM0S2QxMi85U2h1VVBmUXl4N2k2d3EycjhveUIvNTRBVGRZRHhMZ0VR?=
 =?utf-8?B?Y0s3Nk1VcmpqM3RaQUtpVHdSdmhJdkJmWHVTZkpDRmpzK202eW1NQWY0UGNE?=
 =?utf-8?B?UjRrOGtnL2V5RDNYS1NwS0xmQWdtZmp0TTk4M0dTWFBYbGpIUVQyMmNBMFRk?=
 =?utf-8?B?ZWw0RUMyKzFHeDlNVytRN0doVTJiZzFZTldPL3E4ajc3TysvS3ZGR1pZdXQ3?=
 =?utf-8?B?WWIvdUVCYUk3b3l6OTNTcWNCL2daTVNuVGR1MDZTanVoVVUzVU5vMGgyQnNZ?=
 =?utf-8?B?Z1RIcWVhYmx3KzNLWnkzb2FyS0Y5YmE0YTVuYXBBR3BKWE15dXVFY3FFVnBw?=
 =?utf-8?B?THY0bTFpY05pSW5zejEwTGxMVTkyc0MxRjQzTWFlVkRGazcwUzRMbEhiQVFs?=
 =?utf-8?B?d0orTVdXOVFHNVdmODRxU2VTVUZtdE9BbTJHbFQrYlNyZEp5c0VTbmFPZmFj?=
 =?utf-8?B?WXJ6bkdmVEhnR3BaV2ZNK0lWeFAvMXl4RkFGMW9lK1pwOUhjd2l6dENKRE1t?=
 =?utf-8?B?dDNPTnhkVjZ1UXIwV3JMM2RWODF6RjNONE5KcnVQNUxPaVhWUldGaUVxUTRl?=
 =?utf-8?B?Zmswb1pscU44QWd5MWp1UDc5VnBhN3FXb01yOFV1SHBidGJGOWNCU05yQlpz?=
 =?utf-8?B?cVZra1Foc1JsSXp1cjBOdTBkNHZUbWxNRTBSeEsySE4vWHNVcGw1NlBvYUNS?=
 =?utf-8?B?TjJ5M21XZDBvSGhJWktpbnlUTmUvRGZiSjVQTFQ2VmdyTUNrL3hhMkkzUzFi?=
 =?utf-8?B?V0hTL0FXQnIxUWFJNXMyQm9rb2l1WUhkYnRMbFdZdS9uQWVFUzlTaTRxL2N2?=
 =?utf-8?B?bWRITXBSakpFa2NRT1Z4alpvMTdDL0M1bytWekFZU3phWGVGZlZwWG8wcDBn?=
 =?utf-8?B?cHZ3b0pyd3RLVmg2VnBwdisvSXpjL25ibmxZK0FiVUhGWFVCbjFuYTlreXBZ?=
 =?utf-8?B?M2laZ1Y0clZxSjVER053Rkx0TEZMUDNzRStMZ3B3eG9oZE9yQmFHZ0JDbDhj?=
 =?utf-8?B?VVp2UjRHVEluOUhqcE1qd2hjUUtHNmVjZlVYbDBORGNvYVZqMU1LZ2cvWnZL?=
 =?utf-8?B?NUhTcjNHaExYdFlvdFFNUHFOemR2TmFwRUJCOTF5Z2ZmQXhmOEJ0eUtUeVVL?=
 =?utf-8?B?dE9saitHaDJsVFlEeFBDMTY3bFJxcVRQZlViSHVaMDZjdk5lM2ZLSHJ1cUE5?=
 =?utf-8?B?UGtXWjdLdi9YZkVOYi9zNis3c2pQVnNTNG5kcFU3U0tUOU5lU1ErQXFlcHV3?=
 =?utf-8?B?QnFtTFJsNXNvdGxzZjBlU2tPVXBXRHBVeEZ5UEFMOTdYZjhoV29Ea0VnU1k0?=
 =?utf-8?B?OXdqSFJOVjZkeDdBYStqSUxQTWljOEtTbjhSbSswNFpzM3VmMmk0cGk1R2dM?=
 =?utf-8?B?YjlIZGZ1VGhTM1hKVUZ4dTZsK0RXUWtmOWdMS054NHRidUVpUkZIbGdqUUti?=
 =?utf-8?B?OU1iNzdLdEpVNzRCb1FNWk9RNlhDeFVSaUFZZmtlUUhKUU9Ua0Zhc3VYeE1r?=
 =?utf-8?B?TzBQakxhUnJaM25kQ05yTEF0NGdmcGJrMFpZMnQrUDA2cWVqN2ZCMmluM0Ru?=
 =?utf-8?B?N2IzbTFySGU1TVVkZy80L1I5Rml0UTR3a3U1RHZQNTd2eWIzOXVuWlBPQUV1?=
 =?utf-8?B?K0twNW9hTmxvOUhoMzF6OERRRFd0OXo4cWlnU0FEbFlvNWdSTmJObU43akxJ?=
 =?utf-8?B?c1FJZ0JvcFBOaVpQS1RQSDZLVU5ENmJQTzdnK2cxQ1JGOERLUGZXOGZlZWdT?=
 =?utf-8?Q?1N8apGlq7uy6KHqF0Oo42RvGleIGuoO00VOCa4voTA=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 498b59cd-4c4e-489b-b6a3-08d9b8ffb354
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2021 21:31:01.7168
 (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: pEOYEiGSid2MwFL646REnwUPY3PnUqSfAnUVD0J1pyMidUY5H6VQlHntvI92oKFIokABJIGFYHm5xBwM+TuXTUbMQ12Z+L00BRNjC0CrQvQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB1843
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jarkko,

On 12/4/2021 2:59 PM, Jarkko Sakkinen wrote:
> On Wed, Dec 01, 2021 at 11:23:05AM -0800, Reinette Chatre wrote:
>> The SGX reclaimer removes page table entries pointing to pages that are
>> moved to swap. SGX2 enables changes to pages belonging to an initialized
>> enclave, for example changing page permissions. Supporting SGX2 requires
>> this ability to remove page table entries that is available in the
>> SGX reclaimer code.
> 
> Missing: why SGX2 requirest this?

The above paragraph states that SGX2 needs to remove page table entries 
because it modifies page permissions. Could you please elaborate what is 
missing?

> 
>> Factor out the code removing page table entries to a separate function,
>> fixing accuracy of comments in the process, and make it available to other
>> areas within the SGX code.
>>
>> Since the code will no longer be unique to the reclaimer it is relocated
>> to be with the rest of the enclave code in encl.c interacting with the
>> page table.
> 
> This last paragraph should be removed. It can be seen from the code change
> and diffstat.

I understand that the code movement can be seen from the diffstat but 
the reason for the move may not be obvious to everybody. If it is ok 
with you I'd rather keep this text.

Reinette