Patch Set 3:

(18 comments)

Patch Set 2:

Patch Set 2:

(3 comments)

This change is ready for review.

Fat finger - needs changes first.

Eugene,
Thank you for the review. I've uploaded patch set 3 with your feedback. I also updated the SMRAM layout map (described on top of smm_load_module). STM stays as is. Moved stacks to the start of SMRAM since stacks grow downward and they can't corrupt any SMM code. SMM GDT (global descriptor table) is set for flat code and flat data, so there are no protections. By having stacks on top, in theory they could grow downward enough to corrupt code and data, though we have a canary in place, it could still cause problems. That is my reasoning for moving stacks to the bottom for better protection (unless we change the descriptor for stack segment and make it bound, then it can be anywhere in SMRAM).

Rocky,

I've ported your code to my test system and realized that there were a couple of issues.

The first is the location of the fxsave area, the calculation placed the area outside of SMRAM.

The second is that the algorithm to place the entrypoints has to account for the SMM save state areas. I've borrowed some documentation to help give you a start.

   gene

View Change

2 comments:

• File src/cpu/x86/smm/smm_module_loader.c:

  • Patch Set #3, Line 96:

    When building these segments the algorithm needs to account for the SMM save state size and location. This area includes the SMM save state and the TXT Processor SMM Descriptor.
    That layout is as follows:

    SMBASE+0xFC00 SMM Save state map
    SMBASE+0xFB00 TXT Processor SMM Descriptor
    SMBASE+0x8000 Entry point
    SMBASE

    I believe what you are trying to do is something like this:
    (note: this diagram is from - https://github.com/jyao1/STM/blob/master/Bios/StmCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c )

    //
      // The CPU save state and code for the SMI entry point are tiled within an SMRAM
      // allocated buffer.  The minimum size of this buffer for a uniprocessor system
      // is 32 KB, because the entry point is SMBASE + 32KB, and CPU save state area
      // just below SMBASE + 64KB.  If more than one CPU is present in the platform,
      // then the SMI entry point and the CPU save state areas can be tiles to minimize
      // the total amount SMRAM required for all the CPUs.  The tile size can be computed
      // by adding the   // CPU save state size, any extra CPU specific context, and
      // the size of code that must be placed at the SMI entry point to transfer
      // control to a C function in the native SMM execution mode.  This size is
      // rounded up to the nearest power of 2 to give the tile size for a each CPU.
      // The total amount of memory required is the maximum number of CPUs that
      // platform supports times the tile size.  The picture below shows the tiling,
      // where m is the number of tiles that fit in 32KB.
      //
      //  +-----------------------------+  <-- 2^n offset from Base of allocated buffer
      //  |   CPU m+1 Save State        |
      //  +-----------------------------+
      //  |   CPU m+1 Extra Data        |
      //  +-----------------------------+
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU 2m  SMI Entry         |
      //  +#############################+  <-- Base of allocated buffer + 64 KB
      //  |   CPU m-1 Save State        |
      //  +-----------------------------+
      //  |   CPU m-1 Extra Data        |
      //  +-----------------------------+
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU 2m-1 SMI Entry        |
      //  +=============================+  <-- 2^n offset from Base of allocated buffer
      //  |   . . . . . . . . . . . .   |
      //  +=============================+  <-- 2^n offset from Base of allocated buffer
      //  |   CPU 2 Save State          |
      //  +-----------------------------+
      //  |   CPU 2 Extra Data          |
      //  +-----------------------------+
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU m+1 SMI Entry         |
      //  +=============================+  <-- Base of allocated buffer + 32 KB
      //  |   CPU 1 Save State          |
      //  +-----------------------------+
      //  |   CPU 1 Extra Data          |
      //  +-----------------------------+
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU m SMI Entry           |
      //  +#############################+  <-- Base of allocated buffer + 32 KB == CPU 0 SMBASE + 64 KB
      //  |   CPU 0 Save State          |
      //  +-----------------------------+
      //  |   CPU 0 Extra Data          |
      //  +-----------------------------+
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU m-1 SMI Entry         |
      //  +=============================+  <-- 2^n offset from Base of allocated buffer
      //  |   . . . . . . . . . . . .   |
      //  +=============================+  <-- 2^n offset from Base of allocated buffer
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU 1 SMI Entry           |
      //  +=============================+  <-- 2^n offset from Base of allocated buffer
      //  |   Padding                   |
      //  +-----------------------------+
      //  |   CPU 0 SMI Entry           |
      //  +#############################+  <-- Base of allocated buffer == CPU 0 SMBASE + 32 KB

  • Patch Set #3, Line 573: fxsave_area -= total_stack_size + fxsave_size;

    This calculation can place the location of the fxsave_area below the bottom of the TSEG. deleting the above line will place the fxsave_area above the stack. If you use base at this point for the location it will be below the MSEG + BIOS resource list

To view, visit change 41829. To unsubscribe, or for help writing mail filters, visit settings.