Re: [PATCH v6 06/11] intel_sgx: driver for Intel Software Guard Extensions

[Jarkko Sakkinen]

On Sun, Nov 26, 2017 at 08:46:27PM +0200, Jarkko Sakkinen wrote:
> On Sun, Nov 26, 2017 at 07:33:56PM +0200, Jarkko Sakkinen wrote:
> > > +struct sgx_encl_page {
> > > +	unsigned long addr;
> > > +	unsigned int flags;
> > > +	void *epc_page;
> > > +	struct sgx_va_page *va_page;
> > > +	unsigned int va_offset;
> > > +	struct list_head list;
> > > +};
> > 
> > This can be without major streches hoops packed way more. There are 12
> > bits free for use in the addr field. I can use low three bits for flags
> > and upper nine bits for va_offset.
> > 
> > I think I can also quite easily union epc_page and va_page fields.
> > 
> > With these changes the struct would shrink to:
> > 
> > struct sgx_encl_page {
> > 	unsigned long addr;
> > 	union {
> > 		void *epc_page;
> > 		struct sgx_va_page *va_page;
> > 	};
> > 	struct list_head list;
> > };
> 
> Once I started to thinking more about this I realized that this can be
> shrunk even further. I can drop the list field and use a tag for loaded
> pages in the radix tree where these are stored. The end result would be
> 
> struct sgx_encl_page {
> 	unsigned long addr;
>  	union {
>  		void *epc_page;
>  		struct sgx_va_page *va_page;
>  	};
> };

With this last size optimization there is an issue that it would be nice
to select the pages to be swapped out in FIFO order. That kind of means
that tag would not work quite well.

I could try an approach where I have a pointer array to present a queue
inside struct sgx_encl for loaded pages and in that way get rid of the
'list' field.

/Jarkko