On Tue, Aug 7, 2018 at 11:01 AM, Prakhya, Sai Praneeth <sai.praneeth.prakhya@xxxxxxxxx> wrote: > Hi All, > >> > >> The main problem is that we have just merged Sai's code to use a >> > >> work queue for invoking EFI services, and killing kworker threads >> > >> is obviously not going to make EFI any new friends. >> > >> >> > >> So I guess we should probably rework that code to use a dedicated >> > >> kthread, and just freeze it when it performs an illegal memory >> > >> access, and signal the completion in a way that makes the calling >> > >> thread understand that a) the call failed and b) runtime services >> > >> are no longer >> > available. >> > > >> > > Yes, this makes sense to me. >> > > Initially I did use a dedicated kthread for efi but moved to work >> > > queues later so >> > that the synchronization is well handled. I am ok to rework on the >> > patches, could we ask Ingo to hold onto efi_workqueue patches? >> > > >> > >> > I am fine with keeping them. We will have a different approach in >> > v4.19 than in subsequent kernels, but the workqueue approach is still >> > better than nothing at all. > > As discussed in previous mails, I have implemented the below: > > If kernel detects an illegal access by firmware to any efi region other than > efi boot time code/data, > a. It freezes "efi_rts_wq" (efi runtime services work queue). > b. Signals the requested process that an error occurred. > c. Disables EFI Runtime Services forever. > d. Explicitly calls the scheduler to schedule another process. > > But, I have couple of questions: > > 1. Since we don't use "efi_rts_wq" for efi_reset_system(), the above solution > doesn't work if efi_reset_system() is buggy. We cannot neglect it either because > that's the issue faced by Al Stone and hence I started the patch set. So, I am thinking > to use long jump technique *only* for efi_reset_system(). Would you be OK with that? > Or please propose a new solution. The long jump thing is super ugly. I'd be okay with it if it were somehow factored out into a testable mechanism, but it's a nasty complicated bit of assembly, it will only be used very very rarely, and it more or less duplicates switch_to(). Could you just do a fallback reboot from the fault handler if efi_reset_system() fails? > > 2. Also, as suggested, I haven’t used a dedicated "efi_kthread", I got this working with > the existing "efi_rts_wq" setup. Please note that, the existing "efi_rts_wq", has only one > worker thread. Could you please brief me on why we should have a dedicated kthread > and not leverage "efi_rts_wq" (I am unclear on this). Two reasons: 1. If you used a dedicated thread, then you could probably just set that thread's ->mm to the EFI mm rather than manually switching the mm. 2. If you used a dedicated thread, you would be a lot closer to being able to call into EFI at CPL 3. In x86 Linux (and probably most or all other arches), the kernel never calls into CPL 3. Instead it *returns* into CPL 3. So you would set up a thread and make that thread's task_pt_regs() have the correct context for a little stub that calls into EFI and then does "syscall". Then you would return all the way out of the kernel into the stub. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
