Re: [PATCH] x86/MTRR: constrain AP sync and BSP restore

[Jan Beulich <jbeulich@suse.com>]

On 27.03.2025 11:53, Roger Pau Monné wrote:
> On Thu, Mar 27, 2025 at 10:54:23AM +0100, Jan Beulich wrote:
>> mtrr_set_all() has quite a bit of overhead, which is entirely useless
>> when set_mtrr_state() really does nothing. Furthermore, with
>> mtrr_state.def_type never initialized from hardware, post_set()'s
>> unconditional writing of the MSR means would leave us running in UC
>> mode after the sync.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/cpu/mtrr/main.c
>> +++ b/xen/arch/x86/cpu/mtrr/main.c
>> @@ -605,13 +605,15 @@ void mtrr_aps_sync_begin(void)
>>  
>>  void mtrr_aps_sync_end(void)
>>  {
>> -	set_mtrr(~0U, 0, 0, 0);
>> +	if (mtrr_if)
>> +		set_mtrr(~0U, 0, 0, 0);
>>  	hold_mtrr_updates_on_aps = 0;
>>  }
>>  
>>  void mtrr_bp_restore(void)
> 
> Maybe I'm blind, but I cannot find any caller to mtrr_bp_restore()?
> Am I missing something obvious?

You don't. It was lost in 4304ff420e51 ("x86/S3: Drop
{save,restore}_rest_processor_state() completely"), with there being no
indication in the description that this was actually intentional. Looks like
another S3 regression we need to fix. Unless you, Andrew, have an explanation
for this.

Jan