Re: [RFC PATCH v2 01/10] x86/sgx: Introduce mechanism to prevent new initializations of EPC pages

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Cathy,

On 3/17/2022 7:33 PM, Zhang, Cathy wrote:
> Hi Jarkko,
> 
>> -----Original Message-----
>> From: Jarkko Sakkinen <jarkko@kernel.org>
>> Sent: Friday, March 18, 2022 2:53 AM
>> To: Zhang, Cathy <cathy.zhang@intel.com>
>> Cc: linux-sgx@vger.kernel.org; linux-kernel@vger.kernel.org; Hansen, Dave
>> <dave.hansen@intel.com>; Raj, Ashok <ashok.raj@intel.com>
>> Subject: Re: [RFC PATCH v2 01/10] x86/sgx: Introduce mechanism to prevent
>> new initializations of EPC pages
>>
>> On Thu, Mar 17, 2022 at 02:08:21PM +0000, Zhang, Cathy wrote:
>>> Hi Jarkko,
>>>
>>>> -----Original Message-----
>>>> From: Jarkko Sakkinen <jarkko@kernel.org>
>>>> Sent: Thursday, March 17, 2022 12:13 PM
>>>> To: Zhang, Cathy <cathy.zhang@intel.com>
>>>> Cc: linux-sgx@vger.kernel.org; linux-kernel@vger.kernel.org; Hansen,
>>>> Dave <dave.hansen@intel.com>; Raj, Ashok <ashok.raj@intel.com>
>>>> Subject: Re: [RFC PATCH v2 01/10] x86/sgx: Introduce mechanism to
>>>> prevent new initializations of EPC pages
>>>>
>>>> On Tue, Mar 15, 2022 at 09:02:51AM +0800, Cathy Zhang wrote:

...

>>> Yes, it requires to apply the following SGX2 v2 patches to
>>> tip/x86/sgx, prior to this series. As I mentioned in cover letter,
>>> SGX2 introduces new SGX instructions and flows, which should be handled
>> during SVN update.
>>>
>>> The v2 of SGX2 support patch set, with 32 patches in total, refer to the
>> cover letter:
>>> https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@inte
>>> l.com/ The two additional patches SGX2 support requires:
>>> "x86/sgx: Add poison handling to reclaimer"
>>>
>>> https://lore.kernel.org/linux-
>> sgx/dcc95eb2aaefb042527ac50d0a50738c7c16
>>> 0dac.1643830353.git.reinette.chatre@intel.com/
>>> "x86/sgx: Silence softlockup detection when releasing large enclaves"
>>>
>>> https://lore.kernel.org/linux-
>> sgx/b5e9f218064aa76e3026f778e1ad0a1d823e
>>> 3db8.1643133224.git.reinette.chatre@intel.com/
>>
>> Why don't you pick the dependent patches from that patch set and put them
>> to this one? It does not matter if two patch sets contain same patches. If this
>> gets merged first, then Reinette's job will be easier, and vice versa.
>> Tags make sure that everyone gets what they deserve from their contribution.
> 
> For easy to discuss, I split the SGX2 support patches into the following two parts:
> 
> 1. auxiliary changes:
> [V2,09/32] x86/sgx: Export sgx_encl_ewb_cpumask()
> https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@intel.com/T/#m44e2b931e82a87a8b2c80058130182eb747fbcf0
> [V2,10/32] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask()
> https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@intel.com/T/#mf6268a66b5c48ca9a18a772b6eaea097c315dc1d
> [V2,12/32] x86/sgx: Make sgx_ipi_cb() available internally
> https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@intel.com/T/#ma3330f8ee8136aa084d0a2b5f110331e37f44c52
> [V2,14/32] x86/sgx: Keep record of SGX page type
> https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@intel.com/T/#m4ae80fdf67ad330119bfc2abaea845baa24ed14c
> 
> 2. The other SGX2 support patches.
> 
> For #1, we can put them into this series. But regarding #2, it introduces EAUG/EMODPR/EMODT and the related flows, it seems not sensible to pick up only some of the patches, it will be not convenient to run the related test while doing SVN update.
> 
>>
>> Otherwise for instance giving a tested-by is impossible. You have to root this
>> to the mainline, or at least to x86 tip.git.
> 
> Put this series on top of SGX2 support is based on the assumption that SGX2 will get merged first. If it's required to make this series independent of SGX2, we need to pick up the above #1, and make change in patch " x86/sgx: Introduce mechanism to prevent new initializations of EPC pages" to remove the handling on SGX2 flows. Once SGX2 gets merged, provide the changes in following patches.
> 
> How do you think, Jarkko? And I also would like to get suggestion from @Chatre, Reinette for this.

Is it possible to factor out the changes to the SGX2 flows into separate patches?

This should continue to support the testing you mentioned and the patches could
then be dropped from your series and moved to the SGX2 series if the EUPDATESVN
work is merged first. I am not sure about the testing though because I did not
notice a selftest as part of the series. How can I test this work if/when it
moves to the SGX2 series?

Reinette