Re: [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value

[Wei Liu <wei.liu2@citrix.com>]

On Wed, May 16, 2018 at 12:08:02PM +0100, Andrew Cooper wrote:
> On 14/05/18 16:52, Jan Beulich wrote:
> >>>> On 14.05.18 at 17:39, <wei.liu2@citrix.com> wrote:
> >> On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
> >>> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
> >>>          setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
> >>>  
> >>>      print_details(thunk, caps);
> >>> +
> >>> +    /*
> >>> +     * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
> >>> +     * any firmware settings.  For performance reasons on native hardware, we
> >>> +     * delay applying non-zero settings until after dom0 has been constructed.
> >>> +     */
> >>> +    if ( boot_cpu_has(X86_FEATURE_IBRSB) )
> >>> +    {
> >>> +        bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
> >>> +
> >> Why is cpu_has_hypervisor needed here?  This should help nested case as
> >> well. And it wouldn't make the setup less secure, right?
> > Ah, yes, Andrew, this should indeed be explained in at least one of comment
> > or commit message.
> 
> I've adjusted this comment to read:
> 
> /*
>  * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
>  * any firmware settings.  For performance reasons, when safe to do so, we
>  * delay applying non-zero settings until after dom0 has been constructed.
>  *
>  * "when safe to do so" is based on whether we are virtualised.  A native
>  * boot won't have any other code running in a position to mount an
>  * attack.
>  */
> 
> and added the same second paragraph to the commit message.

LGTM. Thanks!

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel