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

["Zhang, Cathy" <cathy.zhang@intel.com>]

Hi Reinette,

> -----Original Message-----
> From: Chatre, Reinette <reinette.chatre@intel.com>
> Sent: Saturday, March 19, 2022 12:24 AM
> To: Zhang, Cathy <cathy.zhang@intel.com>; Jarkko Sakkinen
> <jarkko@kernel.org>
> 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
> 
> 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@in
> >>> te 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@inte
> > l.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@inte
> > l.com/T/#mf6268a66b5c48ca9a18a772b6eaea097c315dc1d
> > [V2,12/32] x86/sgx: Make sgx_ipi_cb() available internally
> > https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@inte
> > l.com/T/#ma3330f8ee8136aa084d0a2b5f110331e37f44c52
> > [V2,14/32] x86/sgx: Keep record of SGX page type
> > https://lore.kernel.org/lkml/cover.1644274683.git.reinette.chatre@inte
> > l.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?

Yes, the changes can be into separate patches. And the separate patches
should be moved out from this series, to make sure this series is applied
to tip branch independent of SGX2 series.

> 
> 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?

To test it, you will the following command while you run SGX2 stress test.

iucode-tool -K xxxxxx.PDB --overwrite ; echo 1 > /sys/devices/system/cpu/microcode/reload

Here xxxxxx.PDB is an ucode patch.

I'm not sure if it's possible to write a selftest for it for selftest should create a
ucode patch.

Additionally, regarding the "auxiliary changes" I list above, would you like to move
them into this SVN update series, or just let me rebase on them, like you rebased
with some other patches?

> 
> Reinette