Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34257 )
Change subject: cpu/x86: Define INSTALL_SMM option ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Don't build smm -class. Absolute minimal resident SMM may be installed, like asm inline "rsm"? Let MP init raise SMIs to relocate SMM stacks? On entry to payload, SMRAM can be locked or left unlocked.
Is this behavior a long term goal?
I just noticed that it was pretty hard to try evaluate what is built into SMM currently with all the guards in place. I think we would want to separate SMM to rmodule file in CBFS. Things should be stable enough such that one may use audited SMM build from previous build, yet update ramstage as needed?
Seems fine to me. smm being built into ramstage proper is mainly legacy.
Here's what's in there now:
$ git grep _binary -- src/ src/cpu/amd/smm/smm_init.c: memcpy((void *)SMM_BASE, _binary_smm_start, src/cpu/amd/smm/smm_init.c: _binary_smm_end - _binary_smm_start); src/cpu/x86/mp_init.c:extern char _binary_sipi_vector_start[]; src/cpu/x86/mp_init.c: if (rmodule_parse(&_binary_sipi_vector_start, &sipi_mod)) { src/cpu/x86/smm/smm_module_loader.c:extern unsigned char _binary_smmstub_start[]; src/cpu/x86/smm/smm_module_loader.c: if (rmodule_parse(&_binary_smmstub_start, &smm_stub)) src/cpu/x86/smm/smm_module_loader.c: if (rmodule_parse(&_binary_smm_start, &smm_mod)) src/drivers/vpd/vpd_tables.h:struct vpd_table_binary_blob_pointer { src/include/cpu/x86/smm.h:extern unsigned char _binary_smm_start[]; src/include/cpu/x86/smm.h:extern unsigned char _binary_smm_end[]; src/include/gpio.h:static inline uint32_t gpio_binary_first_base3_value(const gpio_t gpio[], src/mainboard/google/gale/boardid.c: bid = gpio_binary_first_base3_value(hw_rev_gpios, src/mainboard/google/veyron/boardid.c: code = gpio_binary_first_base3_value(pins, ARRAY_SIZE(pins)); src/mainboard/google/veyron_mickey/boardid.c: code = gpio_binary_first_base3_value(pins, ARRAY_SIZE(pins)); src/mainboard/google/veyron_rialto/boardid.c: code = gpio_binary_first_base3_value(pins, ARRAY_SIZE(pins)); src/southbridge/intel/i82801dx/smi.c: memcpy((void *)0xa0000, _binary_smm_start, src/southbridge/intel/i82801dx/smi.c: _binary_smm_end - _binary_smm_start); src/southbridge/intel/i82801ix/smi.c: memcpy((void *)0xa0000, _binary_smm_start, src/southbridge/intel/i82801ix/smi.c: _binary_smm_end - _binary_smm_start); [adurbin@adurbin:~/devel/coreboot]
Also with the experiments/initiatives to leave SMM loading to OS, I'd like to see that platforms at least pass the build in such a use case. Currently HAVE_SMI_HANDLER=n mostly will not build.
What definition do we have HAVE_SMI_HANDLER to be going forward? That it can support SMM? And we want to turn off the permanent handler installation with INSTALL_SMM?
I think HAVE_SMI_HANDLER would indicate platform code has support to raise and configure SMIs. Essentially, satisfy the requirements of PARALLEL_MP code.
I think the combinations seem legit, but I think the naming could be more descriptive in what we're aiming for.
Sure.