6753 remove acpi_fw.h

Review Request #177 — Created March 16, 2016 and submitted

tsoome
illumos-gate
6753
general
remove acpi_fw.h

boot tests on vmware fusion 8.1.0 VM, supermicro X10SAE, Thinkpad T520.

  • 0
  • 0
  • 3
  • 0
  • 3
Description From Last Updated
jeffpc
  1. Good cleanup! Did you try to see if the code generated is the same via wsdiff? If it is the same then you can skip testing. If it isn't, then at least you know what needs testing.

    1. it will be different because of cpu_apicid_array is extended to array of uint32_t (was uint8_t) and its allocated now and not using static size. 32bit is needed because x2apic id is 32bit. Also note as acpica structures do not have tail pointers for subtables, iterating the tables had to be adjusted accordingly...

    2. Pity. I didn't realize that this actually changed the behavior as well - you're looking at x2apic fields instead of just looking at "good" ol' apic.

    3. logically it should be the same, as x2apic bits were already there, but those related data structures have been changed since the last change, and madt processing (APIC table) can not just dump 32bit values to array of 8bit values:D or perhaps it was just overlooked when this code was originally created...

      also note the current code dumping cpu/memory structures to properties is quite useless, only cpu_acpic_array property is re-used in lgrpplat.c - I have feeling the original intent was to use acpi-srat-processor-X and acpi-srat-memory-* properties as well, so there are some options for future clean up/rework as well:)

  2. 
      
tsoome
tsoome
tsoome
jeffpc
  1. 
      
  2. usr/src/uts/i86pc/os/fakebop.c (Diff revision 1)
     
     

    Minor style suggestion if you wish to change this:

    if (strcmp(...) == 0 &&
        valid_rsdp(...))
            return (...);
    
  3. usr/src/uts/i86pc/os/fakebop.c (Diff revision 1)
     
     

    (1) is the idea to print only unknown entries? or would it make sense to print all entries?
    (2) missing break; (I know, it won't change the behavior)

    1. was debug message and missing if (kbm_debug) :) and yes, meant only for "other" types.

  4. 
      
tsoome
tsoome
risto3
  1. 
      
  2. usr/src/uts/i86pc/os/fakebop.c (Diff revision 2)
     
     

    any reason not to prepare for ACPI_VALIDATE_RSDP_SIG here?

    You could add an #ifndef/#endif and make the same def as in recent ACPICA.

    1. it would mean the cleanup would be in order any way; I think the better would be to wait for acpica update and then implement the change.

  3. 
      
tsoome
jeffpc
  1. 
      
  2. usr/src/uts/i86pc/os/fakebop.c (Diff revisions 1 - 3)
     
     

    It's hard to tell in rb... is the return indented one too many tabs?

    1. yes it was, thanks:)

  3. 
      
tsoome
jeffpc
  1. Ship It!
  2. 
      
hans
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...