| Message ID | 20210426175421.30497-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Initial pieces for guest CET support | expand |
On 26.04.2021 19:54, Andrew Cooper wrote: > VT-x has separate entry/exit control for loading guest/host state. Saving > guest state on vmexit is performed unconditionally. With the latter I find ... > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -2014,6 +2014,9 @@ void vmcs_dump_vcpu(struct vcpu *v) > printk("RFLAGS=0x%08lx (0x%08lx) DR7 = 0x%016lx\n", > vmr(GUEST_RFLAGS), regs->rflags, > vmr(GUEST_DR7)); > + if ( vmentry_ctl & VM_ENTRY_LOAD_GUEST_CET ) > + printk("SSP = 0x%016lx S_CET = 0x%016lx ISST = 0x%016lx\n", > + vmr(GUEST_SSP), vmr(GUEST_S_CET), vmr(GUEST_ISST)); ... the conditional here a little odd, but I expect the plan is to have the various bits all set consistently once actually enabling the functionality. Jan
On 27/04/2021 16:56, Jan Beulich wrote: > On 26.04.2021 19:54, Andrew Cooper wrote: >> VT-x has separate entry/exit control for loading guest/host state. Saving >> guest state on vmexit is performed unconditionally. > With the latter I find ... > >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -2014,6 +2014,9 @@ void vmcs_dump_vcpu(struct vcpu *v) >> printk("RFLAGS=0x%08lx (0x%08lx) DR7 = 0x%016lx\n", >> vmr(GUEST_RFLAGS), regs->rflags, >> vmr(GUEST_DR7)); >> + if ( vmentry_ctl & VM_ENTRY_LOAD_GUEST_CET ) >> + printk("SSP = 0x%016lx S_CET = 0x%016lx ISST = 0x%016lx\n", >> + vmr(GUEST_SSP), vmr(GUEST_S_CET), vmr(GUEST_ISST)); > ... the conditional here a little odd, but I expect the plan is > to have the various bits all set consistently once actually > enabling the functionality. TBH, the general behaviour here is poor. What happens now, as Xen does use CET itself, is that Xen's values propagate into guest context, and are written back into the VMCS on VMExit. There is no way to turn this behaviour off AFAICT. Therefore, we must not print the guest values when the vCPU isn't configured for CET, because otherwise we'd be rendering what is actually Xen state, in the guest state area. Once a VM is using CET, we'll have both VM_ENTRY_LOAD_GUEST_CET and VM_EXIT_LOAD_HOST_CET set. There is theoretically an optimisations to be had for a hypervisor not using CET, to only use the VM_ENTRY_LOAD_GUEST_CET control and leave VM_EXIT_LOAD_HOST_CET clear, but getting this optimisation wrong will leave the VMM running with guest controlled values. Personally, I think it was be a far safer interface for there only to be a single bit to control "switch CET state" or not. ~Andrew
On 27.04.2021 18:27, Andrew Cooper wrote: > On 27/04/2021 16:56, Jan Beulich wrote: >> On 26.04.2021 19:54, Andrew Cooper wrote: >>> VT-x has separate entry/exit control for loading guest/host state. Saving >>> guest state on vmexit is performed unconditionally. >> With the latter I find ... >> >>> --- a/xen/arch/x86/hvm/vmx/vmcs.c >>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >>> @@ -2014,6 +2014,9 @@ void vmcs_dump_vcpu(struct vcpu *v) >>> printk("RFLAGS=0x%08lx (0x%08lx) DR7 = 0x%016lx\n", >>> vmr(GUEST_RFLAGS), regs->rflags, >>> vmr(GUEST_DR7)); >>> + if ( vmentry_ctl & VM_ENTRY_LOAD_GUEST_CET ) >>> + printk("SSP = 0x%016lx S_CET = 0x%016lx ISST = 0x%016lx\n", >>> + vmr(GUEST_SSP), vmr(GUEST_S_CET), vmr(GUEST_ISST)); >> ... the conditional here a little odd, but I expect the plan is >> to have the various bits all set consistently once actually >> enabling the functionality. > > TBH, the general behaviour here is poor. > > What happens now, as Xen does use CET itself, is that Xen's values > propagate into guest context, and are written back into the VMCS on > VMExit. There is no way to turn this behaviour off AFAICT. > > Therefore, we must not print the guest values when the vCPU isn't > configured for CET, because otherwise we'd be rendering what is actually > Xen state, in the guest state area. > > Once a VM is using CET, we'll have both VM_ENTRY_LOAD_GUEST_CET and > VM_EXIT_LOAD_HOST_CET set. As I did assume then, so fair enough. > There is theoretically an optimisations to be had for a hypervisor not > using CET, to only use the VM_ENTRY_LOAD_GUEST_CET control and leave > VM_EXIT_LOAD_HOST_CET clear, but getting this optimisation wrong will > leave the VMM running with guest controlled values. > > Personally, I think it was be a far safer interface for there only to be > a single bit to control "switch CET state" or not. I agree, but this then goes for other state having multiple controls as well, I guess. I've been wondering whether this separation somehow helps them with the implementation of the guest-save, host-load, and guest-load steps. Jan
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index f9f9bc18cd..5849817630 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -2014,6 +2014,9 @@ void vmcs_dump_vcpu(struct vcpu *v) printk("RFLAGS=0x%08lx (0x%08lx) DR7 = 0x%016lx\n", vmr(GUEST_RFLAGS), regs->rflags, vmr(GUEST_DR7)); + if ( vmentry_ctl & VM_ENTRY_LOAD_GUEST_CET ) + printk("SSP = 0x%016lx S_CET = 0x%016lx ISST = 0x%016lx\n", + vmr(GUEST_SSP), vmr(GUEST_S_CET), vmr(GUEST_ISST)); printk("Sysenter RSP=%016lx CS:RIP=%04x:%016lx\n", vmr(GUEST_SYSENTER_ESP), vmr32(GUEST_SYSENTER_CS), vmr(GUEST_SYSENTER_EIP)); @@ -2057,6 +2060,9 @@ void vmcs_dump_vcpu(struct vcpu *v) vmr(HOST_GDTR_BASE), vmr(HOST_IDTR_BASE)); printk("CR0=%016lx CR3=%016lx CR4=%016lx\n", vmr(HOST_CR0), vmr(HOST_CR3), vmr(HOST_CR4)); + if ( vmexit_ctl & VM_EXIT_LOAD_HOST_CET ) + printk("SSP = 0x%016lx S_CET = 0x%016lx ISST = 0x%016lx\n", + vmr(HOST_SSP), vmr(HOST_S_CET), vmr(HOST_ISST)); printk("Sysenter RSP=%016lx CS:RIP=%04x:%016lx\n", vmr(HOST_SYSENTER_ESP), vmr32(HOST_SYSENTER_CS), vmr(HOST_SYSENTER_EIP)); diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 8073af323b..4c4246f190 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -46,7 +46,8 @@ struct ept_data { uint64_t mt:3, /* Memory Type. */ wl:3, /* Walk length -1. */ ad:1, /* Enable EPT A/D bits. */ - :5, /* rsvd. */ + sss:1, /* Supervisor Shadow Stack. */ + :4, /* rsvd. */ mfn:52; }; u64 eptp; @@ -238,6 +239,7 @@ extern u32 vmx_pin_based_exec_control; #define VM_EXIT_LOAD_HOST_EFER 0x00200000 #define VM_EXIT_SAVE_PREEMPT_TIMER 0x00400000 #define VM_EXIT_CLEAR_BNDCFGS 0x00800000 +#define VM_EXIT_LOAD_HOST_CET 0x10000000 extern u32 vmx_vmexit_control; #define VM_ENTRY_IA32E_MODE 0x00000200 @@ -247,6 +249,7 @@ extern u32 vmx_vmexit_control; #define VM_ENTRY_LOAD_GUEST_PAT 0x00004000 #define VM_ENTRY_LOAD_GUEST_EFER 0x00008000 #define VM_ENTRY_LOAD_BNDCFGS 0x00010000 +#define VM_ENTRY_LOAD_GUEST_CET 0x00100000 extern u32 vmx_vmentry_control; #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001 @@ -516,6 +519,9 @@ enum vmcs_field { GUEST_PENDING_DBG_EXCEPTIONS = 0x00006822, GUEST_SYSENTER_ESP = 0x00006824, GUEST_SYSENTER_EIP = 0x00006826, + GUEST_S_CET = 0x00006828, + GUEST_SSP = 0x0000682a, + GUEST_ISST = 0x0000682c, HOST_CR0 = 0x00006c00, HOST_CR3 = 0x00006c02, HOST_CR4 = 0x00006c04, @@ -528,6 +534,9 @@ enum vmcs_field { HOST_SYSENTER_EIP = 0x00006c12, HOST_RSP = 0x00006c14, HOST_RIP = 0x00006c16, + HOST_S_CET = 0x00006c18, + HOST_SSP = 0x00006c1a, + HOST_ISST = 0x00006c1c, }; #define VMCS_VPID_WIDTH 16
VT-x has separate entry/exit control for loading guest/host state. Saving guest state on vmexit is performed unconditionally. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> --- xen/arch/x86/hvm/vmx/vmcs.c | 6 ++++++ xen/include/asm-x86/hvm/vmx/vmcs.h | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-)