On 31/01/2022 10:20, Jan Beulich wrote:
> On 28.01.2022 14:29, Andrew Cooper wrote:
>> In some cases, writes to MSR_SPEC_CTRL do not have interesting side effects,
>> and we should implement lazy context switching like we do with other MSRs.
>>
>> In the short term, this will be used by the SVM infrastructure, but I expect
>> to extend it to other contexts in due course.
>>
>> Introduce cpu_info.last_spec_ctrl for the purpose, and cache writes made from
>> the boot/resume paths. The value can't live in regular per-cpu data when it
>> is eventually used for PV guests when XPTI might be active.
>>
>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> Technically
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks.
> But ...
>
>> --- a/xen/arch/x86/acpi/power.c
>> +++ b/xen/arch/x86/acpi/power.c
>> @@ -296,7 +296,10 @@ static int enter_state(u32 state)
>> ci->spec_ctrl_flags |= (default_spec_ctrl_flags & SCF_ist_wrmsr);
>>
>> if ( boot_cpu_has(X86_FEATURE_IBRSB) )
>> + {
>> wrmsrl(MSR_SPEC_CTRL, default_xen_mcu_opt_ctrl);
>> + ci->last_spec_ctrl = default_xen_mcu_opt_ctrl;
>> + }
> ... wouldn't we better have a write_spec_ctrl() helper doing both,
> perhaps with the check-if-same logic added as well (overridable by
> a "force" boolean input, unless the cases where the write cannot be
> skipped can be entirely determined from previous and new values)?
I considered that, but decided against it. The PV and IST caching logic
is likely to be in asm, meaning that the single piece of C check-if-same
logic is in vmentry_spec_ctrl().
i.e. I think this is the right way forward. By the end of the PV work,
if there is obvious tidy-up to do, I will do so.
~Andrew
