7161 uefi reboot needs to use acpi_reset_system() if available

Review Request #205 — Created July 2, 2016 and submitted

tsoome
illumos-gate
7161
general
7161 uefi reboot needs to use acpi_reset_system() if available

tested on vmware fusion with uefi firmware, qemu + OVMF-X64-r15214, supermicro X10SAE and MacBookPro10,1 - all behaving as expected.

tsoome
andy_js
  1. I would rather to see the existing "if" block updated (i.e "options_dip != NULL" changed to "options_dip != NULL && bootops != NULL") but that's just a matter of preference. LGTM.

    1. I think (may be wrong tho, hard to tell without large scale testing on differnt systems), we still want to keep efi_reboot() in case acpi reset fails (as it can fail, there are few cases in acpica code). Also, the reset may be called before kernel is fully initialised (as noted in comment below the added lines), so the intent is to call acpi reset method first if the kernel is ready, otherwise fall back to efi_reset().

  2. 
      
rm
  1. Can you elaborate on why the presence or lack of bootops is the proper way for us to determine whether or not to call into ACPI?

    1. I was actually relaying on comment starting at line 474, which also is pointing to the issue that we dont really have better way to track the availability of the acpica method. Also note that this workaround is only valid till we have mechanism to access UEFI runtime services, as runtime services does provide access to reset method with ability to specify if cold or warm reset is to be performed. Which also will allow to fix the second issue in reset() function - uefi system should not use the bios data area (address 0x472 for setting warm reset). However, even once runtime services support is implemented and available, the issue about how to determine if the mechanism is actually available (if the module is loaded), is still there.

    2. Okay, sounds like bootops is basically just a proxy for those stubs referenced in that comment. Make sense. Thanks.

  2. 
      
tsoome
rm
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...