RE: [PATCH V4 2/3] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

From: Prakhya, Sai Praneeth
Date: Fri Sep 07 2018 - 14:30:19 EST


> > + if (efi_rts_work.efi_rts_id == RESET_SYSTEM) {
> > + pr_info("efi_reset_system() buggy! Reboot through BIOS\n");
> > + machine_real_restart(MRR_BIOS);
> > + return 0;
> > + }
> > +
> > + /* Firmware has caused page fault, hence, freeze efi_rts_wq. */
> > + set_current_state(TASK_UNINTERRUPTIBLE);
>
> This doesn't freeze it, as such, it just sets the state.

True! Thanks for pointing it out. I will update the comment.

> > +
> > + /*
> > + * Before calling EFI Runtime Service, the kernel has switched the
> > + * calling process to efi_mm. Hence, switch back to task_mm.
> > + */
> > + arch_efi_call_virt_teardown();
> > +
> > + /* Signal error status to the efi caller process */
> > + efi_rts_work.status = EFI_ABORTED;
> > + complete(&efi_rts_work.efi_rts_comp);
> > +
> > + clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> > + pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n");
>
> > + schedule();
>
> So what happens when we get a spurious wakeup and return from this?
>
> Quite possibly you want something like:
>
> for (;;) {
> set_current_state(TASK_IDLE);
> schedule();
> }
>
> here. The TASK_UNINTERRUPTIBLE thing will cause the load-avg to spike; is that
> what you want?

Yes, makes sense. TASK_IDLE seems more appropriate. I will change it.

Regards,
Sai