[coreboot-gerrit] Change in coreboot[master]: soc/intel/xeon_sp: Allow ACPI SMI GNVS

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48492 )

Change subject: soc/intel/xeon_sp: Allow ACPI SMI GNVS
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2: Code-Review-1
> >
> > This requires properly fetching SMM save states which is not implemented correctly with smmloader v2. Then again other things in that smihandler accessing the save state are also there and are also possibly broken...
>
> smm loader v2 will work with state save.  The trick is to get the smbase using the MSR of the CPU executing the code (remember only 1 cpu thread is allowed to enter SMI handling code, while others wait).  Below is a snippet of code I'm using to modify memory in state save.
>
> static uint64_t ras_get_smbase(void)
> {
>    msr_t msr = rdmsr(IA32_SMBASE);
>    uint64_t smbase = 0;
>    smbase = msr.hi;
>    smbase = smbase << 32;
>    smbase |= msr.lo;
>    return smbase;
> }
> static void ras_post_cmci(void)
> {
>    em64t101_smm_state_save_area_t *save_state;
>    uintptr_t base = ras_get_smbase();
>    printk(BIOS_DEBUG, "%s, smbase = 0x%zx from MSR\n", __func__, base);
>    save_state = (void *)(base + SMM_DEFAULT_SIZE - sizeof(*save_state));
>    printk(BIOS_DEBUG, "%s, smbase & = %x\n", __func__, (uint32_t)&save_state->smbase);
>    printk(BIOS_DEBUG, "%s, smbase = %x\n", __func__, save_state->smbase);
>    printk(BIOS_DEBUG, "%s, smm_revision = %x\n", __func__, save_state->smm_revision);
>    save_state->event_control = 0x1;
> }

What our code currently does is to loop over all save states to find which core initiated the Synchronous IO SMI to account for cases where the core initiating the SMI != core in SMM (and where IA32_SMBASE is not supported). For this one needs to know the location of all save states which is a bit more complicated with smmloader v2. (https://review.coreboot.org/c/coreboot/+/47081/2 not working)