On Mon, Jul 23, 2018 at 12:45:17PM +0300, Kirill A. Shutemov wrote:

On Wed, Jul 18, 2018 at 04:38:02PM -0700, Dave Hansen wrote:

quoted

On 07/17/2018 04:20 AM, Kirill A. Shutemov wrote:

quoted

Store KeyID in bits 31:16 of extended page flags. These bits are unused.

I'd love a two sentence remind of what page_ext is and why you chose to
use it.  Yes, you need this.  No, not everybody that you want to review
this patch set knows what it is or why you chose it.

Okay.

quoted

quoted

page_keyid() returns zero until page_ext is ready.

Is there any implication of this?  Or does it not matter because we
don't run userspace until after page_ext initialization is done?

It matters in sense that we shouldn't reference page_ext before it's
initialized otherwise we will get garbage and crash.

quoted

quoted

page_ext initializer enables static branch to indicate that

			"enables a static branch"

quoted

page_keyid() can use page_ext. The same static branch will gate MKTME
readiness in general.

Can you elaborate on this a bit?  It would also be a nice place to hint
to the folks working hard on the APIs to ensure she checks this.

Okay.

At API init time we can check if (MKTME_ENABLED &&  mktme_nr_keyids > 0)
Sounds like this is another dependency we need to check and 'wait' on?
It happens after MKTME_ENABLED is set?  Let me know.

quoted

quoted

We don't yet set KeyID for the page. It will come in the following
patch that implements prep_encrypted_page(). All pages have KeyID-0 for
now.

It also wouldn't hurt to mention why you don't use an X86_FEATURE_* for
this rather than an explicit static branch.  I'm sure the x86
maintainers will be curious.

Sure.

-- 
 Kirill A. Shutemov