Message-ID: <a4c8f423-372c-9ec5-8e8a-52e212f0ddc6@intel.com>
Date:   Fri, 3 Dec 2021 10:18:17 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.3.2
From:   Reinette Chatre <reinette.chatre@intel.com>
Subject: Re: [PATCH 10/25] x86/sgx: Support enclave page permission changes
To:     Dave Hansen <dave.hansen@intel.com>, <dave.hansen@linux.intel.com>,
        <jarkko@kernel.org>, <tglx@linutronix.de>, <bp@alien8.de>,
        <luto@kernel.org>, <mingo@redhat.com>, <linux-sgx@vger.kernel.org>,
        <x86@kernel.org>
CC:     <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>
 <44fe170cfd855760857660b9f56cae8c4747cc15.1638381245.git.reinette.chatre@intel.com>
 <24447a03-139a-c7e0-9ad5-34e2019c4df5@intel.com>
Content-Language: en-US
In-Reply-To: <24447a03-139a-c7e0-9ad5-34e2019c4df5@intel.com>
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?N2ljb3lGelE2NUQrdVdCcmx3VFI5MmNiTWZ2UmU2blF2dmlRUzJhY2o3WjhL?=
 =?utf-8?B?VWZBTEtlVFZUKythcVBRcTFudFpCSnZZRXVLeXBXVk92d1hmVm96Uk5GZlZi?=
 =?utf-8?B?enYvcWk5REJnd3dLSXF0dHhoRGtQYnZ5ZEFYNDgvZmNOMlkzT28wVmdEVldk?=
 =?utf-8?B?bWVXL2YyYlEzd0g2TkM1STZybERBK2ZySzBydUFBMXRVYllmMlErZG5Qd2RW?=
 =?utf-8?B?OThaa0xKb1NMK1Z1NDIzVjV3czNhRHVqTUZ3c1V5RldYY3R6YWlDVkUwVG1Q?=
 =?utf-8?B?SG9XdndmL0k0OVg1c1ZuRmlyVFBGYlRmdjNCWklQRkF0cTZiTmVGMTlrUVMv?=
 =?utf-8?B?QU04NXdRVllWa0M2cEU0MzZaN0VhdjlwZnRjRTd2TTVrc25hYWk5aXA1bnJR?=
 =?utf-8?B?THFVRytUK3VySTArSkxaalc3ZWUxbGFnYzVoVHBVaHlaK0IwaWdiSWhJRjA3?=
 =?utf-8?B?cWZBM2JJY2lvd1ZjVEIzWWhZd3NGUjVUYTlGQXJLdG16ZXVONnF6VFJoU2Zw?=
 =?utf-8?B?L25NMnNCdmU5UkVTZTEvemt1ZUpMV1ZodEtaLzdZMU9kdm9RTTZSTFVUK2V1?=
 =?utf-8?B?NDdJYm4yWnVJbTdxYzRXQTY5cnFobzRna2JmME9uUTZHOEhrQ0U4Y0NISC93?=
 =?utf-8?B?TG9qZ2l1Um9FdmNhYms0SHMvMWJ1azhTeWVNb2V5NVBNcVZRT2FWSFZTNFI2?=
 =?utf-8?B?WE5MT2xQTGpNVHhzdGV0Sk9jdjdvdjFwazVzNFljUTlxYnJvemVFb0JZUFRl?=
 =?utf-8?B?RllKRS9pTHo0MlFDUXYxRjdHQ2JQVVNvQWRUQzVZRHpFTTlWS2RHb2JiOEZt?=
 =?utf-8?B?YTczMnFUZmRvMW40T0kvamZ6K3Vmb0NHMzlOelROQ093ejJWRldscFZLemJw?=
 =?utf-8?B?eGRJTmd3dHJkTmpUWkJCWTBRRzlWNm5XV3VXeWJBL2hhQ2NuTkNzeWI4akdj?=
 =?utf-8?B?Wk8wRmZxRW5RRW9waTZLYjhqRGlzVVFjajhjaWtLdW9oMUovc2JxaG1XbkFM?=
 =?utf-8?B?ZndpUXVlY2pidFBDRkNLY1FpWWxJY3B5emRRbkFXY2NxRVpvb3lPOGJjTzhn?=
 =?utf-8?B?ejFlcHZ6ZG0wUXpVVDBGRG9WNngvUk40TmNoWEhWUmF5NVhWcXN4ZDVGb1Jo?=
 =?utf-8?B?UlF0YTFsdGxiY2U3WGdPMGRiMEFwOG1EUjNPaExFZnVkSjFCZzkvUkFreGla?=
 =?utf-8?B?dE1RWSt0bGg4UUVEcjBxYndNUFRFYUhJM0ZHa3NpS0w5eW5YNVFhWDB1bkov?=
 =?utf-8?B?eWZwd2tGMnFzV0lvUkplQURoVmlVamhtb3pFMHF3N1ZxeUhNWnNyYW4xRnNM?=
 =?utf-8?B?R1h3aXRhcHVnaDlLb3FHVGFFMDEzVDgwa3V5bVlmaitScTNXUk5PWmllTDJk?=
 =?utf-8?B?WHdmRUR4S0p0WEpQcFdvTUxrVFVxZ1RnMDlEWUE2ckhCU24zcFZKWXFxQWJH?=
 =?utf-8?B?ZVNRdGkrdkRqdENaOVNqZGppT1RGZG5TNzFVMWRNT2ZNeUdtL0FOazczQTRF?=
 =?utf-8?B?czlWRm1OUmE0bXpLRGtZelE3a0lkUm9GQXpaaFh1ckJMSWJpa0wvZ21lbUJD?=
 =?utf-8?B?SlJhbTNuRXdVd2thVWVvLzliTUw1RmFZSHByYWFtZHFwZjV5RUczNDYrS0cr?=
 =?utf-8?B?cG9XM3JJM1RVUGNUb2FudW04WXNPYzg0VkRDYTk0V1hNa2xZL0lhTEQ2Yzhr?=
 =?utf-8?B?WUZCMU9uaHJmUWQ0M3VWbVp6RW9pWXNDZDJKaGFPR01ucC9wSW01TUZwSTNv?=
 =?utf-8?B?Ukc2dzE3S1NMSGNnNTY0QzFaRzA2NkJUV3ZZUUsrTUhiUWcvU0w4ajJIN3Zk?=
 =?utf-8?B?ak5OY2VkYkNnQ2UvZmRGS0JKcDJ0ci9MSk5sM2p6aE5OZGxFKzJkdnZKU3Fi?=
 =?utf-8?B?Ry9zVnIrR2RXVWJHWnhhQmVVVVVrKzlRY1BqaUtHUEpaM0JuakpocTU3ZkFV?=
 =?utf-8?B?TnkwcVZ5dHliOXhDcjF0dEg0OC8yTC90T0hrZ0xNT0dxU1lCVjBveVZac1RH?=
 =?utf-8?B?U0lGTm5DbzZPcDRQVEtlMDB5d0RQNkRYSjc0TVBHZ2ZvUlMzSzJRaVVaSFNy?=
 =?utf-8?B?RWZTcllMMGFNUGZjM0JVWlBSTXVKUXcvS2FIME5oWDVlU3o4eVBnRi9wVmZ0?=
 =?utf-8?B?eTV4aGNPeFpyaE9Icm1YNjFDTDJSNUluNlRXZTk1aGNwMjVZL1RKR1NGRmxp?=
 =?utf-8?B?ZFMyQUErV1dPS3lIM3pvd2pWNWgxRzNGcmZVcjVnZ0w4REFIekR1cDcwOFov?=
 =?utf-8?Q?2IJgFDuGeGDXx1zvZmblG0/Iec5pR2TuORuW+1RCAI=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: faf17f16-62c8-4f76-3683-08d9b6894a67
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Dec 2021 18:18:22.6938
 (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: wdWuocSY9lYalhU1jTPmEUPiR8MQlT2MTbS4hxDj5fbU3yRPOyDjOFz99JrJPHLIbznhfsYHw9jimdiAIpLCL6HTh2R61/m3qnL55l3i6ro=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR11MB3554
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Dave,

On 12/2/2021 3:48 PM, Dave Hansen wrote:
> On 12/1/21 11:23 AM, Reinette Chatre wrote:
>> + * EPCM permissions can be extended anytime directly from the enclave with
>> + * no visibility from the OS. This is accomplished with ENCLU[EMODPE]
>> + * run from within enclave. Accessing pages with the new, extended,
>> + * permissions requires the OS to update the PTE to handle the subsequent
>> + * #PF correctly.
> 
> Hi Reinette,
> 
> I really dislike the Intel nomenclature here.  I know the Intel docs are
> all written around permission "extension", but I find it ambiguous.
> 
> I've been looking at these instructions literally for years now and
> permission extension to me can mean either:
>   1. The set of things you can do is extended
>   2. The set of things you can *NOT* do is extended
> 
> I much rather prefer nomenclature like:
> 
> 	EPCM permissions can be relaxed anytime directly from the
> 	enclave with no visibility from the OS. This is accomplished
> 	with ENCLU[EMODPE] run from within enclave. Accessing pages with
> 	the new, relaxed permissions requires the OS to update the PTE
> 	to handle the subsequent correctly.
> 
> "Relax" is less ambiguous.  Relaxing a restriction and relaxing
> permissions both mean doing things less strictly.  Extending
> restrictions and extending what is allowed are opposites.

Very good point.

> Maybe it's just me and I need to get this through my thick skull at some
> point.  But, I do think it's OK to improve on the architecture names for
> things when they go into the kernel.  The XSAVE XSTATE_BV->xfeatures
> rename comes to mind.
> 
> Anyway, I'd appreciate if you could keep this in mind and consider
> changing it if a future revision is needed if you believe it is more clear.
> 

Will do. I see that there is opportunity to use this terminology in my 
reply to your other message in response to this patch. I'll do so and we 
can then further judge how it sounds.

Reinette