[coreboot-gerrit] [S] Change in coreboot[main]: cpu/x86/smm: Fix get_save_state calculation

[Eugene Myers (Code Review)]

Attention is currently required from: EliasOfWaffle, Jérémy Compostella, Martin L Roth, Matt DeVillier, Michał Żygowski, Paul Menzel, Stefan Reinauer.

Eugene Myers has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78889?usp=email )

Change subject: cpu/x86/smm: Fix get_save_state calculation
......................................................................

Patch Set 5:

(2 comments)

Patchset:

PS5: 
This should work and actually a simpler solution. From what I can see, it is equivalent

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

https://review.coreboot.org/c/coreboot/+/78889/comment/bbac5873_04076f55 :
PS4, Line 110: 		stm_psd_size = ALIGN_UP(sizeof(TXT_PROCESSOR_SMM_DESCRIPTOR),
> I am talking about a huge change. Just sharing definition. Here is what I have in mind.
> Author:     Jeremy Compostella <[e-mail]>
> AuthorDate: Thu Nov 2 13:55:24 2023 -0700
> Commit:     Jeremy Compostella <[e-mail]>
> CommitDate: Thu Nov 2 13:59:04 2023 -0700
>
> cpu/x86/smm: Define STM descriptor size in SMM save state
>
> Change-Id: Id68f9e650be24dd4d96788d2eeaea3dd88c26bd3
> Signed-off-by: Jeremy Compostella <[e-mail]>
>
> 3 files changed, 11 insertions(+), 12 deletions(-)
> src/cpu/x86/mp_init.c                | 5 +----
> src/cpu/x86/smm/smm_module_handler.c | 9 +--------
> src/include/cpu/x86/smm.h            | 9 +++++++++
>
> modified   src/cpu/x86/mp_init.c
> @@ -1087,10 +1087,7 @@ static void fill_mp_state_smm(struct mp_state *state, const struct mp_ops *ops)
>    /*
>     * Make sure there is enough room for the SMM descriptor
>     */
> -	if (CONFIG(STM)) {
> -		state->smm_save_state_size +=
> -			ALIGN_UP(sizeof(TXT_PROCESSOR_SMM_DESCRIPTOR), 0x100);
> -	}
> +	state->smm_save_state_size += STM_PSD_SIZE;
>
>    /*
>     * Default to smm_initiate_relocation() if trigger callback isn't
> modified   src/cpu/x86/smm/smm_module_handler.c
> @@ -101,18 +101,11 @@ struct global_nvs *gnvs;
>
>  void *smm_get_save_state(int cpu)
>  {
> -	size_t stm_psd_size = 0;
> -
>    if (cpu > smm_runtime.num_cpus)
>    	return NULL;
>
> -	if (CONFIG(STM)) {
> -		stm_psd_size = ALIGN_UP(sizeof(TXT_PROCESSOR_SMM_DESCRIPTOR),
> -					0x100);
> -	}
> -
>    return (void *)(smm_runtime.save_state_top[cpu] -
> -			(smm_runtime.save_state_size - stm_psd_size));
> +			(smm_runtime.save_state_size - STM_PSD_SIZE));
>  }
>
>  uint32_t smm_revision(void)
> modified   src/include/cpu/x86/smm.h
> @@ -33,6 +33,15 @@
>
>  #define SMM_PCI_RESOURCE_STORE_NUM_RESOURCES 6
>
> +/*
> + * SMI Transfer Monitor (STM) descriptor reserved in SMM save state.
> + */
> +#if CONFIG(STM)
> +#define STM_PSD_SIZE ALIGN_UP(sizeof(TXT_PROCESSOR_SMM_DESCRIPTOR), 0x100)
> +#else
> +#define STM_PSD_SIZE 0
> +#endif
> +
>  /* Send cmd to APM_CNT with HAVE_SMI_HANDLER checking. */
>  int apm_control(u8 cmd);
>  u8 apm_get_apmc(void);