Re: [PATCH for-4.17 1/4] amd/virt_ssbd: set SSBD at vCPU context switch

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.10.2022 18:02, Roger Pau Monne wrote:
> @@ -140,6 +135,7 @@ __UNLIKELY_END(nsvm_hap)
>           */
>          stgi
>  GLOBAL(svm_stgi_label)
> +
>          mov  %rsp,%rdi
>          call svm_vmexit_handler
>          jmp  .Lsvm_do_resume

Seemingly stray change?

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -973,6 +973,14 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v)
>  
>      /* Resume use of ISTs now that the host TR is reinstated. */
>      enable_each_ist(idt_tables[cpu]);
> +
> +    /*
> +     * Clear previous guest selection of SSBD if set.  Note that 
> SPEC_CTRL.SSBD
> +     * is already cleared by svm_vmexit_spec_ctrl.
> +     */
> +    if ( v->domain->arch.cpuid->extd.virt_ssbd &&

With this false, can ...

> +         (v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD) )

... this bit ever be set? IOW if the former condition actually needed here?

> +        amd_set_ssbd(false);
>  }
>  
>  static void cf_check svm_ctxt_switch_to(struct vcpu *v)
> @@ -1000,6 +1008,11 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v)
>  
>      if ( cpu_has_msr_tsc_aux )
>          wrmsr_tsc_aux(v->arch.msrs->tsc_aux);
> +
> +    /* Load SSBD if set by the guest. */
> +    if ( v->domain->arch.cpuid->extd.virt_ssbd &&
> +         (v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD) )
> +        amd_set_ssbd(true);
>  }

Same here then.

> @@ -2518,6 +2531,10 @@ static void cf_check svm_set_reg(struct vcpu *v, 
> unsigned int reg, uint64_t val)
>          vmcb->spec_ctrl = val;
>          break;
>  
> +    case MSR_VIRT_SPEC_CTRL:
> +        amd_set_ssbd(v->arch.msrs->virt_spec_ctrl.raw);

Would seem cheaper to pass "val & SPEC_CTRL_SSBD" here.

Jan