[coreboot] Question about finalization of SMM

[Nico Huber]

On 18.08.2017 09:20, Shawn wrote:
> On Tue, Aug 15, 2017 at 1:04 PM, Persmule <persmule at gmail.com> wrote:
>> Hi all,
>>
>> When using chipsec ( https://github.com/chipsec/chipsec ) to analyse
>> possible vulnerabilities inside coreboot systems, I noticed that on
>> several intel-based systems running coreboot,(e.g.
>> https://review.coreboot.org/cgit/board-status.git/tree/lenovo/x230/4.6-938-gb08d73b845/2017-08-01T23_05_52Z
>> ) several registers on the pci-e root complex (host bridge) is not
>> locked while locked on the same system running oem firmware.
>>
>> Digging into the source code, I found a function defined inside
>> ${COREBOOT_DIR}/src/northbridge/intel/{nehalem, sandybridge,
>> haswell}/finalize.c to lock these registers, but this function will only
>> be called if #SMI APM_CNT gets triggered with a certain parameter. ( The
>> handler of #SMI APM_CNT is usually defined as function
>> "southbridge_smi_apmc" inside
>> ${COREBOOT_DIR}/src/${VENDOR}/${MAINBOARD}/smihandler.c or
>> ${COREBOOT_DIR}/src/southbridge/intel/${CHIPSET}/smihandler.c, and the
>> lockdown function will be called with parameter register APM_CNT ==
>> APM_CNT_FINALIZE.)
>>
>> That these registers are left unlocked indicates that #SMI APM_CNT is
>> never triggered with APM_CNT ==  APM_CNT_FINALIZE during boot. I would
>> like to ask, that when does the #SMI APM_CNT is expected to be triggered
>> with APM_CNT ==  APM_CNT_FINALIZE, and which component of the system
>> (e.g. coreboot, payload, or os kernel) is responsible for the triggering?

It should be triggered by coreboot or the payload, IMO. The original
idea was to do it from the payload, I guess, because it wouldn't make
much sense to gather all of this in the SMM handler otherwise.

>>
> It seems will be triggered after S3, southbridge/intel/bd82x6x/lpc.c:
> 
> static void lpc_final(struct device *dev)
> {
>         if (CONFIG_HAVE_SMI_HANDLER && acpi_is_wakeup_s3()) {
>                 /* Call SMM finalize() handlers before resume */
>                 outb(0xcb, 0xb2);
>         }
> }
> 
> It's weird a bit. Those LOCKs bit should be set at the boot time.

I guess it was originally called from the payload and thus had to be
done by coreboot on the resume path (where no payload is called). AFAIK,
coreboot also does it on the regular path for newer chipsets. A lot
of people already stumbled upon this problem on Sandy Bridge but it
seems nobody chose to do it in coreboot, yet.

> Will
> coreboot provide an option in the future?

My oracle says yes. Though it's not specific about when or by whom it
will be done. In other words: Patches are welcome.

Nico