On Thu, Feb 11, 2021 at 09:18:03AM -0800, Sean Christopherson wrote:
> On Tue, Feb 09, 2021, Yang Weijiang wrote:
> > When L2 guest status has been changed by L1 QEMU/KVM, sync the change back
> > to L2 guest before the later's next vm-entry. On the other hand, if it's
> > changed due to L2 guest, sync it back so as to let L1 guest see the change.
> >
> > Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> > ---
> > arch/x86/kvm/vmx/nested.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > index 9728efd529a1..b9d8db8facea 100644
> > --- a/arch/x86/kvm/vmx/nested.c
> > +++ b/arch/x86/kvm/vmx/nested.c
> > @@ -2602,6 +2602,12 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
> > /* Note: may modify VM_ENTRY/EXIT_CONTROLS and GUEST/HOST_IA32_EFER */
> > vmx_set_efer(vcpu, vcpu->arch.efer);
> >
> > + if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_CET_STATE) {
> > + vmcs_writel(GUEST_SSP, vmcs12->guest_ssp);
> > + vmcs_writel(GUEST_INTR_SSP_TABLE, vmcs12->guest_ssp_tbl);
> > + vmcs_writel(GUEST_S_CET, vmcs12->guest_s_cet);
> > + }
> > +
>
> This is incomplete. If VM_ENTRY_LOAD_CET_STATE is not set, then CET state needs
> to be propagated from vmcs01 to vmcs02. See nested.vmcs01_debugctl and
> nested.vmcs01_guest_bndcfgs.
>
> It's tempting to say that we should add machinery to simplify implementing new
> fields that are conditionally loading, e.g. define an array that specifies the
> field, its control, and its offset in vmcs12, then process the array at the
> appropriate time. That might be overkill though...
>
Thanks Sean! I'll check the implementation of the two features.
> > /*
> > * Guest state is invalid and unrestricted guest is disabled,
> > * which means L1 attempted VMEntry to L2 with invalid state.
> > @@ -4152,6 +4158,12 @@ static void sync_vmcs02_to_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> >
> > if (vmcs12->vm_exit_controls & VM_EXIT_SAVE_IA32_EFER)
> > vmcs12->guest_ia32_efer = vcpu->arch.efer;
> > +
> > + if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_CET_STATE) {
>
> This is wrong, guest state is saved on VM-Exit if the control is _supported_,
> it doesn't have to be enabled.
>
> If the processor supports the 1-setting of the “load CET” VM-entry control,
> the contents of the IA32_S_CET and IA32_INTERRUPT_SSP_TABLE_ADDR MSRs are
> saved into the corresponding fields. On processors that do not support Intel
> 64 architecture, bits 63:32 of these MSRs are not saved.
>
> And I'm pretty sure we should define these fields as a so called "rare" fields,
> i.e. add 'em to the case statement in is_vmcs12_ext_field() and process them in
> sync_vmcs02_to_vmcs12_rare(). CET isn't easily emulated, so they should almost
> never be read/written by a VMM, and thus aren't with synchronizing to vmcs12 on
> every exit.
Sure, will modifiy the patch accordingly.
>
> > + vmcs12->guest_ssp = vmcs_readl(GUEST_SSP);
> > + vmcs12->guest_ssp_tbl = vmcs_readl(GUEST_INTR_SSP_TABLE);
> > + vmcs12->guest_s_cet = vmcs_readl(GUEST_S_CET);
> > + }
> > }
> >
> > /*
> > --
> > 2.26.2
> >
