On Mon, Mar 08, 2021, Yang Weijiang wrote:
> On Thu, Mar 04, 2021 at 08:46:45AM -0800, Sean Christopherson wrote:
> > On Thu, Mar 04, 2021, Yang Weijiang wrote:
> > > @@ -3375,6 +3391,12 @@ enum nvmx_vmentry_status
> > > nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
> > > if (kvm_mpx_supported() &&
> > > !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))
> > > vmx->nested.vmcs01_guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS);
> > > + if (kvm_cet_supported() &&
> > > + !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_CET_STATE)) {
> >
> > Alignment.
> >
> > > + vmx->nested.vmcs01_guest_ssp = vmcs_readl(GUEST_SSP);
> > > + vmx->nested.vmcs01_guest_s_cet = vmcs_readl(GUEST_S_CET);
> > > + vmx->nested.vmcs01_guest_ssp_tbl =
> > > vmcs_readl(GUEST_INTR_SSP_TABLE);
> > > + }
> > >
> > > /*
> > > * Overwrite vmcs01.GUEST_CR3 with L1's CR3 if EPT is disabled *and*
> > > @@ -4001,6 +4023,9 @@ static bool is_vmcs12_ext_field(unsigned long field)
> > > case GUEST_IDTR_BASE:
> > > case GUEST_PENDING_DBG_EXCEPTIONS:
> > > case GUEST_BNDCFGS:
> > > + case GUEST_SSP:
> > > + case GUEST_INTR_SSP_TABLE:
> > > + case GUEST_S_CET:
> > > return true;
> > > default:
> > > break;
> > > @@ -4052,6 +4077,11 @@ static void sync_vmcs02_to_vmcs12_rare(struct
> > > kvm_vcpu *vcpu,
> > > vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS);
> > > if (kvm_mpx_supported())
> > > vmcs12->guest_bndcfgs = vmcs_read64(GUEST_BNDCFGS);
> > > + if (kvm_cet_supported()) {
> >
> > Isn't the existing kvm_mpx_supported() check wrong in the sense that KVM
> > only
> > needs to sync to vmcs12 if KVM and the guest both support MPX?
>
> For MPX, if guest_cpuid_has() is not efficent, can it be checked by BNDCFGS
> EN bit?
> E.g.:
>
> if (kvm_mpx_supported() && (vmcs12->guest_bndcfgs & 1))
>
> > Same would apply to CET. Not sure it'd be a net positive in terms of
> > performance since
> > guest_cpuid_has() can be quite slow, but overwriting vmcs12 fields that
> > technically don't exist
> > feels wrong.
>
> For CET, can we get equivalent effect by checking vmcs12->guest_cr4.CET?
> E.g.:
> if (kvm_cet_supported() && (vmcs12->guest_cr4 & X86_CR4_CET))
No, because the existence of the fields does not depend on them being enabled.
E.g. things will go sideways if the values change while L2 is running, L2
disables CET, and then an exit occurs.
This is already a slow path, maybe the guest_cpuid_has() checks are a non-issue.
>
> >
> > > + vmcs12->guest_ssp = vmcs_readl(GUEST_SSP);
> > > + vmcs12->guest_s_cet = vmcs_readl(GUEST_S_CET);
> > > + vmcs12->guest_ssp_tbl = vmcs_readl(GUEST_INTR_SSP_TABLE);
> > > + }
> > >
> > > vmx->nested.need_sync_vmcs02_to_vmcs12_rare = false;
> > > }
> > > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> > > index 9d3a557949ac..36dc4fdb0909 100644
> > > --- a/arch/x86/kvm/vmx/vmx.h
> > > +++ b/arch/x86/kvm/vmx/vmx.h
> > > @@ -155,6 +155,9 @@ struct nested_vmx {
> > > /* to migrate it to L2 if VM_ENTRY_LOAD_DEBUG_CONTROLS is off */
> > > u64 vmcs01_debugctl;
> > > u64 vmcs01_guest_bndcfgs;
> > > + u64 vmcs01_guest_ssp;
> > > + u64 vmcs01_guest_s_cet;
> > > + u64 vmcs01_guest_ssp_tbl;
> > >
> > > /* to migrate it to L1 if L2 writes to L1's CR8 directly */
> > > int l1_tpr_threshold;
> > > --
> > > 2.26.2
> > >
