Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-24 Thread Jarkko Sakkinen
On Wed Apr 24, 2024 at 8:44 PM EEST, Jarkko Sakkinen wrote:
> On Wed Apr 24, 2024 at 2:50 PM EEST, Bojun Zhu wrote:
> > I still have some questions:
> >
> > It seems that the variable "ret" is set to 0 if there is **some** EPC pages 
> > have been 
> > added when interrupted by signal(Supposed that sgx_encl_add_page() 
> > always returns successfully).
>
> Ah, ok.
>
> Returning zero is right thing to do because it also returns count of
> pages successfully added. I.e. the function does not guarantee that
> all pages are processsed but it does guarantee that the system is in
> predictable state.
>
> It could be that e.g. sgx_alloc_epc_page() calls fails.
>
> So, it is a bit like how read system call works.

Good that you asked that had rethink for a while.

What I would suggest that you just put out 2nd verson out and then we
see where it is going.

For sgx_ioc_encl_add_pages() it is important to maintain exact semantics
as we need to maintain backwards compatibility.

BR, Jarkko



Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-24 Thread Jarkko Sakkinen
On Wed Apr 24, 2024 at 2:50 PM EEST, Bojun Zhu wrote:
> I still have some questions:
>
> It seems that the variable "ret" is set to 0 if there is **some** EPC pages 
> have been 
> added when interrupted by signal(Supposed that sgx_encl_add_page() 
> always returns successfully).

Ah, ok.

Returning zero is right thing to do because it also returns count of
pages successfully added. I.e. the function does not guarantee that
all pages are processsed but it does guarantee that the system is in
predictable state.

It could be that e.g. sgx_alloc_epc_page() calls fails.

So, it is a bit like how read system call works.

BR, Jarkko



Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-24 Thread Bojun Zhu
Hi Jarkko,

> On Apr 24, 2024, at 18:42, Jarkko Sakkinen  wrote:
> 
> On Wed Apr 24, 2024 at 10:02 AM EEST, Jarkko Sakkinen wrote:
>> On Wed Apr 24, 2024 at 9:46 AM EEST, Bojun Zhu wrote:
>>> Based on the the discussion among you, Jarkko and Reinette,
>>> I will keep the need_resched() and wrap the logic in using sgx_resched(),
>>> as suggested by Jarkko.
>> 
>> Sounds like a plan :-)
> 
> In sgx_ioc_enclave_add_pages() "if (!c)" check might cause possibly
> some  confusion.
> 
> Reason for it is that in "transaction sense" the operation can
> be only meaningfully restarted when no pages have not been added
> as MRENCLAVE checksum cannot be reset.
> 
> BR, Jarkko
> 


Thanks for your reminder.

According to the SGX hardware specification, similar to "ADD PAGES”,
the operations in  "MODT PAGEs” and  "REMOVE PAGEs(at runtime)”  
can be only meaningfully restarted when no page has been handled.

For the following code in sgx_ioc_enclave_add_pages():

```c
if (signal_pending(current)) {
if (!c)
ret = -ERESTARTSYS;

break;
}
```
I still have some questions:

It seems that the variable "ret" is set to 0 if there is **some** EPC pages 
have been 
added when interrupted by signal(Supposed that sgx_encl_add_page() 
always returns successfully).

Shall we set the return value as "-EINTR" if the context is interrupted by 
signal when some EPC pages have been added?
(BTW, return values contain -EINTR as shown in
https://elixir.bootlin.com/linux/latest/source/arch/x86/kernel/cpu/sgx/ioctl.c#L402)

Please correct me if misunderstood it.

Regards,
Bojun


Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-24 Thread Jarkko Sakkinen
On Wed Apr 24, 2024 at 10:02 AM EEST, Jarkko Sakkinen wrote:
> On Wed Apr 24, 2024 at 9:46 AM EEST, Bojun Zhu wrote:
> > Based on the the discussion among you, Jarkko and Reinette,
> > I will keep the need_resched() and wrap the logic in using sgx_resched(),
> > as suggested by Jarkko.
>
> Sounds like a plan :-)

In sgx_ioc_enclave_add_pages() "if (!c)" check might cause possibly
some  confusion.

Reason for it is that in "transaction sense" the operation can
be only meaningfully restarted when no pages have not been added
as MRENCLAVE checksum cannot be reset.

BR, Jarkko



Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-24 Thread Jarkko Sakkinen
On Wed Apr 24, 2024 at 9:46 AM EEST, Bojun Zhu wrote:
> Based on the the discussion among you, Jarkko and Reinette,
> I will keep the need_resched() and wrap the logic in using sgx_resched(),
> as suggested by Jarkko.

Sounds like a plan :-)

> Regards,
>
> Bojun

BR, Jarkko



Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-24 Thread Bojun Zhu

Hi Kai,

On 2024/4/23 19:50, Huang, Kai wrote:

On Tue, 2024-04-23 at 17:25 +0800, 朱伯君(杰铭) wrote:

EDMM's ioctl()s support batch operations, which may be
time-consuming. Try to explicitly give up the CPU at
the every end of "for loop" in
sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
to give other tasks a chance to run, and avoid softlockup warning.

The following has been observed on Linux v6.9-rc5 with kernel
preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
is requested to restrict page permissions of a large number of EPC pages.

 [ cut here ]
 watchdog: BUG: soft lockup - CPU#45 stuck for 22s! [occlum-run:3905]
 ...
 CPU: 45 PID: 3905 Comm: occlum-run Not tainted 6.9.0-rc5 #7
 ...
 RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
 Code: 48 c1 e6 05 48 89 d1 48 8d 5c 24 40 b8 0e 00 00 00 48 2b 8e 70 8e f5 93 48 
c1 e9 05 48 c1 e1 0c 48 03 8e 68 8e f5 93 0f 01 cf  00 00 00 40 0f 85 b2 00 
00 00 85 c0 0f 85 db 00 00 00 4c 89 ef
 RSP: 0018:b55a6591fa80 EFLAGS: 0202
 RAX:  RBX: b55a6591fac0 RCX: b581e7384000
 RDX: b59a9e4e8080 RSI: 0020 RDI: 91d69e8cc000
 RBP: b55a6591fb70 R08: 0002 R09: 91d646e12be0
 R10: 006e R11: 0002 R12: 00072052d000
 R13: 91d69e8cc000 R14: b55a6591fbd8 R15: 91d69e8cc020
 FS:  7fe10dbda740() GS:92163e48() 
knlGS:
 CS:  0010 DS:  ES:  CR0: 80050033
 CR2: 7fc041811000 CR3: 0040d95c8005 CR4: 00770ef0
 DR0:  DR1:  DR2: 
 DR3:  DR6: fffe07f0 DR7: 0400
 PKRU: 5554
 Call Trace:
  
  ? show_regs+0x67/0x70
  ? watchdog_timer_fn+0x1f3/0x280
  ? __pfx_watchdog_timer_fn+0x10/0x10
  ? __hrtimer_run_queues+0xc8/0x220
  ? hrtimer_interrupt+0x10c/0x250
  ? __sysvec_apic_timer_interrupt+0x53/0x130
  ? sysvec_apic_timer_interrupt+0x7b/0x90
  
  
  ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
  ? sgx_enclave_restrict_permissions+0xba/0x1f0
  ? __pte_offset_map_lock+0x94/0x110
  ? sgx_encl_test_and_clear_young_cb+0x40/0x60
  sgx_ioctl+0x1ab/0x900
  ? do_syscall_64+0x79/0x110
  ? apply_to_page_range+0x14/0x20
  ? sgx_encl_test_and_clear_young+0x6c/0x80
  ? sgx_vma_fault+0x132/0x4f0
  __x64_sys_ioctl+0x95/0xd0
  x64_sys_call+0x1209/0x20c0
  do_syscall_64+0x6d/0x110
  ? do_syscall_64+0x79/0x110
  ? do_pte_missing+0x2e8/0xcc0
  ? __pte_offset_map+0x1c/0x190
  ? __handle_mm_fault+0x7b9/0xe60
  ? __count_memcg_events+0x70/0x100
  ? handle_mm_fault+0x256/0x360
  ? do_user_addr_fault+0x3c1/0x860
  ? irqentry_exit_to_user_mode+0x67/0x190
  ? irqentry_exit+0x3b/0x50
  ? exc_page_fault+0x89/0x180
  entry_SYSCALL_64_after_hwframe+0x76/0x7e
 RIP: 0033:0x7fe10e2ee5cb
 Code: 0f 1e fa 48 8b 05 c5 78 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 
66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 
48 8b 0d 95 78 0d 00 f7 d8 64 89 01 48
 RSP: 002b:7fffb2c75518 EFLAGS: 0246 ORIG_RAX: 0010
 RAX: ffda RBX: 00078000 RCX: 7fe10e2ee5cb
 RDX: 7fffb2c75520 RSI: c028a405 RDI: 0005
 RBP: 0005 R08:  R09: 7fffb2c75594
 R10: 7fffb2c755c8 R11: 0246 R12: c028a405
 R13: 7fffb2c75520 R14: 00078000 R15: 7fe10e1a7980
  
  [ end trace ]

Could you trim down the trace to only include the relevant part?

E.g., please at least remove the two register dumps at the beginning and
end of the trace.

Please refer to "Backtraces in commit messages" section in
Documentation/process/submitting-patches.rst.



Thanks for your advice! I will refine the trace log according to the 
suggestions
in Documentation/process/submitting-patches.rst. and highlight the 
related part.




Signed-off-by: Bojun Zhu 
---
  arch/x86/kernel/cpu/sgx/ioctl.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index b65ab214bdf5..2340a82fa796 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
}
  
  		mutex_unlock(>lock);

+
+   if (need_resched())
+   cond_resched();
}
  
  	ret = 0;

@@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
*encl,
entry->type = page_type;
  
  		mutex_unlock(>lock);

+
+   if (need_resched())
+   cond_resched();
}
  
  	ret = 0;

@@ -1156,6 +1162,9 @@ static long 

Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Huang, Kai
On Wed, 2024-04-24 at 00:27 +0300, Jarkko Sakkinen wrote:
> On Tue Apr 23, 2024 at 8:08 PM EEST, Reinette Chatre wrote:
> > Hi Kai,
> > 
> > On 4/23/2024 4:50 AM, Huang, Kai wrote:
> > > > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c 
> > > > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > > > index b65ab214bdf5..2340a82fa796 100644
> > > > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > > > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > > > @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl 
> > > > *encl,
> > > > }
> > > >  
> > > > mutex_unlock(>lock);
> > > > +
> > > > +   if (need_resched())
> > > > +   cond_resched();
> > > > }
> > > >  
> > > > ret = 0;
> > > > @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct 
> > > > sgx_encl *encl,
> > > > entry->type = page_type;
> > > >  
> > > > mutex_unlock(>lock);
> > > > +
> > > > +   if (need_resched())
> > > > +   cond_resched();
> > > > }
> > > >  
> > > > ret = 0;
> > > > @@ -1156,6 +1162,9 @@ static long sgx_encl_remove_pages(struct sgx_encl 
> > > > *encl,
> > > > kfree(entry);
> > > >  
> > > > mutex_unlock(>lock);
> > > > +
> > > > +   if (need_resched())
> > > > +   cond_resched();
> > > > }
> > > > 
> > > 
> > > You can remove the need_reshced() in all 3 places above but just call
> > > cond_resched() directly.
> > > 
> > 
> > This change will call cond_resched() after dealing with each page in a
> > potentially large page range (cover mentions 30GB but we have also had to
> > make optimizations for enclaves larger than this). Adding a cond_resched()
> > here will surely placate the soft lockup detector, but we need to take care
> > how changes like this impact the performance of the system and having 
> > actions
> > on these page ranges take much longer than necessary.
> > For reference, please see 7b72c823ddf8 ("x86/sgx: Reduce delay and 
> > interference
> > of enclave release") that turned frequent cond_resched() into batches
> > to address performance issues.

Ah I didn't know this.  Thanks for the info.

> > 
> > It looks to me like the need_resched() may be a quick check that can be used
> > to improve performance? 
> > 

Perhaps?  My assumption is eventually cond_resched() will do similar check
of need_resched() but I am not entirely sure about of that.

Reading the code, it seems cond_resched() eventually does
should_resched().  The generic version indeed does similar check of
need_resched() but it seems the x86 version has a different
implementation.

> > I am not familiar with all use cases that need to be
> > considered to determine if a batching solution may be needed.

Looks at least the REMOVE_PAGES ioctls() could have the same impact to the
performance downgrade problem mentioned in commit 7b72c823ddf8 ("x86/sgx:
Reduce delay and interference of enclave release"), but I guess it's
acceptable to fix softlockup first and then improve performance if there's
someone hit any real issue. 

> 
> Ya, well no matter it is the reasoning will need to be documented
> because this should have symmetry with sgx_ioc_enclave_add_pages()
> (see my response to Kai).

Yeah I was actually going to mention this, but somehow I didn't choose to.

> 
> I because this makes dealing with need_resched() a change in code
> even if it is left out as a side-effect, I'd support of not removing
> which means adding need_resched() as a side-effect.

I am fine with keeping the need_resched().


Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Jarkko Sakkinen
On Tue Apr 23, 2024 at 8:08 PM EEST, Reinette Chatre wrote:
> Hi Kai,
>
> On 4/23/2024 4:50 AM, Huang, Kai wrote:
> >> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c 
> >> b/arch/x86/kernel/cpu/sgx/ioctl.c
> >> index b65ab214bdf5..2340a82fa796 100644
> >> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> >> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> >> @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
> >>}
> >>  
> >>mutex_unlock(>lock);
> >> +
> >> +  if (need_resched())
> >> +  cond_resched();
> >>}
> >>  
> >>ret = 0;
> >> @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
> >> *encl,
> >>entry->type = page_type;
> >>  
> >>mutex_unlock(>lock);
> >> +
> >> +  if (need_resched())
> >> +  cond_resched();
> >>}
> >>  
> >>ret = 0;
> >> @@ -1156,6 +1162,9 @@ static long sgx_encl_remove_pages(struct sgx_encl 
> >> *encl,
> >>kfree(entry);
> >>  
> >>mutex_unlock(>lock);
> >> +
> >> +  if (need_resched())
> >> +  cond_resched();
> >>}
> >>
> > 
> > You can remove the need_reshced() in all 3 places above but just call
> > cond_resched() directly.
> > 
>
> This change will call cond_resched() after dealing with each page in a
> potentially large page range (cover mentions 30GB but we have also had to
> make optimizations for enclaves larger than this). Adding a cond_resched()
> here will surely placate the soft lockup detector, but we need to take care
> how changes like this impact the performance of the system and having actions
> on these page ranges take much longer than necessary.
> For reference, please see 7b72c823ddf8 ("x86/sgx: Reduce delay and 
> interference
> of enclave release") that turned frequent cond_resched() into batches
> to address performance issues.
>
> It looks to me like the need_resched() may be a quick check that can be used
> to improve performance? I am not familiar with all use cases that need to be
> considered to determine if a batching solution may be needed.

Ya, well no matter it is the reasoning will need to be documented
because this should have symmetry with sgx_ioc_enclave_add_pages()
(see my response to Kai).

I because this makes dealing with need_resched() a change in code
even if it is left out as a side-effect, I'd support of not removing
which means adding need_resched() as a side-effect.

>From this follows that *if* need_resched() needs to be removed then
that is not really part of the bug fix, so in all cases the bug fix
itself must include need_resched() :-)

phew, hope you got my logic here, i think it reasonable...

BR, Jarkko



Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Jarkko Sakkinen
On Tue Apr 23, 2024 at 2:50 PM EEST, Huang, Kai wrote:
> On Tue, 2024-04-23 at 17:25 +0800, 朱伯君(杰铭) wrote:
> > EDMM's ioctl()s support batch operations, which may be
> > time-consuming. Try to explicitly give up the CPU at
> > the every end of "for loop" in
> > sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> > to give other tasks a chance to run, and avoid softlockup warning.
> > 
> > The following has been observed on Linux v6.9-rc5 with kernel
> > preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> > is requested to restrict page permissions of a large number of EPC pages.
> > 
> > [ cut here ]
> > watchdog: BUG: soft lockup - CPU#45 stuck for 22s! [occlum-run:3905]
> > ...
> > CPU: 45 PID: 3905 Comm: occlum-run Not tainted 6.9.0-rc5 #7
> > ...
> > RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> > Code: 48 c1 e6 05 48 89 d1 48 8d 5c 24 40 b8 0e 00 00 00 48 2b 8e 70 8e 
> > f5 93 48 c1 e9 05 48 c1 e1 0c 48 03 8e 68 8e f5 93 0f 01 cf  00 00 00 
> > 40 0f 85 b2 00 00 00 85 c0 0f 85 db 00 00 00 4c 89 ef
> > RSP: 0018:b55a6591fa80 EFLAGS: 0202
> > RAX:  RBX: b55a6591fac0 RCX: b581e7384000
> > RDX: b59a9e4e8080 RSI: 0020 RDI: 91d69e8cc000
> > RBP: b55a6591fb70 R08: 0002 R09: 91d646e12be0
> > R10: 006e R11: 0002 R12: 00072052d000
> > R13: 91d69e8cc000 R14: b55a6591fbd8 R15: 91d69e8cc020
> > FS:  7fe10dbda740() GS:92163e48() 
> > knlGS:
> > CS:  0010 DS:  ES:  CR0: 80050033
> > CR2: 7fc041811000 CR3: 0040d95c8005 CR4: 00770ef0
> > DR0:  DR1:  DR2: 
> > DR3:  DR6: fffe07f0 DR7: 0400
> > PKRU: 5554
> > Call Trace:
> >  
> >  ? show_regs+0x67/0x70
> >  ? watchdog_timer_fn+0x1f3/0x280
> >  ? __pfx_watchdog_timer_fn+0x10/0x10
> >  ? __hrtimer_run_queues+0xc8/0x220
> >  ? hrtimer_interrupt+0x10c/0x250
> >  ? __sysvec_apic_timer_interrupt+0x53/0x130
> >  ? sysvec_apic_timer_interrupt+0x7b/0x90
> >  
> >  
> >  ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
> >  ? sgx_enclave_restrict_permissions+0xba/0x1f0
> >  ? __pte_offset_map_lock+0x94/0x110
> >  ? sgx_encl_test_and_clear_young_cb+0x40/0x60
> >  sgx_ioctl+0x1ab/0x900
> >  ? do_syscall_64+0x79/0x110
> >  ? apply_to_page_range+0x14/0x20
> >  ? sgx_encl_test_and_clear_young+0x6c/0x80
> >  ? sgx_vma_fault+0x132/0x4f0
> >  __x64_sys_ioctl+0x95/0xd0
> >  x64_sys_call+0x1209/0x20c0
> >  do_syscall_64+0x6d/0x110
> >  ? do_syscall_64+0x79/0x110
> >  ? do_pte_missing+0x2e8/0xcc0
> >  ? __pte_offset_map+0x1c/0x190
> >  ? __handle_mm_fault+0x7b9/0xe60
> >  ? __count_memcg_events+0x70/0x100
> >  ? handle_mm_fault+0x256/0x360
> >  ? do_user_addr_fault+0x3c1/0x860
> >  ? irqentry_exit_to_user_mode+0x67/0x190
> >  ? irqentry_exit+0x3b/0x50
> >  ? exc_page_fault+0x89/0x180
> >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > RIP: 0033:0x7fe10e2ee5cb
> > Code: 0f 1e fa 48 8b 05 c5 78 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff 
> > ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 
> > ff ff 73 01 c3 48 8b 0d 95 78 0d 00 f7 d8 64 89 01 48
> > RSP: 002b:7fffb2c75518 EFLAGS: 0246 ORIG_RAX: 0010
> > RAX: ffda RBX: 00078000 RCX: 7fe10e2ee5cb
> > RDX: 7fffb2c75520 RSI: c028a405 RDI: 0005
> > RBP: 0005 R08:  R09: 7fffb2c75594
> > R10: 7fffb2c755c8 R11: 0246 R12: c028a405
> > R13: 7fffb2c75520 R14: 00078000 R15: 7fe10e1a7980
> >  
> >  [ end trace ]
>
> Could you trim down the trace to only include the relevant part?
>
> E.g., please at least remove the two register dumps at the beginning and
> end of the trace.
>
> Please refer to "Backtraces in commit messages" section in
> Documentation/process/submitting-patches.rst.
>
> > 
> > Signed-off-by: Bojun Zhu 
> > ---
> >  arch/x86/kernel/cpu/sgx/ioctl.c | 9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c 
> > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > index b65ab214bdf5..2340a82fa796 100644
> > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
> > }
> >  
> > mutex_unlock(>lock);
> > +
> > +   if (need_resched())
> > +   cond_resched();
> > }
> >  
> > ret = 0;
> > @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
> > *encl,
> > 

Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Jarkko Sakkinen
On Wed Apr 24, 2024 at 12:10 AM EEST, Jarkko Sakkinen wrote:
> On Tue Apr 23, 2024 at 12:25 PM EEST, =?UTF-8?B?5pyx5Lyv5ZCbKOadsOmTrSk=?= 
> wrote:
> > EDMM's ioctl()s support batch operations, which may be
> > time-consuming. Try to explicitly give up the CPU at
> > the every end of "for loop" in
> > sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> > to give other tasks a chance to run, and avoid softlockup warning.
> >
> > The following has been observed on Linux v6.9-rc5 with kernel
> > preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> > is requested to restrict page permissions of a large number of EPC pages.
> >
> > [ cut here ]
> > watchdog: BUG: soft lockup - CPU#45 stuck for 22s! [occlum-run:3905]
> > ...
> > CPU: 45 PID: 3905 Comm: occlum-run Not tainted 6.9.0-rc5 #7
> > ...
> > RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> > Code: 48 c1 e6 05 48 89 d1 48 8d 5c 24 40 b8 0e 00 00 00 48 2b 8e 70 8e 
> > f5 93 48 c1 e9 05 48 c1 e1 0c 48 03 8e 68 8e f5 93 0f 01 cf  00 00 00 
> > 40 0f 85 b2 00 00 00 85 c0 0f 85 db 00 00 00 4c 89 ef
> > RSP: 0018:b55a6591fa80 EFLAGS: 0202
> > RAX:  RBX: b55a6591fac0 RCX: b581e7384000
> > RDX: b59a9e4e8080 RSI: 0020 RDI: 91d69e8cc000
> > RBP: b55a6591fb70 R08: 0002 R09: 91d646e12be0
> > R10: 006e R11: 0002 R12: 00072052d000
> > R13: 91d69e8cc000 R14: b55a6591fbd8 R15: 91d69e8cc020
> > FS:  7fe10dbda740() GS:92163e48() 
> > knlGS:
> > CS:  0010 DS:  ES:  CR0: 80050033
> > CR2: 7fc041811000 CR3: 0040d95c8005 CR4: 00770ef0
> > DR0:  DR1:  DR2: 
> > DR3:  DR6: fffe07f0 DR7: 0400
> > PKRU: 5554
> > Call Trace:
> >  
> >  ? show_regs+0x67/0x70
> >  ? watchdog_timer_fn+0x1f3/0x280
> >  ? __pfx_watchdog_timer_fn+0x10/0x10
> >  ? __hrtimer_run_queues+0xc8/0x220
> >  ? hrtimer_interrupt+0x10c/0x250
> >  ? __sysvec_apic_timer_interrupt+0x53/0x130
> >  ? sysvec_apic_timer_interrupt+0x7b/0x90
> >  
> >  
> >  ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
> >  ? sgx_enclave_restrict_permissions+0xba/0x1f0
> >  ? __pte_offset_map_lock+0x94/0x110
> >  ? sgx_encl_test_and_clear_young_cb+0x40/0x60
> >  sgx_ioctl+0x1ab/0x900
> >  ? do_syscall_64+0x79/0x110
> >  ? apply_to_page_range+0x14/0x20
> >  ? sgx_encl_test_and_clear_young+0x6c/0x80
> >  ? sgx_vma_fault+0x132/0x4f0
> >  __x64_sys_ioctl+0x95/0xd0
> >  x64_sys_call+0x1209/0x20c0
> >  do_syscall_64+0x6d/0x110
> >  ? do_syscall_64+0x79/0x110
> >  ? do_pte_missing+0x2e8/0xcc0
> >  ? __pte_offset_map+0x1c/0x190
> >  ? __handle_mm_fault+0x7b9/0xe60
> >  ? __count_memcg_events+0x70/0x100
> >  ? handle_mm_fault+0x256/0x360
> >  ? do_user_addr_fault+0x3c1/0x860
> >  ? irqentry_exit_to_user_mode+0x67/0x190
> >  ? irqentry_exit+0x3b/0x50
> >  ? exc_page_fault+0x89/0x180
> >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > RIP: 0033:0x7fe10e2ee5cb
> > Code: 0f 1e fa 48 8b 05 c5 78 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff 
> > ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 
> > ff ff 73 01 c3 48 8b 0d 95 78 0d 00 f7 d8 64 89 01 48
> > RSP: 002b:7fffb2c75518 EFLAGS: 0246 ORIG_RAX: 0010
> > RAX: ffda RBX: 00078000 RCX: 7fe10e2ee5cb
> > RDX: 7fffb2c75520 RSI: c028a405 RDI: 0005
> > RBP: 0005 R08:  R09: 7fffb2c75594
> > R10: 7fffb2c755c8 R11: 0246 R12: c028a405
> > R13: 7fffb2c75520 R14: 00078000 R15: 7fe10e1a7980
> >  
> >  [ end trace ]
> >
> > Signed-off-by: Bojun Zhu 
>
> Can you also fixup this as your "firstname lastname" in your emails
> from field? This matters so that author field in git log matches your
> sob.
>
> > ---
> >  arch/x86/kernel/cpu/sgx/ioctl.c | 9 +
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c 
> > b/arch/x86/kernel/cpu/sgx/ioctl.c
> > index b65ab214bdf5..2340a82fa796 100644
> > --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> > +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> > @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
> > }
> >  
> > mutex_unlock(>lock);
> > +
> > +   if (need_resched())
> > +   cond_resched();
> > }
> >  
> > ret = 0;
> > @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
> > *encl,
> > entry->type = page_type;
> >  
> > mutex_unlock(>lock);
> > +
> > +   if 

Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Jarkko Sakkinen
On Tue Apr 23, 2024 at 12:25 PM EEST, =?UTF-8?B?5pyx5Lyv5ZCbKOadsOmTrSk=?= 
wrote:
> EDMM's ioctl()s support batch operations, which may be
> time-consuming. Try to explicitly give up the CPU at
> the every end of "for loop" in
> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> to give other tasks a chance to run, and avoid softlockup warning.
>
> The following has been observed on Linux v6.9-rc5 with kernel
> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> is requested to restrict page permissions of a large number of EPC pages.
>
> [ cut here ]
> watchdog: BUG: soft lockup - CPU#45 stuck for 22s! [occlum-run:3905]
> ...
> CPU: 45 PID: 3905 Comm: occlum-run Not tainted 6.9.0-rc5 #7
> ...
> RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> Code: 48 c1 e6 05 48 89 d1 48 8d 5c 24 40 b8 0e 00 00 00 48 2b 8e 70 8e 
> f5 93 48 c1 e9 05 48 c1 e1 0c 48 03 8e 68 8e f5 93 0f 01 cf  00 00 00 40 
> 0f 85 b2 00 00 00 85 c0 0f 85 db 00 00 00 4c 89 ef
> RSP: 0018:b55a6591fa80 EFLAGS: 0202
> RAX:  RBX: b55a6591fac0 RCX: b581e7384000
> RDX: b59a9e4e8080 RSI: 0020 RDI: 91d69e8cc000
> RBP: b55a6591fb70 R08: 0002 R09: 91d646e12be0
> R10: 006e R11: 0002 R12: 00072052d000
> R13: 91d69e8cc000 R14: b55a6591fbd8 R15: 91d69e8cc020
> FS:  7fe10dbda740() GS:92163e48() 
> knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 7fc041811000 CR3: 0040d95c8005 CR4: 00770ef0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe07f0 DR7: 0400
> PKRU: 5554
> Call Trace:
>  
>  ? show_regs+0x67/0x70
>  ? watchdog_timer_fn+0x1f3/0x280
>  ? __pfx_watchdog_timer_fn+0x10/0x10
>  ? __hrtimer_run_queues+0xc8/0x220
>  ? hrtimer_interrupt+0x10c/0x250
>  ? __sysvec_apic_timer_interrupt+0x53/0x130
>  ? sysvec_apic_timer_interrupt+0x7b/0x90
>  
>  
>  ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
>  ? sgx_enclave_restrict_permissions+0xba/0x1f0
>  ? __pte_offset_map_lock+0x94/0x110
>  ? sgx_encl_test_and_clear_young_cb+0x40/0x60
>  sgx_ioctl+0x1ab/0x900
>  ? do_syscall_64+0x79/0x110
>  ? apply_to_page_range+0x14/0x20
>  ? sgx_encl_test_and_clear_young+0x6c/0x80
>  ? sgx_vma_fault+0x132/0x4f0
>  __x64_sys_ioctl+0x95/0xd0
>  x64_sys_call+0x1209/0x20c0
>  do_syscall_64+0x6d/0x110
>  ? do_syscall_64+0x79/0x110
>  ? do_pte_missing+0x2e8/0xcc0
>  ? __pte_offset_map+0x1c/0x190
>  ? __handle_mm_fault+0x7b9/0xe60
>  ? __count_memcg_events+0x70/0x100
>  ? handle_mm_fault+0x256/0x360
>  ? do_user_addr_fault+0x3c1/0x860
>  ? irqentry_exit_to_user_mode+0x67/0x190
>  ? irqentry_exit+0x3b/0x50
>  ? exc_page_fault+0x89/0x180
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7fe10e2ee5cb
> Code: 0f 1e fa 48 8b 05 c5 78 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff 
> ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff 
> ff 73 01 c3 48 8b 0d 95 78 0d 00 f7 d8 64 89 01 48
> RSP: 002b:7fffb2c75518 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 00078000 RCX: 7fe10e2ee5cb
> RDX: 7fffb2c75520 RSI: c028a405 RDI: 0005
> RBP: 0005 R08:  R09: 7fffb2c75594
> R10: 7fffb2c755c8 R11: 0246 R12: c028a405
> R13: 7fffb2c75520 R14: 00078000 R15: 7fe10e1a7980
>  
>  [ end trace ]
>
> Signed-off-by: Bojun Zhu 

Can you also fixup this as your "firstname lastname" in your emails
from field? This matters so that author field in git log matches your
sob.

> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index b65ab214bdf5..2340a82fa796 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
>   }
>  
>   mutex_unlock(>lock);
> +
> + if (need_resched())
> + cond_resched();
>   }
>  
>   ret = 0;
> @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
> *encl,
>   entry->type = page_type;
>  
>   mutex_unlock(>lock);
> +
> + if (need_resched())
> + cond_resched();
>   }
>  
>   ret = 0;
> @@ -1156,6 +1162,9 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl,
>   kfree(entry);
>  
>   mutex_unlock(>lock);
> +
> + if 

Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Reinette Chatre
Hi Kai,

On 4/23/2024 4:50 AM, Huang, Kai wrote:
>> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c 
>> b/arch/x86/kernel/cpu/sgx/ioctl.c
>> index b65ab214bdf5..2340a82fa796 100644
>> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
>> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
>> @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
>>  }
>>  
>>  mutex_unlock(>lock);
>> +
>> +if (need_resched())
>> +cond_resched();
>>  }
>>  
>>  ret = 0;
>> @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
>> *encl,
>>  entry->type = page_type;
>>  
>>  mutex_unlock(>lock);
>> +
>> +if (need_resched())
>> +cond_resched();
>>  }
>>  
>>  ret = 0;
>> @@ -1156,6 +1162,9 @@ static long sgx_encl_remove_pages(struct sgx_encl 
>> *encl,
>>  kfree(entry);
>>  
>>  mutex_unlock(>lock);
>> +
>> +if (need_resched())
>> +cond_resched();
>>  }
>>
> 
> You can remove the need_reshced() in all 3 places above but just call
> cond_resched() directly.
> 

This change will call cond_resched() after dealing with each page in a
potentially large page range (cover mentions 30GB but we have also had to
make optimizations for enclaves larger than this). Adding a cond_resched()
here will surely placate the soft lockup detector, but we need to take care
how changes like this impact the performance of the system and having actions
on these page ranges take much longer than necessary.
For reference, please see 7b72c823ddf8 ("x86/sgx: Reduce delay and interference
of enclave release") that turned frequent cond_resched() into batches
to address performance issues.

It looks to me like the need_resched() may be a quick check that can be used
to improve performance? I am not familiar with all use cases that need to be
considered to determine if a batching solution may be needed.

Reinette



Re: [RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread Huang, Kai
On Tue, 2024-04-23 at 17:25 +0800, 朱伯君(杰铭) wrote:
> EDMM's ioctl()s support batch operations, which may be
> time-consuming. Try to explicitly give up the CPU at
> the every end of "for loop" in
> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> to give other tasks a chance to run, and avoid softlockup warning.
> 
> The following has been observed on Linux v6.9-rc5 with kernel
> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> is requested to restrict page permissions of a large number of EPC pages.
> 
> [ cut here ]
> watchdog: BUG: soft lockup - CPU#45 stuck for 22s! [occlum-run:3905]
> ...
> CPU: 45 PID: 3905 Comm: occlum-run Not tainted 6.9.0-rc5 #7
> ...
> RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> Code: 48 c1 e6 05 48 89 d1 48 8d 5c 24 40 b8 0e 00 00 00 48 2b 8e 70 8e 
> f5 93 48 c1 e9 05 48 c1 e1 0c 48 03 8e 68 8e f5 93 0f 01 cf  00 00 00 40 
> 0f 85 b2 00 00 00 85 c0 0f 85 db 00 00 00 4c 89 ef
> RSP: 0018:b55a6591fa80 EFLAGS: 0202
> RAX:  RBX: b55a6591fac0 RCX: b581e7384000
> RDX: b59a9e4e8080 RSI: 0020 RDI: 91d69e8cc000
> RBP: b55a6591fb70 R08: 0002 R09: 91d646e12be0
> R10: 006e R11: 0002 R12: 00072052d000
> R13: 91d69e8cc000 R14: b55a6591fbd8 R15: 91d69e8cc020
> FS:  7fe10dbda740() GS:92163e48() 
> knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 7fc041811000 CR3: 0040d95c8005 CR4: 00770ef0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe07f0 DR7: 0400
> PKRU: 5554
> Call Trace:
>  
>  ? show_regs+0x67/0x70
>  ? watchdog_timer_fn+0x1f3/0x280
>  ? __pfx_watchdog_timer_fn+0x10/0x10
>  ? __hrtimer_run_queues+0xc8/0x220
>  ? hrtimer_interrupt+0x10c/0x250
>  ? __sysvec_apic_timer_interrupt+0x53/0x130
>  ? sysvec_apic_timer_interrupt+0x7b/0x90
>  
>  
>  ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
>  ? sgx_enclave_restrict_permissions+0xba/0x1f0
>  ? __pte_offset_map_lock+0x94/0x110
>  ? sgx_encl_test_and_clear_young_cb+0x40/0x60
>  sgx_ioctl+0x1ab/0x900
>  ? do_syscall_64+0x79/0x110
>  ? apply_to_page_range+0x14/0x20
>  ? sgx_encl_test_and_clear_young+0x6c/0x80
>  ? sgx_vma_fault+0x132/0x4f0
>  __x64_sys_ioctl+0x95/0xd0
>  x64_sys_call+0x1209/0x20c0
>  do_syscall_64+0x6d/0x110
>  ? do_syscall_64+0x79/0x110
>  ? do_pte_missing+0x2e8/0xcc0
>  ? __pte_offset_map+0x1c/0x190
>  ? __handle_mm_fault+0x7b9/0xe60
>  ? __count_memcg_events+0x70/0x100
>  ? handle_mm_fault+0x256/0x360
>  ? do_user_addr_fault+0x3c1/0x860
>  ? irqentry_exit_to_user_mode+0x67/0x190
>  ? irqentry_exit+0x3b/0x50
>  ? exc_page_fault+0x89/0x180
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7fe10e2ee5cb
> Code: 0f 1e fa 48 8b 05 c5 78 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff 
> ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff 
> ff 73 01 c3 48 8b 0d 95 78 0d 00 f7 d8 64 89 01 48
> RSP: 002b:7fffb2c75518 EFLAGS: 0246 ORIG_RAX: 0010
> RAX: ffda RBX: 00078000 RCX: 7fe10e2ee5cb
> RDX: 7fffb2c75520 RSI: c028a405 RDI: 0005
> RBP: 0005 R08:  R09: 7fffb2c75594
> R10: 7fffb2c755c8 R11: 0246 R12: c028a405
> R13: 7fffb2c75520 R14: 00078000 R15: 7fe10e1a7980
>  
>  [ end trace ]

Could you trim down the trace to only include the relevant part?

E.g., please at least remove the two register dumps at the beginning and
end of the trace.

Please refer to "Backtraces in commit messages" section in
Documentation/process/submitting-patches.rst.

> 
> Signed-off-by: Bojun Zhu 
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index b65ab214bdf5..2340a82fa796 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
>   }
>  
>   mutex_unlock(>lock);
> +
> + if (need_resched())
> + cond_resched();
>   }
>  
>   ret = 0;
> @@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
> *encl,
>   entry->type = page_type;
>  
>   mutex_unlock(>lock);
> +
> + if (need_resched())
> + cond_resched();
>   }
>  
>   ret = 0;
> @@ -1156,6 +1162,9 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl,
>   

[RFC PATCH 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

2024-04-23 Thread 朱伯君(杰铭)
EDMM's ioctl()s support batch operations, which may be
time-consuming. Try to explicitly give up the CPU at
the every end of "for loop" in
sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
to give other tasks a chance to run, and avoid softlockup warning.

The following has been observed on Linux v6.9-rc5 with kernel
preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
is requested to restrict page permissions of a large number of EPC pages.

[ cut here ]
watchdog: BUG: soft lockup - CPU#45 stuck for 22s! [occlum-run:3905]
...
CPU: 45 PID: 3905 Comm: occlum-run Not tainted 6.9.0-rc5 #7
...
RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
Code: 48 c1 e6 05 48 89 d1 48 8d 5c 24 40 b8 0e 00 00 00 48 2b 8e 70 8e f5 
93 48 c1 e9 05 48 c1 e1 0c 48 03 8e 68 8e f5 93 0f 01 cf  00 00 00 40 0f 85 
b2 00 00 00 85 c0 0f 85 db 00 00 00 4c 89 ef
RSP: 0018:b55a6591fa80 EFLAGS: 0202
RAX:  RBX: b55a6591fac0 RCX: b581e7384000
RDX: b59a9e4e8080 RSI: 0020 RDI: 91d69e8cc000
RBP: b55a6591fb70 R08: 0002 R09: 91d646e12be0
R10: 006e R11: 0002 R12: 00072052d000
R13: 91d69e8cc000 R14: b55a6591fbd8 R15: 91d69e8cc020
FS:  7fe10dbda740() GS:92163e48() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7fc041811000 CR3: 0040d95c8005 CR4: 00770ef0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe07f0 DR7: 0400
PKRU: 5554
Call Trace:
 
 ? show_regs+0x67/0x70
 ? watchdog_timer_fn+0x1f3/0x280
 ? __pfx_watchdog_timer_fn+0x10/0x10
 ? __hrtimer_run_queues+0xc8/0x220
 ? hrtimer_interrupt+0x10c/0x250
 ? __sysvec_apic_timer_interrupt+0x53/0x130
 ? sysvec_apic_timer_interrupt+0x7b/0x90
 
 
 ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
 ? sgx_enclave_restrict_permissions+0xba/0x1f0
 ? __pte_offset_map_lock+0x94/0x110
 ? sgx_encl_test_and_clear_young_cb+0x40/0x60
 sgx_ioctl+0x1ab/0x900
 ? do_syscall_64+0x79/0x110
 ? apply_to_page_range+0x14/0x20
 ? sgx_encl_test_and_clear_young+0x6c/0x80
 ? sgx_vma_fault+0x132/0x4f0
 __x64_sys_ioctl+0x95/0xd0
 x64_sys_call+0x1209/0x20c0
 do_syscall_64+0x6d/0x110
 ? do_syscall_64+0x79/0x110
 ? do_pte_missing+0x2e8/0xcc0
 ? __pte_offset_map+0x1c/0x190
 ? __handle_mm_fault+0x7b9/0xe60
 ? __count_memcg_events+0x70/0x100
 ? handle_mm_fault+0x256/0x360
 ? do_user_addr_fault+0x3c1/0x860
 ? irqentry_exit_to_user_mode+0x67/0x190
 ? irqentry_exit+0x3b/0x50
 ? exc_page_fault+0x89/0x180
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7fe10e2ee5cb
Code: 0f 1e fa 48 8b 05 c5 78 0d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff 
ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 
01 c3 48 8b 0d 95 78 0d 00 f7 d8 64 89 01 48
RSP: 002b:7fffb2c75518 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 00078000 RCX: 7fe10e2ee5cb
RDX: 7fffb2c75520 RSI: c028a405 RDI: 0005
RBP: 0005 R08:  R09: 7fffb2c75594
R10: 7fffb2c755c8 R11: 0246 R12: c028a405
R13: 7fffb2c75520 R14: 00078000 R15: 7fe10e1a7980
 
 [ end trace ]

Signed-off-by: Bojun Zhu 
---
 arch/x86/kernel/cpu/sgx/ioctl.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index b65ab214bdf5..2340a82fa796 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -806,6 +806,9 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
}
 
mutex_unlock(>lock);
+
+   if (need_resched())
+   cond_resched();
}
 
ret = 0;
@@ -1010,6 +1013,9 @@ static long sgx_enclave_modify_types(struct sgx_encl 
*encl,
entry->type = page_type;
 
mutex_unlock(>lock);
+
+   if (need_resched())
+   cond_resched();
}
 
ret = 0;
@@ -1156,6 +1162,9 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl,
kfree(entry);
 
mutex_unlock(>lock);
+
+   if (need_resched())
+   cond_resched();
}
 
ret = 0;
-- 
2.25.1