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;
Date:   Wed, 2 Mar 2022 05:03:58 +0100
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     Reinette Chatre <reinette.chatre@intel.com>
Cc:     Dave Hansen <dave.hansen@intel.com>,
        "Dhanraj, Vijay" <vijay.dhanraj@intel.com>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "bp@alien8.de" <bp@alien8.de>,
        "Lutomirski, Andy" <luto@kernel.org>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "linux-sgx@vger.kernel.org" <linux-sgx@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "Huang, Kai" <kai.huang@intel.com>,
        "Zhang, Cathy" <cathy.zhang@intel.com>,
        "Xing, Cedric" <cedric.xing@intel.com>,
        "Huang, Haitao" <haitao.huang@intel.com>,
        "Shanahan, Mark" <mark.shanahan@intel.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V2 16/32] x86/sgx: Support restricting of enclave page
 permissions
Message-ID: <Yh7srrm9S9ml/yGL@iki.fi>
References: <4ce06608b5351f65f4e6bc6fc87c88a71215a2e7.1644274683.git.reinette.chatre@intel.com>
 <YhLhoMFPyOFZ2fsX@iki.fi>
 <DM8PR11MB55917F499CDF4CC7D426B0A7F63C9@DM8PR11MB5591.namprd11.prod.outlook.com>
 <Yhy/GvJegnTqYdq6@iki.fi>
 <f6a256a6-a7d7-a8e7-c9a8-e232203c63af@intel.com>
 <Yh4fGORDaJyVrJQW@iki.fi>
 <Yh4i4hVcnfZ8QDAl@iki.fi>
 <2d2d3471-78ce-9faa-daf6-138078f5ffaa@intel.com>
 <Yh7Q5fbOtr+6YWaS@iki.fi>
 <Yh7SOgqdFxJdVPMq@iki.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yh7SOgqdFxJdVPMq@iki.fi>
Precedence: bulk

On Wed, Mar 02, 2022 at 03:11:06AM +0100, Jarkko Sakkinen wrote:
> On Wed, Mar 02, 2022 at 03:05:25AM +0100, Jarkko Sakkinen wrote:
> > > The work that follows this series aimed to do the integration with user
> > > space policy.
> > 
> > What do you mean by "user space policy" anyway exactly? I'm sorry but I
> > just don't fully understand this.
> > 
> > It's too big of a risk to accept this series without X taken care of. Patch
> > series should neither have TODO nor TBD comments IMHO. I don't want to ack
> > a series based on speculation what might happen in the future.
> 
> If I accept this, then I'm kind of pre-acking code that I have no idea what
> it looks like, can it be acked, or am I doing the right thing for the
> kernel by acking this. 
> 
> It's unfortunately force majeure situation for me. I simply could not ack
> this, whether I want it or not.

I'd actually to leave out permission change madness completely out of this
patch set, as we all know it is a grazy beast of microarchitecture. For
user space having that is less critical than having executable pages.

Simply with EAUG/EACCEPTCOPY you can already populate enclave with any
permissions you had in mind. Augmenting alone would be logically consistent
patch set that is actually usable for many workloads.

Now there is half-broken augmenting (this is even writtend down to the TBD
comment) and complex code for EMODPR and EMODT that is usable only for
kselftests and not much else before there is fully working augmenting.

This way we get actually sound patch set that is easy to review and apply
to the mainline. It is also factors easier for you to iterate a smaller
set of patches.

After this it is so much easier to start to look at remaining functionality,
and at the same time augmenting part can be stress tested with real-world
code and it will mature quickly.

This whole thing *really* needs a serious U-turn on how it is delivered to
the upstream. Sometimes it is better just to admit that this didn't start
with the right foot.

BR, Jarkko