diff mbox series

[3/3] x86/VT-x: Enumeration for CET

Message ID 20210426175421.30497-4-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86: Initial pieces for guest CET support | expand

Commit Message

Andrew Cooper April 26, 2021, 5:54 p.m. UTC
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(-)

Comments

Jan Beulich April 27, 2021, 3:56 p.m. UTC | #1
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
Andrew Cooper April 27, 2021, 4:27 p.m. UTC | #2
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
Jan Beulich April 28, 2021, 9:18 a.m. UTC | #3
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 mbox series

Patch

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