Message ID | bd83f72d30ccfc7c1bc7ce9ab81bdf66e78a1d7d.1543903910.git.alison.schofield@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Multi-Key Total Memory Encryption API (MKTME) | expand |
On Mon, Dec 03, 2018 at 11:39:51PM -0800, Alison Schofield wrote: > +int mktme_map_keyid_from_key(void *key) > +{ > + int i; > + > + for (i = 1; i <= mktme_nr_keyids; i++) > + if (mktme_map->key[i] == key) > + return i; CodingStyle > + return 0; > +} > +int mktme_map_get_free_keyid(void) > +{ > + int i; > + > + if (mktme_map->mapped_keyids < mktme_nr_keyids) { > + for (i = 1; i <= mktme_nr_keyids; i++) > + if (mktme_map->key[i] == 0) > + return i; CodingStyle > + } > + return 0; > +}
> On Dec 3, 2018, at 11:39 PM, Alison Schofield <alison.schofield@intel.com> wrote: > > Define a global mapping structure to manage the mapping of userspace > Keys to hardware KeyIDs in MKTME (Multi-Key Total Memory Encryption). > Implement helper functions that access this mapping structure. > Why is a key “void *”? Who owns the memory? Can a real type be used?
On Tue, Dec 04, 2018 at 10:14:34AM +0100, Peter Zijlstra wrote: > On Mon, Dec 03, 2018 at 11:39:51PM -0800, Alison Schofield wrote: > > CodingStyle > CodingStyle > Thanks Peter. I'll repair all the badly nested if statements.
On Tue, Dec 04, 2018 at 07:35:50AM -0800, Andy Lutomirski wrote: > > > > On Dec 3, 2018, at 11:39 PM, Alison Schofield <alison.schofield@intel.com> wrote: > > > > Define a global mapping structure to manage the mapping of userspace > > Keys to hardware KeyIDs in MKTME (Multi-Key Total Memory Encryption). > > Implement helper functions that access this mapping structure. > > > > Why is a key “void *”? Who owns the memory? Can a real type be used? > It's of type "struct key" of the kernel key service. Replacing void w 'struct key'.
On Mon, 2018-12-03 at 23:39 -0800, Alison Schofield wrote: > +extern int mktme_map_alloc(void); > +extern void mktme_map_free(void); > +extern void mktme_map_lock(void); > +extern void mktme_map_unlock(void); > +extern int mktme_map_mapped_keyids(void); > +extern void mktme_map_set_keyid(int keyid, void *key); > +extern void mktme_map_free_keyid(int keyid); > +extern int mktme_map_keyid_from_key(void *key); > +extern void *mktme_map_key_from_keyid(int keyid); > +extern int mktme_map_get_free_keyid(void); No need for extern keyword for function declarations. It is only needed for variable declarations. > + > DECLARE_STATIC_KEY_FALSE(mktme_enabled_key); > static inline bool mktme_enabled(void) > { > diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c > index c81727540e7c..34224d4e3f45 100644 > --- a/arch/x86/mm/mktme.c > +++ b/arch/x86/mm/mktme.c > @@ -40,6 +40,97 @@ int __vma_keyid(struct vm_area_struct *vma) > return (prot & mktme_keyid_mask) >> mktme_keyid_shift; > } > > +/* > + * struct mktme_map and the mktme_map_* functions manage the mapping > + * of userspace Keys to hardware KeyIDs. These are used by the MKTME Key What are "userspace Keys" anyway and why Key and not key? > + * Service API and the encrypt_mprotect() system call. > + */ > + > +struct mktme_mapping { > + struct mutex lock; /* protect this map & HW state */ > + unsigned int mapped_keyids; > + void *key[]; > +}; Personally, I prefer not to align struct fields (I do align enums because there it makes more sense) as often you end up realigning everything. Documentation would bring more clarity. For example, what does key[] contain, why there is a lock and what mapped_keyids field contains? /Jarkko
diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h index f05baa15e6f6..dbb49909d665 100644 --- a/arch/x86/include/asm/mktme.h +++ b/arch/x86/include/asm/mktme.h @@ -12,6 +12,18 @@ extern phys_addr_t mktme_keyid_mask; extern int mktme_nr_keyids; extern int mktme_keyid_shift; +/* Manage mappings between hardware KeyIDs and userspace Keys */ +extern int mktme_map_alloc(void); +extern void mktme_map_free(void); +extern void mktme_map_lock(void); +extern void mktme_map_unlock(void); +extern int mktme_map_mapped_keyids(void); +extern void mktme_map_set_keyid(int keyid, void *key); +extern void mktme_map_free_keyid(int keyid); +extern int mktme_map_keyid_from_key(void *key); +extern void *mktme_map_key_from_keyid(int keyid); +extern int mktme_map_get_free_keyid(void); + DECLARE_STATIC_KEY_FALSE(mktme_enabled_key); static inline bool mktme_enabled(void) { diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c index c81727540e7c..34224d4e3f45 100644 --- a/arch/x86/mm/mktme.c +++ b/arch/x86/mm/mktme.c @@ -40,6 +40,97 @@ int __vma_keyid(struct vm_area_struct *vma) return (prot & mktme_keyid_mask) >> mktme_keyid_shift; } +/* + * struct mktme_map and the mktme_map_* functions manage the mapping + * of userspace Keys to hardware KeyIDs. These are used by the MKTME Key + * Service API and the encrypt_mprotect() system call. + */ + +struct mktme_mapping { + struct mutex lock; /* protect this map & HW state */ + unsigned int mapped_keyids; + void *key[]; +}; + +struct mktme_mapping *mktme_map; + +static inline long mktme_map_size(void) +{ + long size = 0; + + size += sizeof(*mktme_map); + size += sizeof(mktme_map->key[0]) * (mktme_nr_keyids + 1); + return size; +} + +int mktme_map_alloc(void) +{ + mktme_map = kvzalloc(mktme_map_size(), GFP_KERNEL); + if (!mktme_map) + return 0; + mutex_init(&mktme_map->lock); + return 1; +} + +void mktme_map_free(void) +{ + kvfree(mktme_map); +} + +void mktme_map_lock(void) +{ + mutex_lock(&mktme_map->lock); +} + +void mktme_map_unlock(void) +{ + mutex_unlock(&mktme_map->lock); +} + +int mktme_map_mapped_keyids(void) +{ + return mktme_map->mapped_keyids; +} + +void mktme_map_set_keyid(int keyid, void *key) +{ + mktme_map->key[keyid] = key; + mktme_map->mapped_keyids++; +} + +void mktme_map_free_keyid(int keyid) +{ + mktme_map->key[keyid] = 0; + mktme_map->mapped_keyids--; +} + +int mktme_map_keyid_from_key(void *key) +{ + int i; + + for (i = 1; i <= mktme_nr_keyids; i++) + if (mktme_map->key[i] == key) + return i; + return 0; +} + +void *mktme_map_key_from_keyid(int keyid) +{ + return mktme_map->key[keyid]; +} + +int mktme_map_get_free_keyid(void) +{ + int i; + + if (mktme_map->mapped_keyids < mktme_nr_keyids) { + for (i = 1; i <= mktme_nr_keyids; i++) + if (mktme_map->key[i] == 0) + return i; + } + return 0; +} + /* Prepare page to be used for encryption. Called from page allocator. */ void __prep_encrypted_page(struct page *page, int order, int keyid, bool zero) {