Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-14 Thread Borislav Petkov
On Sat, Mar 10, 2018 at 12:33:35AM +, Prakhya, Sai Praneeth wrote: > Although the discussions were off my understanding, the present issue > I see is, (and also the motivation for me to do the patch is) when a > thread tries to execute any efi_runtime_service() we switch to efi_pgd > (which

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-09 Thread Prakhya, Sai Praneeth
> > That's true! AFAIK, we don't have any issues handling NMI while in efi_pgd. > > We might have issues only when, we are already in efi_pgd, NMI comes > > along > > Can you trigger this? Or is it something hypothetical? > AFAIK, it's hypothetical. I did try to trigger the issue, but failed

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-09 Thread Borislav Petkov
On Fri, Mar 09, 2018 at 02:37:59AM +, Prakhya, Sai Praneeth wrote: > But, I guess, we have some downsides with this design: > 1. We are doing this to have "no exceptions to use efi_rts_wq", but we will > be making > the common case complicated. i.e. When a user requests to write some efi >

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-09 Thread Borislav Petkov
On Thu, Mar 08, 2018 at 05:05:44PM +, Luck, Tony wrote: > > "Hence, pstore calls efi_runtime_services() without using efi_rts_wq" - > > that doesn't sound like optimal design to me. I would try to shove them > > all through the workqueue - not have exceptions. > > But pstore is trying to save

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-08 Thread Prakhya, Sai Praneeth
> > Another warning by checkpatch is "use of in_atomic() in drivers code" > > I'm assuming it warns because you're touching files in drivers/ but the efi > fun is > not really a driver... True! That makes sense :) > > But looking at patch 3, that thing looks like a real mess. Some of the

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-08 Thread Prakhya, Sai Praneeth
/* > >> > +* Since we process only one efi_runtime_service() at a time, an > >> > +* ordered workqueue (which creates only one execution context) > >> > +* should suffice all our needs. > >> > +*/ > >> > + efi_rts_wq =

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-08 Thread Luck, Tony
> "Hence, pstore calls efi_runtime_services() without using efi_rts_wq" - > that doesn't sound like optimal design to me. I would try to shove them > all through the workqueue - not have exceptions. But pstore is trying to save the last gasp dying words from a kernel that has paniced. There isn't

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-08 Thread Borislav Petkov
On Thu, Mar 08, 2018 at 05:31:03AM +, Prakhya, Sai Praneeth wrote: > Another warning by checkpatch is "use of in_atomic() in drivers code" I'm assuming it warns because you're touching files in drivers/ but the efi fun is not really a driver... But looking at patch 3, that thing looks like a

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-08 Thread Miguel Ojeda
On Thu, Mar 8, 2018 at 5:22 AM, Prakhya, Sai Praneeth wrote: >> > +struct workqueue_struct *efi_rts_wq; >> > + >> > static bool disable_runtime; >> > static int __init setup_noefi(char *arg) { @@ -329,6 +331,19 @@ >> > static int __init efisubsys_init(void) >> >

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-07 Thread Prakhya, Sai Praneeth
+Cc Miguel Ojeda > > > +({ > > > \ > > > + struct efi_runtime_work efi_rts_work; \ > > > + \ > > > + INIT_WORK_ONSTACK(_rts_work.work,

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-07 Thread Prakhya, Sai Praneeth
> > +({ \ > > + struct efi_runtime_work efi_rts_work; \ > > + \ > > + INIT_WORK_ONSTACK(_rts_work.work, efi_call_rts);\ > > +

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-07 Thread Prakhya, Sai Praneeth
> > +struct workqueue_struct *efi_rts_wq; > > + > > static bool disable_runtime; > > static int __init setup_noefi(char *arg) { @@ -329,6 +331,19 @@ > > static int __init efisubsys_init(void) > > return 0; > > > > /* > > +* Since we process only one

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-07 Thread Prakhya, Sai Praneeth
-0800, Sai Praneeth Prakhya wrote: > > @@ -329,6 +331,19 @@ static int __init efisubsys_init(void) > > return 0; > > > > /* > > +* Since we process only one efi_runtime_service() at a time, an > > +* ordered workqueue (which creates only one execution context) > > +*

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-07 Thread Borislav Petkov
On Mon, Mar 05, 2018 at 03:23:09PM -0800, Sai Praneeth Prakhya wrote: > +#define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5) > \ > +({ \ > + struct efi_runtime_work efi_rts_work;

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-07 Thread Miguel Ojeda
On Tue, Mar 6, 2018 at 12:23 AM, Sai Praneeth Prakhya wrote: > From: Sai Praneeth > > When a process requests the kernel to execute any efi_runtime_service(), > the requested efi_runtime_service (represented as an identifier) and

Re: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-06 Thread Mark Rutland
On Mon, Mar 05, 2018 at 03:23:09PM -0800, Sai Praneeth Prakhya wrote: > @@ -329,6 +331,19 @@ static int __init efisubsys_init(void) > return 0; > > /* > + * Since we process only one efi_runtime_service() at a time, an > + * ordered workqueue (which creates only one

[PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

2018-03-05 Thread Sai Praneeth Prakhya
From: Sai Praneeth When a process requests the kernel to execute any efi_runtime_service(), the requested efi_runtime_service (represented as an identifier) and its arguments are packed into a struct named efi_runtime_work and queued onto work queue named