[coreboot-gerrit] Change in coreboot[master]: cpu/x86/smm: Introduce SMM module loader version 2

[Arthur Heymans (Code Review)]

Attention is currently required from: Rocky Phagura.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43684 )

Change subject: cpu/x86/smm: Introduce SMM module loader version 2
......................................................................

Patch Set 11:

(1 comment)

File src/cpu/x86/smm/smm_module_loaderv2.c:

https://review.coreboot.org/c/coreboot/+/43684/comment/84cbcb22_3314ca80 
PS11, Line 425: if (!smm_stub_place_staggered_entry_points(base, params, &smm_stub)) {
              : 		printk(BIOS_ERR, "%s: staggered entry points failed\n", __func__);
              : 		return -1;
              : 	}
              : 
              : 	/* Setup the parameters for the stub code. */
              : 	stub_params = rmodule_parameters(&smm_stub);
              : 	stub_params->stack_top = (uintptr_t)stacks_top;
              : 	stub_params->stack_size = params->per_cpu_stack_size;
              : 	stub_params->c_handler = (uintptr_t)params->handler;
              : 	stub_params->c_handler_arg = (uintptr_t)params->handler_arg;
              : 	stub_params->fxsave_area = (uintptr_t)fxsave_area;
              : 	stub_params->fxsave_area_size = FXSAVE_SIZE;
              : 	stub_params->runtime.smbase = (uintptr_t)smbase;
              : 	stub_params->runtime.smm_size = smm_size;
              : 	stub_params->runtime.save_state_size = params->per_cpu_save_state_size;
              : 	stub_params->runtime.num_cpus = params->num_concurrent_stacks;
hmmmfff this is bad! this only sets the c_handler on the BSP entry. On all the APs SMIs result in jumps to whatever c_handler is, likely 0. This means the OS kernel can likely highjack SMM!