| Message ID | 1486487114-2785-2-git-send-email-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 07, 2017 at 09:05:14AM -0800, Sean Christopherson wrote: > Move the call to sgx_ewb() into sgx_evict_page(), and always perform > EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA > for the page cannot be found. Killing an enclave due to VMA closure > is a lazy operation, i.e. an enclave may contain active threads even > after a VMA is closed. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko > --- > drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index 72f8ef1..8224729 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, > static void sgx_evict_page(struct sgx_encl_page *entry, > struct sgx_encl *encl) > { > + sgx_ewb(encl, entry); > sgx_free_page(entry->epc_page, encl); > entry->epc_page = NULL; > entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > { > struct sgx_encl_page *entry; > struct sgx_encl_page *tmp; > - struct vm_area_struct *evma; > + struct vm_area_struct *vma; > > if (list_empty(src)) > return; > @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > > /* EBLOCK */ > list_for_each_entry_safe(entry, tmp, src, load_list) { > - evma = sgx_find_vma(encl, entry->addr); > - if (!evma) { > - list_del(&entry->load_list); > - sgx_evict_page(entry, encl); > - continue; > + vma = sgx_find_vma(encl, entry->addr); > + if (vma) { > + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); > } > > - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); > sgx_eblock(encl, entry->epc_page); > } > > @@ -350,20 +348,14 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > load_list); > list_del(&entry->load_list); > > - > - evma = sgx_find_vma(encl, entry->addr); > - if (evma) { > - sgx_ewb(encl, entry); > - encl->secs_child_cnt--; > - } > - > sgx_evict_page(entry, encl); > + > + encl->secs_child_cnt--; > } > > if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) { > - sgx_ewb(encl, &encl->secs_page); > - encl->flags |= SGX_ENCL_SECS_EVICTED; > sgx_evict_page(&encl->secs_page, encl); > + encl->flags |= SGX_ENCL_SECS_EVICTED; > } > > mutex_unlock(&encl->lock); > -- > 2.7.4 > > _______________________________________________ > intel-sgx-kernel-dev mailing list > intel-sgx-kernel-dev@lists.01.org > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
On Tue, Feb 07, 2017 at 09:05:14AM -0800, Sean Christopherson wrote: > Move the call to sgx_ewb() into sgx_evict_page(), and always perform > EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA > for the page cannot be found. Killing an enclave due to VMA closure > is a lazy operation, i.e. an enclave may contain active threads even > after a VMA is closed. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Doesn't apply. /Jarkko > --- > drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index 72f8ef1..8224729 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, > static void sgx_evict_page(struct sgx_encl_page *entry, > struct sgx_encl *encl) > { > + sgx_ewb(encl, entry); > sgx_free_page(entry->epc_page, encl); > entry->epc_page = NULL; > entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > { > struct sgx_encl_page *entry; > struct sgx_encl_page *tmp; > - struct vm_area_struct *evma; > + struct vm_area_struct *vma; > > if (list_empty(src)) > return; > @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > > /* EBLOCK */ > list_for_each_entry_safe(entry, tmp, src, load_list) { > - evma = sgx_find_vma(encl, entry->addr); > - if (!evma) { > - list_del(&entry->load_list); > - sgx_evict_page(entry, encl); > - continue; > + vma = sgx_find_vma(encl, entry->addr); > + if (vma) { > + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); > } > > - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); > sgx_eblock(encl, entry->epc_page); > } > > @@ -350,20 +348,14 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > load_list); > list_del(&entry->load_list); > > - > - evma = sgx_find_vma(encl, entry->addr); > - if (evma) { > - sgx_ewb(encl, entry); > - encl->secs_child_cnt--; > - } > - > sgx_evict_page(entry, encl); > + > + encl->secs_child_cnt--; > } > > if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) { > - sgx_ewb(encl, &encl->secs_page); > - encl->flags |= SGX_ENCL_SECS_EVICTED; > sgx_evict_page(&encl->secs_page, encl); > + encl->flags |= SGX_ENCL_SECS_EVICTED; > } > > mutex_unlock(&encl->lock); > -- > 2.7.4 > > _______________________________________________ > intel-sgx-kernel-dev mailing list > intel-sgx-kernel-dev@lists.01.org > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev
On Tue, Feb 07, 2017 at 09:05:14AM -0800, Sean Christopherson wrote: > Move the call to sgx_ewb() into sgx_evict_page(), and always perform > EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA > for the page cannot be found. Killing an enclave due to VMA closure > is a lazy operation, i.e. an enclave may contain active threads even > after a VMA is closed. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> I found an issue when I started to merge this one. > --- > drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index 72f8ef1..8224729 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, > static void sgx_evict_page(struct sgx_encl_page *entry, > struct sgx_encl *encl) > { > + sgx_ewb(encl, entry); > sgx_free_page(entry->epc_page, encl); > entry->epc_page = NULL; > entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > { > struct sgx_encl_page *entry; > struct sgx_encl_page *tmp; > - struct vm_area_struct *evma; > + struct vm_area_struct *vma; > > if (list_empty(src)) > return; > @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > > /* EBLOCK */ > list_for_each_entry_safe(entry, tmp, src, load_list) { > - evma = sgx_find_vma(encl, entry->addr); > - if (!evma) { > - list_del(&entry->load_list); > - sgx_evict_page(entry, encl); > - continue; > + vma = sgx_find_vma(encl, entry->addr); > + if (vma) { > + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); > } > > - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); We want the pages to be freed even if the VMA does not exist. /Jarkko
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > On Tue, Feb 07, 2017 at 09:05:14AM -0800, Sean Christopherson wrote: > > Move the call to sgx_ewb() into sgx_evict_page(), and always perform > > EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA > > for the page cannot be found. Killing an enclave due to VMA closure > > is a lazy operation, i.e. an enclave may contain active threads even > > after a VMA is closed. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > I found an issue when I started to merge this one. > > > --- > > drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- > > 1 file changed, 8 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c > > b/drivers/platform/x86/intel_sgx_page_cache.c index 72f8ef1..8224729 100644 > > --- a/drivers/platform/x86/intel_sgx_page_cache.c > > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > > @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, > > static void sgx_evict_page(struct sgx_encl_page *entry, > > struct sgx_encl *encl) > > { > > + sgx_ewb(encl, entry); > > sgx_free_page(entry->epc_page, encl); > > entry->epc_page = NULL; > > entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > > @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, > > struct list_head *src) { > > struct sgx_encl_page *entry; > > struct sgx_encl_page *tmp; > > - struct vm_area_struct *evma; > > + struct vm_area_struct *vma; > > > > if (list_empty(src)) > > return; > > @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, > > struct list_head *src) > > /* EBLOCK */ > > list_for_each_entry_safe(entry, tmp, src, load_list) { > > - evma = sgx_find_vma(encl, entry->addr); > > - if (!evma) { > > - list_del(&entry->load_list); > > - sgx_evict_page(entry, encl); > > - continue; > > + vma = sgx_find_vma(encl, entry->addr); > > + if (vma) { > > + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); > > } > > > > - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); > > We want the pages to be freed even if the VMA does not exist. > > /Jarkko Pages without a VMA are still freed, the only functional change in this patch is that pages without a VMA now go through EBLOCK and ETRACK, i.e. the entries are left in the list and later processed in the common EWB loop.
On Mon, Mar 13, 2017 at 06:07:20PM +0000, Christopherson, Sean J wrote: > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Tue, Feb 07, 2017 at 09:05:14AM -0800, Sean Christopherson wrote: > > > Move the call to sgx_ewb() into sgx_evict_page(), and always perform > > > EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA > > > for the page cannot be found. Killing an enclave due to VMA closure > > > is a lazy operation, i.e. an enclave may contain active threads even > > > after a VMA is closed. > > > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > > > I found an issue when I started to merge this one. > > > > > --- > > > drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- > > > 1 file changed, 8 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c > > > b/drivers/platform/x86/intel_sgx_page_cache.c index 72f8ef1..8224729 100644 > > > --- a/drivers/platform/x86/intel_sgx_page_cache.c > > > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > > > @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, > > > static void sgx_evict_page(struct sgx_encl_page *entry, > > > struct sgx_encl *encl) > > > { > > > + sgx_ewb(encl, entry); > > > sgx_free_page(entry->epc_page, encl); > > > entry->epc_page = NULL; > > > entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > > > @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, > > > struct list_head *src) { > > > struct sgx_encl_page *entry; > > > struct sgx_encl_page *tmp; > > > - struct vm_area_struct *evma; > > > + struct vm_area_struct *vma; > > > > > > if (list_empty(src)) > > > return; > > > @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, > > > struct list_head *src) > > > /* EBLOCK */ > > > list_for_each_entry_safe(entry, tmp, src, load_list) { > > > - evma = sgx_find_vma(encl, entry->addr); > > > - if (!evma) { > > > - list_del(&entry->load_list); > > > - sgx_evict_page(entry, encl); > > > - continue; > > > + vma = sgx_find_vma(encl, entry->addr); > > > + if (vma) { > > > + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); > > > } > > > > > > - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); > > > > We want the pages to be freed even if the VMA does not exist. > > > > /Jarkko > > Pages without a VMA are still freed, the only functional change in this patch > is that pages without a VMA now go through EBLOCK and ETRACK, i.e. the entries > are left in the list and later processed in the common EWB loop. Right, I see. The commit message was just very misleading (emphasizing clean up instead of this). I'll eventually merge this commit. Thank you. /Jarkko
On Wed, Mar 15, 2017 at 10:25:52AM +0200, Jarkko Sakkinen wrote: > On Mon, Mar 13, 2017 at 06:07:20PM +0000, Christopherson, Sean J wrote: > > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > > On Tue, Feb 07, 2017 at 09:05:14AM -0800, Sean Christopherson wrote: > > > > Move the call to sgx_ewb() into sgx_evict_page(), and always perform > > > > EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA > > > > for the page cannot be found. Killing an enclave due to VMA closure > > > > is a lazy operation, i.e. an enclave may contain active threads even > > > > after a VMA is closed. > > > > > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > > > > > I found an issue when I started to merge this one. > > > > > > > --- > > > > drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- > > > > 1 file changed, 8 insertions(+), 16 deletions(-) > > > > > > > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c > > > > b/drivers/platform/x86/intel_sgx_page_cache.c index 72f8ef1..8224729 100644 > > > > --- a/drivers/platform/x86/intel_sgx_page_cache.c > > > > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > > > > @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, > > > > static void sgx_evict_page(struct sgx_encl_page *entry, > > > > struct sgx_encl *encl) > > > > { > > > > + sgx_ewb(encl, entry); > > > > sgx_free_page(entry->epc_page, encl); > > > > entry->epc_page = NULL; > > > > entry->flags &= ~SGX_ENCL_PAGE_RESERVED; > > > > @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, > > > > struct list_head *src) { > > > > struct sgx_encl_page *entry; > > > > struct sgx_encl_page *tmp; > > > > - struct vm_area_struct *evma; > > > > + struct vm_area_struct *vma; > > > > > > > > if (list_empty(src)) > > > > return; > > > > @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, > > > > struct list_head *src) > > > > /* EBLOCK */ > > > > list_for_each_entry_safe(entry, tmp, src, load_list) { > > > > - evma = sgx_find_vma(encl, entry->addr); > > > > - if (!evma) { > > > > - list_del(&entry->load_list); > > > > - sgx_evict_page(entry, encl); > > > > - continue; > > > > + vma = sgx_find_vma(encl, entry->addr); > > > > + if (vma) { > > > > + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); > > > > } > > > > > > > > - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); > > > > > > We want the pages to be freed even if the VMA does not exist. > > > > > > /Jarkko > > > > Pages without a VMA are still freed, the only functional change in this patch > > is that pages without a VMA now go through EBLOCK and ETRACK, i.e. the entries > > are left in the list and later processed in the common EWB loop. > > Right, I see. > > The commit message was just very misleading (emphasizing clean up > instead of this). > > I'll eventually merge this commit. Thank you. > > /Jarkko It's now merged. Thank you. /Jarkko
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index 72f8ef1..8224729 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -310,6 +310,7 @@ static void sgx_ewb(struct sgx_encl *encl, static void sgx_evict_page(struct sgx_encl_page *entry, struct sgx_encl *encl) { + sgx_ewb(encl, entry); sgx_free_page(entry->epc_page, encl); entry->epc_page = NULL; entry->flags &= ~SGX_ENCL_PAGE_RESERVED; @@ -319,7 +320,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) { struct sgx_encl_page *entry; struct sgx_encl_page *tmp; - struct vm_area_struct *evma; + struct vm_area_struct *vma; if (list_empty(src)) return; @@ -330,14 +331,11 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) /* EBLOCK */ list_for_each_entry_safe(entry, tmp, src, load_list) { - evma = sgx_find_vma(encl, entry->addr); - if (!evma) { - list_del(&entry->load_list); - sgx_evict_page(entry, encl); - continue; + vma = sgx_find_vma(encl, entry->addr); + if (vma) { + zap_vma_ptes(vma, entry->addr, PAGE_SIZE); } - zap_vma_ptes(evma, entry->addr, PAGE_SIZE); sgx_eblock(encl, entry->epc_page); } @@ -350,20 +348,14 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) load_list); list_del(&entry->load_list); - - evma = sgx_find_vma(encl, entry->addr); - if (evma) { - sgx_ewb(encl, entry); - encl->secs_child_cnt--; - } - sgx_evict_page(entry, encl); + + encl->secs_child_cnt--; } if (!encl->secs_child_cnt && (encl->flags & SGX_ENCL_INITIALIZED)) { - sgx_ewb(encl, &encl->secs_page); - encl->flags |= SGX_ENCL_SECS_EVICTED; sgx_evict_page(&encl->secs_page, encl); + encl->flags |= SGX_ENCL_SECS_EVICTED; } mutex_unlock(&encl->lock);
Move the call to sgx_ewb() into sgx_evict_page(), and always perform EBLOCK->ETRACK-EWB for pages that are being evicted even if the VMA for the page cannot be found. Killing an enclave due to VMA closure is a lazy operation, i.e. an enclave may contain active threads even after a VMA is closed. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- drivers/platform/x86/intel_sgx_page_cache.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)