mbox series

[v10,00/27] Enable CET Virtualization

Message ID 20240219074733.122080-1-weijiang.yang@intel.com (mailing list archive)
Headers show
Series Enable CET Virtualization | expand

Message

Yang, Weijiang Feb. 19, 2024, 7:47 a.m. UTC
Control-flow Enforcement Technology (CET) is a kind of CPU feature used
to prevent Return/CALL/Jump-Oriented Programming (ROP/COP/JOP) attacks.
It provides two sub-features(SHSTK,IBT) to defend against ROP/COP/JOP
style control-flow subversion attacks.

Shadow Stack (SHSTK):
  A shadow stack is a second stack used exclusively for control transfer
  operations. The shadow stack is separate from the data/normal stack and
  can be enabled individually in user and kernel mode. When shadow stack
  is enabled, CALL pushes the return address on both the data and shadow
  stack. RET pops the return address from both stacks and compares them.
  If the return addresses from the two stacks do not match, the processor
  generates a #CP.

Indirect Branch Tracking (IBT):
  IBT introduces new instruction(ENDBRANCH)to mark valid target addresses of
  indirect branches (CALL, JMP etc...). If an indirect branch is executed
  and the next instruction is _not_ an ENDBRANCH, the processor generates a
  #CP. These instruction behaves as a NOP on platforms that doesn't support
  CET.

Dependency:
=====================
CET native series for user mode shadow stack has already been merged in v6.6
mainline kernel.

The first 7 kernel patches are prerequisites for this KVM patch series since
guest CET user mode and supervisor mode states depends on kernel FPU framework
to properly save/restore the states whenever FPU context switch is required,
e.g., after VM-Exit and before vCPU thread exits to userspace.

In this series, guest supervisor SHSTK mitigation solution isn't introduced
for Intel platform therefore guest SSS_CET bit of CPUID(0x7,1):EDX[bit18] is
cleared. Check SDM (Vol 1, Section 17.2.3) for details.

CET states management:
======================
KVM cooperates with host kernel FPU framework to manage guest CET registers.
With CET supervisor mode state support in this series, KVM can save/restore
full guest CET xsave-managed states.

CET user mode and supervisor mode xstates, i.e., MSR_IA32_{U_CET,PL3_SSP}
and MSR_IA32_PL{0,1,2}, depend on host FPU framework to swap guest and host
xstates. On VM-Exit, guest CET xstates are saved to guest fpu area and host
CET xstates are loaded from task/thread context before vCPU returns to
userspace, vice-versa on VM-Entry. See details in kvm_{load,put}_guest_fpu().
So guest CET xstates management depends on CET xstate bits(U_CET/S_CET bit)
set in host XSS MSR.

CET supervisor mode states are grouped into two categories : XSAVE-managed
and non-XSAVE-managed, the former includes MSR_IA32_PL{0,1,2}_SSP and are
controlled by CET supervisor mode bit(S_CET bit) in XSS, the later consists
of MSR_IA32_S_CET and MSR_IA32_INTR_SSP_TBL.

VMX introduces new VMCS fields, {GUEST|HOST}_{S_CET,SSP,INTR_SSP_TABL}, to
facilitate guest/host non-XSAVES-managed states. When VMX CET entry/exit load
bits are set, guest/host MSR_IA32_{S_CET,INTR_SSP_TBL,SSP} are loaded from
equivalent fields at VM-Exit/Entry. With these new fields, such supervisor
states require no addtional KVM save/reload actions.

Tests:
======================
This series passed basic CET user shadow stack test and kernel IBT test in L1
and L2 guest.
The patch series _has_ impact to existing vmx test cases in KVM-unit-tests,the
failures have been fixed here[1].
One new selftest app[2] is introduced for testing CET MSRs accessibilities.

Note, this series hasn't been tested on AMD platform yet.

To run user SHSTK test and kernel IBT test in guest, an CET capable platform
is required, e.g., Sapphire Rapids server, and follow below steps to build
the binaries:

1. Host kernel: Apply this series to mainline kernel (>= v6.6) and build.

2. Guest kernel: Pull kernel (>= v6.6), opt-in CONFIG_X86_KERNEL_IBT
and CONFIG_X86_USER_SHADOW_STACK options. Build with CET enabled gcc versions
(>= 8.5.0).

3. Apply CET QEMU patches[3] before build mainline QEMU.

Check kernel selftest test_shadow_stack_64 output:
[INFO]  new_ssp = 7f8c82100ff8, *new_ssp = 7f8c82101001
[INFO]  changing ssp from 7f8c82900ff0 to 7f8c82100ff8
[INFO]  ssp is now 7f8c82101000
[OK]    Shadow stack pivot
[OK]    Shadow stack faults
[INFO]  Corrupting shadow stack
[INFO]  Generated shadow stack violation successfully
[OK]    Shadow stack violation test
[INFO]  Gup read -> shstk access success
[INFO]  Gup write -> shstk access success
[INFO]  Violation from normal write
[INFO]  Gup read -> write access success
[INFO]  Violation from normal write
[INFO]  Gup write -> write access success
[INFO]  Cow gup write -> write access success
[OK]    Shadow gup test
[INFO]  Violation from shstk access
[OK]    mprotect() test
[SKIP]  Userfaultfd unavailable.
[OK]    32 bit test


Check kernel IBT with dmesg | grep CET:
CET detected: Indirect Branch Tracking enabled

Changes in v10:
=====================
1. Add Reviewed-by tags from Chao and Rick. [Chao, Rick]
2. Use two bit flags to check CET guarded instructions in KVM emulator. [Chao]
3. Refine reset handling of xsave-managed guest FPU states. [Chao]
4. Add nested CET MSR sync when entry/exit-load-bit is not set. [Chao]
5. Other minor changes per comments from Chao and Rick.
6. Rebased on https://github.com/kvm-x86/linux commit: c0f8b0752b09


[1]: KVM-unit-tests fixup:
https://lore.kernel.org/all/20230913235006.74172-1-weijiang.yang@intel.com/
[2]: Selftest for CET MSRs:
https://lore.kernel.org/all/20230914064201.85605-1-weijiang.yang@intel.com/
[3]: QEMU patch:
https://lore.kernel.org/all/20230720111445.99509-1-weijiang.yang@intel.com/
[4]: v9 patchset:
https://lore.kernel.org/all/20240124024200.102792-1-weijiang.yang@intel.com/


Patch 1-7:	Fixup patches for kernel xstate and enable CET supervisor xstate.
Patch 8-11:	Cleanup patches for KVM.
Patch 12-15:	Enable KVM XSS MSR support.
Patch 16:	Fault check for CR4.CET setting.
Patch 17:	Report CET MSRs to userspace.
Patch 18:	Introduce CET VMCS fields.
Patch 19:	Add SHSTK/IBT to KVM-governed framework.(to be deprecated)
Patch 20:	Emulate CET MSR access.
Patch 21:	Handle SSP at entry/exit to SMM.
Patch 22:	Set up CET MSR interception.
Patch 23:	Initialize host constant supervisor state.
Patch 24:	Enable CET virtualization settings.
Patch 25-26:	Add CET nested support.
Patch 27:	KVM emulation handling for branch instructions


Sean Christopherson (4):
  x86/fpu/xstate: Always preserve non-user xfeatures/flags in
    __state_perm
  KVM: x86: Rework cpuid_get_supported_xcr0() to operate on vCPU data
  KVM: x86: Report XSS as to-be-saved if there are supported features
  KVM: x86: Load guest FPU state when access XSAVE-managed MSRs

Yang Weijiang (23):
  x86/fpu/xstate: Refine CET user xstate bit enabling
  x86/fpu/xstate: Add CET supervisor mode state support
  x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
  x86/fpu/xstate: Introduce fpu_guest_cfg for guest FPU configuration
  x86/fpu/xstate: Create guest fpstate with guest specific config
  x86/fpu/xstate: Warn if kernel dynamic xfeatures detected in normal
    fpstate
  KVM: x86: Rename kvm_{g,s}et_msr()* to menifest emulation operations
  KVM: x86: Refine xsave-managed guest register/MSR reset handling
  KVM: x86: Add kvm_msr_{read,write}() helpers
  KVM: x86: Refresh CPUID on write to guest MSR_IA32_XSS
  KVM: x86: Initialize kvm_caps.supported_xss
  KVM: x86: Add fault checks for guest CR4.CET setting
  KVM: x86: Report KVM supported CET MSRs as to-be-saved
  KVM: VMX: Introduce CET VMCS fields and control bits
  KVM: x86: Use KVM-governed feature framework to track "SHSTK/IBT
    enabled"
  KVM: VMX: Emulate read and write to CET MSRs
  KVM: x86: Save and reload SSP to/from SMRAM
  KVM: VMX: Set up interception for CET MSRs
  KVM: VMX: Set host constant supervisor states to VMCS fields
  KVM: x86: Enable CET virtualization for VMX and advertise to userspace
  KVM: nVMX: Introduce new VMX_BASIC bit for event error_code delivery
    to L1
  KVM: nVMX: Enable CET support for nested guest
  KVM: x86: Don't emulate instructions guarded by CET

 arch/x86/include/asm/fpu/types.h     |  16 +-
 arch/x86/include/asm/fpu/xstate.h    |  11 +-
 arch/x86/include/asm/kvm_host.h      |  12 +-
 arch/x86/include/asm/msr-index.h     |   1 +
 arch/x86/include/asm/vmx.h           |   8 +
 arch/x86/include/uapi/asm/kvm_para.h |   1 +
 arch/x86/kernel/fpu/core.c           |  53 +++--
 arch/x86/kernel/fpu/xstate.c         |  44 ++++-
 arch/x86/kernel/fpu/xstate.h         |   3 +
 arch/x86/kvm/cpuid.c                 |  80 ++++++--
 arch/x86/kvm/emulate.c               |  46 +++--
 arch/x86/kvm/governed_features.h     |   2 +
 arch/x86/kvm/smm.c                   |  12 +-
 arch/x86/kvm/smm.h                   |   2 +-
 arch/x86/kvm/vmx/capabilities.h      |  10 +
 arch/x86/kvm/vmx/nested.c            | 120 ++++++++++--
 arch/x86/kvm/vmx/nested.h            |   5 +
 arch/x86/kvm/vmx/vmcs12.c            |   6 +
 arch/x86/kvm/vmx/vmcs12.h            |  14 +-
 arch/x86/kvm/vmx/vmx.c               | 112 ++++++++++-
 arch/x86/kvm/vmx/vmx.h               |   9 +-
 arch/x86/kvm/x86.c                   | 280 ++++++++++++++++++++++++---
 arch/x86/kvm/x86.h                   |  28 +++
 23 files changed, 761 insertions(+), 114 deletions(-)


base-commit: c0f8b0752b0988e5116c78e8b6c3cfdf89806e45

Comments

Yang, Weijiang March 6, 2024, 2:44 p.m. UTC | #1
Hi, Dave,

Could you kindly review the kernel patches(patch 1-7) at your convenience?
Rick has added RB tags on these patches, so I'd get your opinions on them.

Thanks a lot!

On 2/19/2024 3:47 PM, Yang Weijiang wrote:
> Control-flow Enforcement Technology (CET) is a kind of CPU feature used
> to prevent Return/CALL/Jump-Oriented Programming (ROP/COP/JOP) attacks.
> It provides two sub-features(SHSTK,IBT) to defend against ROP/COP/JOP
> style control-flow subversion attacks.
>
> Shadow Stack (SHSTK):
>    A shadow stack is a second stack used exclusively for control transfer
>    operations. The shadow stack is separate from the data/normal stack and
>    can be enabled individually in user and kernel mode. When shadow stack
>    is enabled, CALL pushes the return address on both the data and shadow
>    stack. RET pops the return address from both stacks and compares them.
>    If the return addresses from the two stacks do not match, the processor
>    generates a #CP.
>
> Indirect Branch Tracking (IBT):
>    IBT introduces new instruction(ENDBRANCH)to mark valid target addresses of
>    indirect branches (CALL, JMP etc...). If an indirect branch is executed
>    and the next instruction is _not_ an ENDBRANCH, the processor generates a
>    #CP. These instruction behaves as a NOP on platforms that doesn't support
>    CET.
>
> Dependency:
> =====================
> CET native series for user mode shadow stack has already been merged in v6.6
> mainline kernel.
>
> The first 7 kernel patches are prerequisites for this KVM patch series since
> guest CET user mode and supervisor mode states depends on kernel FPU framework
> to properly save/restore the states whenever FPU context switch is required,
> e.g., after VM-Exit and before vCPU thread exits to userspace.
>
> In this series, guest supervisor SHSTK mitigation solution isn't introduced
> for Intel platform therefore guest SSS_CET bit of CPUID(0x7,1):EDX[bit18] is
> cleared. Check SDM (Vol 1, Section 17.2.3) for details.
>
> CET states management:
> ======================
> KVM cooperates with host kernel FPU framework to manage guest CET registers.
> With CET supervisor mode state support in this series, KVM can save/restore
> full guest CET xsave-managed states.
>
> CET user mode and supervisor mode xstates, i.e., MSR_IA32_{U_CET,PL3_SSP}
> and MSR_IA32_PL{0,1,2}, depend on host FPU framework to swap guest and host
> xstates. On VM-Exit, guest CET xstates are saved to guest fpu area and host
> CET xstates are loaded from task/thread context before vCPU returns to
> userspace, vice-versa on VM-Entry. See details in kvm_{load,put}_guest_fpu().
> So guest CET xstates management depends on CET xstate bits(U_CET/S_CET bit)
> set in host XSS MSR.
>
> CET supervisor mode states are grouped into two categories : XSAVE-managed
> and non-XSAVE-managed, the former includes MSR_IA32_PL{0,1,2}_SSP and are
> controlled by CET supervisor mode bit(S_CET bit) in XSS, the later consists
> of MSR_IA32_S_CET and MSR_IA32_INTR_SSP_TBL.
>
> VMX introduces new VMCS fields, {GUEST|HOST}_{S_CET,SSP,INTR_SSP_TABL}, to
> facilitate guest/host non-XSAVES-managed states. When VMX CET entry/exit load
> bits are set, guest/host MSR_IA32_{S_CET,INTR_SSP_TBL,SSP} are loaded from
> equivalent fields at VM-Exit/Entry. With these new fields, such supervisor
> states require no addtional KVM save/reload actions.
>
> Tests:
> ======================
> This series passed basic CET user shadow stack test and kernel IBT test in L1
> and L2 guest.
> The patch series _has_ impact to existing vmx test cases in KVM-unit-tests,the
> failures have been fixed here[1].
> One new selftest app[2] is introduced for testing CET MSRs accessibilities.
>
> Note, this series hasn't been tested on AMD platform yet.
>
> To run user SHSTK test and kernel IBT test in guest, an CET capable platform
> is required, e.g., Sapphire Rapids server, and follow below steps to build
> the binaries:
>
> 1. Host kernel: Apply this series to mainline kernel (>= v6.6) and build.
>
> 2. Guest kernel: Pull kernel (>= v6.6), opt-in CONFIG_X86_KERNEL_IBT
> and CONFIG_X86_USER_SHADOW_STACK options. Build with CET enabled gcc versions
> (>= 8.5.0).
>
> 3. Apply CET QEMU patches[3] before build mainline QEMU.
>
> Check kernel selftest test_shadow_stack_64 output:
> [INFO]  new_ssp = 7f8c82100ff8, *new_ssp = 7f8c82101001
> [INFO]  changing ssp from 7f8c82900ff0 to 7f8c82100ff8
> [INFO]  ssp is now 7f8c82101000
> [OK]    Shadow stack pivot
> [OK]    Shadow stack faults
> [INFO]  Corrupting shadow stack
> [INFO]  Generated shadow stack violation successfully
> [OK]    Shadow stack violation test
> [INFO]  Gup read -> shstk access success
> [INFO]  Gup write -> shstk access success
> [INFO]  Violation from normal write
> [INFO]  Gup read -> write access success
> [INFO]  Violation from normal write
> [INFO]  Gup write -> write access success
> [INFO]  Cow gup write -> write access success
> [OK]    Shadow gup test
> [INFO]  Violation from shstk access
> [OK]    mprotect() test
> [SKIP]  Userfaultfd unavailable.
> [OK]    32 bit test
>
>
> Check kernel IBT with dmesg | grep CET:
> CET detected: Indirect Branch Tracking enabled
>
> Changes in v10:
> =====================
> 1. Add Reviewed-by tags from Chao and Rick. [Chao, Rick]
> 2. Use two bit flags to check CET guarded instructions in KVM emulator. [Chao]
> 3. Refine reset handling of xsave-managed guest FPU states. [Chao]
> 4. Add nested CET MSR sync when entry/exit-load-bit is not set. [Chao]
> 5. Other minor changes per comments from Chao and Rick.
> 6. Rebased on https://github.com/kvm-x86/linux commit: c0f8b0752b09
>
>
> [1]: KVM-unit-tests fixup:
> https://lore.kernel.org/all/20230913235006.74172-1-weijiang.yang@intel.com/
> [2]: Selftest for CET MSRs:
> https://lore.kernel.org/all/20230914064201.85605-1-weijiang.yang@intel.com/
> [3]: QEMU patch:
> https://lore.kernel.org/all/20230720111445.99509-1-weijiang.yang@intel.com/
> [4]: v9 patchset:
> https://lore.kernel.org/all/20240124024200.102792-1-weijiang.yang@intel.com/
>
>
> Patch 1-7:	Fixup patches for kernel xstate and enable CET supervisor xstate.
> Patch 8-11:	Cleanup patches for KVM.
> Patch 12-15:	Enable KVM XSS MSR support.
> Patch 16:	Fault check for CR4.CET setting.
> Patch 17:	Report CET MSRs to userspace.
> Patch 18:	Introduce CET VMCS fields.
> Patch 19:	Add SHSTK/IBT to KVM-governed framework.(to be deprecated)
> Patch 20:	Emulate CET MSR access.
> Patch 21:	Handle SSP at entry/exit to SMM.
> Patch 22:	Set up CET MSR interception.
> Patch 23:	Initialize host constant supervisor state.
> Patch 24:	Enable CET virtualization settings.
> Patch 25-26:	Add CET nested support.
> Patch 27:	KVM emulation handling for branch instructions
>
>
> Sean Christopherson (4):
>    x86/fpu/xstate: Always preserve non-user xfeatures/flags in
>      __state_perm
>    KVM: x86: Rework cpuid_get_supported_xcr0() to operate on vCPU data
>    KVM: x86: Report XSS as to-be-saved if there are supported features
>    KVM: x86: Load guest FPU state when access XSAVE-managed MSRs
>
> Yang Weijiang (23):
>    x86/fpu/xstate: Refine CET user xstate bit enabling
>    x86/fpu/xstate: Add CET supervisor mode state support
>    x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
>    x86/fpu/xstate: Introduce fpu_guest_cfg for guest FPU configuration
>    x86/fpu/xstate: Create guest fpstate with guest specific config
>    x86/fpu/xstate: Warn if kernel dynamic xfeatures detected in normal
>      fpstate
>    KVM: x86: Rename kvm_{g,s}et_msr()* to menifest emulation operations
>    KVM: x86: Refine xsave-managed guest register/MSR reset handling
>    KVM: x86: Add kvm_msr_{read,write}() helpers
>    KVM: x86: Refresh CPUID on write to guest MSR_IA32_XSS
>    KVM: x86: Initialize kvm_caps.supported_xss
>    KVM: x86: Add fault checks for guest CR4.CET setting
>    KVM: x86: Report KVM supported CET MSRs as to-be-saved
>    KVM: VMX: Introduce CET VMCS fields and control bits
>    KVM: x86: Use KVM-governed feature framework to track "SHSTK/IBT
>      enabled"
>    KVM: VMX: Emulate read and write to CET MSRs
>    KVM: x86: Save and reload SSP to/from SMRAM
>    KVM: VMX: Set up interception for CET MSRs
>    KVM: VMX: Set host constant supervisor states to VMCS fields
>    KVM: x86: Enable CET virtualization for VMX and advertise to userspace
>    KVM: nVMX: Introduce new VMX_BASIC bit for event error_code delivery
>      to L1
>    KVM: nVMX: Enable CET support for nested guest
>    KVM: x86: Don't emulate instructions guarded by CET
>
>   arch/x86/include/asm/fpu/types.h     |  16 +-
>   arch/x86/include/asm/fpu/xstate.h    |  11 +-
>   arch/x86/include/asm/kvm_host.h      |  12 +-
>   arch/x86/include/asm/msr-index.h     |   1 +
>   arch/x86/include/asm/vmx.h           |   8 +
>   arch/x86/include/uapi/asm/kvm_para.h |   1 +
>   arch/x86/kernel/fpu/core.c           |  53 +++--
>   arch/x86/kernel/fpu/xstate.c         |  44 ++++-
>   arch/x86/kernel/fpu/xstate.h         |   3 +
>   arch/x86/kvm/cpuid.c                 |  80 ++++++--
>   arch/x86/kvm/emulate.c               |  46 +++--
>   arch/x86/kvm/governed_features.h     |   2 +
>   arch/x86/kvm/smm.c                   |  12 +-
>   arch/x86/kvm/smm.h                   |   2 +-
>   arch/x86/kvm/vmx/capabilities.h      |  10 +
>   arch/x86/kvm/vmx/nested.c            | 120 ++++++++++--
>   arch/x86/kvm/vmx/nested.h            |   5 +
>   arch/x86/kvm/vmx/vmcs12.c            |   6 +
>   arch/x86/kvm/vmx/vmcs12.h            |  14 +-
>   arch/x86/kvm/vmx/vmx.c               | 112 ++++++++++-
>   arch/x86/kvm/vmx/vmx.h               |   9 +-
>   arch/x86/kvm/x86.c                   | 280 ++++++++++++++++++++++++---
>   arch/x86/kvm/x86.h                   |  28 +++
>   23 files changed, 761 insertions(+), 114 deletions(-)
>
>
> base-commit: c0f8b0752b0988e5116c78e8b6c3cfdf89806e45
Sean Christopherson May 1, 2024, 11:27 p.m. UTC | #2
On Sun, Feb 18, 2024, Yang Weijiang wrote:
> Sean Christopherson (4):
>   x86/fpu/xstate: Always preserve non-user xfeatures/flags in
>     __state_perm
>   KVM: x86: Rework cpuid_get_supported_xcr0() to operate on vCPU data
>   KVM: x86: Report XSS as to-be-saved if there are supported features
>   KVM: x86: Load guest FPU state when access XSAVE-managed MSRs
> 
> Yang Weijiang (23):
>   x86/fpu/xstate: Refine CET user xstate bit enabling
>   x86/fpu/xstate: Add CET supervisor mode state support
>   x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
>   x86/fpu/xstate: Introduce fpu_guest_cfg for guest FPU configuration
>   x86/fpu/xstate: Create guest fpstate with guest specific config
>   x86/fpu/xstate: Warn if kernel dynamic xfeatures detected in normal
>     fpstate
>   KVM: x86: Rename kvm_{g,s}et_msr()* to menifest emulation operations
>   KVM: x86: Refine xsave-managed guest register/MSR reset handling
>   KVM: x86: Add kvm_msr_{read,write}() helpers
>   KVM: x86: Refresh CPUID on write to guest MSR_IA32_XSS
>   KVM: x86: Initialize kvm_caps.supported_xss
>   KVM: x86: Add fault checks for guest CR4.CET setting
>   KVM: x86: Report KVM supported CET MSRs as to-be-saved
>   KVM: VMX: Introduce CET VMCS fields and control bits
>   KVM: x86: Use KVM-governed feature framework to track "SHSTK/IBT
>     enabled"
>   KVM: VMX: Emulate read and write to CET MSRs
>   KVM: x86: Save and reload SSP to/from SMRAM
>   KVM: VMX: Set up interception for CET MSRs
>   KVM: VMX: Set host constant supervisor states to VMCS fields
>   KVM: x86: Enable CET virtualization for VMX and advertise to userspace
>   KVM: nVMX: Introduce new VMX_BASIC bit for event error_code delivery
>     to L1
>   KVM: nVMX: Enable CET support for nested guest
>   KVM: x86: Don't emulate instructions guarded by CET

A decent number of comments, but almost all of them are quite minor.  The big
open is how to handle save/restore of SSP from userspace.

Instead of spinning a full v10, maybe send an RFC for KVM_{G,S}ET_ONE_REG idea?
That will make it easier to review, and if you delay v11 a bit, I should be able
to get various series applied that have minor conflicts/dependencies, e.g. the
MSR access and the kvm_host series.
Yang, Weijiang May 6, 2024, 9:31 a.m. UTC | #3
On 5/2/2024 7:27 AM, Sean Christopherson wrote:
> On Sun, Feb 18, 2024, Yang Weijiang wrote:
>> Sean Christopherson (4):
>>    x86/fpu/xstate: Always preserve non-user xfeatures/flags in
>>      __state_perm
>>    KVM: x86: Rework cpuid_get_supported_xcr0() to operate on vCPU data
>>    KVM: x86: Report XSS as to-be-saved if there are supported features
>>    KVM: x86: Load guest FPU state when access XSAVE-managed MSRs
>>
>> Yang Weijiang (23):
>>    x86/fpu/xstate: Refine CET user xstate bit enabling
>>    x86/fpu/xstate: Add CET supervisor mode state support
>>    x86/fpu/xstate: Introduce XFEATURE_MASK_KERNEL_DYNAMIC xfeature set
>>    x86/fpu/xstate: Introduce fpu_guest_cfg for guest FPU configuration
>>    x86/fpu/xstate: Create guest fpstate with guest specific config
>>    x86/fpu/xstate: Warn if kernel dynamic xfeatures detected in normal
>>      fpstate
>>    KVM: x86: Rename kvm_{g,s}et_msr()* to menifest emulation operations
>>    KVM: x86: Refine xsave-managed guest register/MSR reset handling
>>    KVM: x86: Add kvm_msr_{read,write}() helpers
>>    KVM: x86: Refresh CPUID on write to guest MSR_IA32_XSS
>>    KVM: x86: Initialize kvm_caps.supported_xss
>>    KVM: x86: Add fault checks for guest CR4.CET setting
>>    KVM: x86: Report KVM supported CET MSRs as to-be-saved
>>    KVM: VMX: Introduce CET VMCS fields and control bits
>>    KVM: x86: Use KVM-governed feature framework to track "SHSTK/IBT
>>      enabled"
>>    KVM: VMX: Emulate read and write to CET MSRs
>>    KVM: x86: Save and reload SSP to/from SMRAM
>>    KVM: VMX: Set up interception for CET MSRs
>>    KVM: VMX: Set host constant supervisor states to VMCS fields
>>    KVM: x86: Enable CET virtualization for VMX and advertise to userspace
>>    KVM: nVMX: Introduce new VMX_BASIC bit for event error_code delivery
>>      to L1
>>    KVM: nVMX: Enable CET support for nested guest
>>    KVM: x86: Don't emulate instructions guarded by CET
> A decent number of comments, but almost all of them are quite minor.  The big
> open is how to handle save/restore of SSP from userspace.
>
> Instead of spinning a full v10, maybe send an RFC for KVM_{G,S}ET_ONE_REG idea?

OK, I'll send an RFC patch after relevant discussion is settled.

> That will make it easier to review, and if you delay v11 a bit, I should be able
> to get various series applied that have minor conflicts/dependencies, e.g. the
> MSR access and the kvm_host series.
I can wait until the series landed in x86-kvm tree.
Appreciated for your review and comments!
Edgecombe, Rick P Nov. 5, 2024, 6:25 p.m. UTC | #4
On Mon, 2024-05-06 at 17:31 +0800, Yang, Weijiang wrote:
> > A decent number of comments, but almost all of them are quite minor.  The
> > big
> > open is how to handle save/restore of SSP from userspace.
> > 
> > Instead of spinning a full v10, maybe send an RFC for KVM_{G,S}ET_ONE_REG
> > idea?
> 
> OK, I'll send an RFC patch after relevant discussion is settled.
> 
> > That will make it easier to review, and if you delay v11 a bit, I should be
> > able
> > to get various series applied that have minor conflicts/dependencies, e.g.
> > the
> > MSR access and the kvm_host series.
> I can wait until the series landed in x86-kvm tree.
> Appreciated for your review and comments!

It looks like this series is very close. Since this v10, there was some
discussion on the FPU part that seemed settled:
https://lore.kernel.org/lkml/1c2fd06e-2e97-4724-80ab-8695aa4334e7@intel.com/

Then there was also some discussion on the synthetic MSR solution, which seemed
prescriptive enough:
https://lore.kernel.org/kvm/20240509075423.156858-1-weijiang.yang@intel.com/

Weijiang, had you started a v2 on the synthetic MSR series? Where did you get to
on incorporating the other small v10 feedback?
Yang, Weijiang Nov. 6, 2024, 1:45 a.m. UTC | #5
On 11/6/2024 2:25 AM, Edgecombe, Rick P wrote:
> On Mon, 2024-05-06 at 17:31 +0800, Yang, Weijiang wrote:
>>> A decent number of comments, but almost all of them are quite minor.  The
>>> big
>>> open is how to handle save/restore of SSP from userspace.
>>>
>>> Instead of spinning a full v10, maybe send an RFC for KVM_{G,S}ET_ONE_REG
>>> idea?
>> OK, I'll send an RFC patch after relevant discussion is settled.
>>
>>> That will make it easier to review, and if you delay v11 a bit, I should be
>>> able
>>> to get various series applied that have minor conflicts/dependencies, e.g.
>>> the
>>> MSR access and the kvm_host series.
>> I can wait until the series landed in x86-kvm tree.
>> Appreciated for your review and comments!
> It looks like this series is very close. Since this v10, there was some
> discussion on the FPU part that seemed settled:
> https://lore.kernel.org/lkml/1c2fd06e-2e97-4724-80ab-8695aa4334e7@intel.com/

Hi, Rick,
I have an internal branch to hold a v11 candidate for this series, which resolved Sean's comments
for this v10, waiting for someone to take over and continue the upstream work.

>
> Then there was also some discussion on the synthetic MSR solution, which seemed
> prescriptive enough:
> https://lore.kernel.org/kvm/20240509075423.156858-1-weijiang.yang@intel.com/
>
> Weijiang, had you started a v2 on the synthetic MSR series? Where did you get to
> on incorporating the other small v10 feedback?

Yes, Sean's review feedback for v1 is also included in my above v11 candidate.
Edgecombe, Rick P Nov. 6, 2024, 3:20 a.m. UTC | #6
On Wed, 2024-11-06 at 09:45 +0800, Yang, Weijiang wrote:
> > > Appreciated for your review and comments!
> > It looks like this series is very close. Since this v10, there was some
> > discussion on the FPU part that seemed settled:
> > https://lore.kernel.org/lkml/1c2fd06e-2e97-4724-80ab-8695aa4334e7@intel.com/
> 
> Hi, Rick,
> I have an internal branch to hold a v11 candidate for this series, which
> resolved Sean's comments
> for this v10, waiting for someone to take over and continue the upstream work.
> 
> > 
> > Then there was also some discussion on the synthetic MSR solution, which
> > seemed
> > prescriptive enough:
> > https://lore.kernel.org/kvm/20240509075423.156858-1-weijiang.yang@intel.com/
> > 
> > Weijiang, had you started a v2 on the synthetic MSR series? Where did you
> > get to
> > on incorporating the other small v10 feedback?
> 
> Yes, Sean's review feedback for v1 is also included in my above v11 candidate.

Nice, sounds like another version (which could be the last) is basically ready
to go. Please let me know if it gets stuck for lack of someone to take it over.
Sean Christopherson Nov. 6, 2024, 4:32 p.m. UTC | #7
On Wed, Nov 06, 2024, Rick P Edgecombe wrote:
> On Wed, 2024-11-06 at 09:45 +0800, Yang, Weijiang wrote:
> > > > Appreciated for your review and comments!
> > > It looks like this series is very close. Since this v10, there was some
> > > discussion on the FPU part that seemed settled:
> > > https://lore.kernel.org/lkml/1c2fd06e-2e97-4724-80ab-8695aa4334e7@intel.com/
> > 
> > Hi, Rick,
> > I have an internal branch to hold a v11 candidate for this series, which
> > resolved Sean's comments
> > for this v10, waiting for someone to take over and continue the upstream work.
> > 
> > > 
> > > Then there was also some discussion on the synthetic MSR solution, which
> > > seemed
> > > prescriptive enough:
> > > https://lore.kernel.org/kvm/20240509075423.156858-1-weijiang.yang@intel.com/
> > > 
> > > Weijiang, had you started a v2 on the synthetic MSR series? Where did you
> > > get to
> > > on incorporating the other small v10 feedback?
> > 
> > Yes, Sean's review feedback for v1 is also included in my above v11 candidate.
> 
> Nice, sounds like another version (which could be the last) is basically ready
> to go. Please let me know if it gets stuck for lack of someone to take it over.

Or me, if Intel can't conjure up the resource.  I have spent way, way too much
time and effort on CET virtualization to let it die on the vine :-)
Edgecombe, Rick P Nov. 7, 2024, 2:05 a.m. UTC | #8
On Wed, 2024-11-06 at 08:32 -0800, Sean Christopherson wrote:
> > 
> > Nice, sounds like another version (which could be the last) is basically
> > ready
> > to go. Please let me know if it gets stuck for lack of someone to take it
> > over.
> 
> Or me, if Intel can't conjure up the resource.  I have spent way, way too much
> time and effort on CET virtualization to let it die on the vine :-)

Sounds like Weijiang will hand if off to someone and be available for questions.
So someone will post a v11 with a proper transition.

We still need to collect an ack from Dave on the FPU pieces. I'll see if I can
restart that in the meantime.
Chao Gao Nov. 12, 2024, 12:33 a.m. UTC | #9
On Wed, Nov 06, 2024 at 08:32:19AM -0800, Sean Christopherson wrote:
>On Wed, Nov 06, 2024, Rick P Edgecombe wrote:
>> On Wed, 2024-11-06 at 09:45 +0800, Yang, Weijiang wrote:
>> > > > Appreciated for your review and comments!
>> > > It looks like this series is very close. Since this v10, there was some
>> > > discussion on the FPU part that seemed settled:
>> > > https://lore.kernel.org/lkml/1c2fd06e-2e97-4724-80ab-8695aa4334e7@intel.com/
>> > 
>> > Hi, Rick,
>> > I have an internal branch to hold a v11 candidate for this series, which
>> > resolved Sean's comments
>> > for this v10, waiting for someone to take over and continue the upstream work.
>> > 
>> > > 
>> > > Then there was also some discussion on the synthetic MSR solution, which
>> > > seemed
>> > > prescriptive enough:
>> > > https://lore.kernel.org/kvm/20240509075423.156858-1-weijiang.yang@intel.com/
>> > > 
>> > > Weijiang, had you started a v2 on the synthetic MSR series? Where did you
>> > > get to
>> > > on incorporating the other small v10 feedback?
>> > 
>> > Yes, Sean's review feedback for v1 is also included in my above v11 candidate.
>> 
>> Nice, sounds like another version (which could be the last) is basically ready
>> to go. Please let me know if it gets stuck for lack of someone to take it over.
>
>Or me, if Intel can't conjure up the resource.  I have spent way, way too much
>time and effort on CET virtualization to let it die on the vine :-)

Just FYI, I will take it over; I plan to submit v11 after x86 fpu changes [*]
are settled.

[*]: https://lore.kernel.org/kvm/67c5a358-0e40-4b2f-b679-33dd0dfe73fb@intel.com/
Edgecombe, Rick P Nov. 12, 2024, 8:03 p.m. UTC | #10
On Tue, 2024-11-12 at 08:33 +0800, Chao Gao wrote:
> > 
> > Or me, if Intel can't conjure up the resource.  I have spent way, way too
> > much
> > time and effort on CET virtualization to let it die on the vine :-)
> 
> Just FYI, I will take it over; I plan to submit v11 after x86 fpu changes [*]
> are settled.
> 
> [*]:
> https://lore.kernel.org/kvm/67c5a358-0e40-4b2f-b679-33dd0dfe73fb@intel.com/

This series has some CET intersection:
https://lore.kernel.org/kvm/20241001050110.3643764-13-xin@zytor.com/

Have you checked if there needs to be any changes to either?
Chao Gao Nov. 13, 2024, 10:53 a.m. UTC | #11
On Wed, Nov 13, 2024 at 04:03:11AM +0800, Edgecombe, Rick P wrote:
>On Tue, 2024-11-12 at 08:33 +0800, Chao Gao wrote:
>> > 
>> > Or me, if Intel can't conjure up the resource.  I have spent way, way too
>> > much
>> > time and effort on CET virtualization to let it die on the vine :-)
>> 
>> Just FYI, I will take it over; I plan to submit v11 after x86 fpu changes [*]
>> are settled.
>> 
>> [*]:
>> https://lore.kernel.org/kvm/67c5a358-0e40-4b2f-b679-33dd0dfe73fb@intel.com/
>
>This series has some CET intersection:
>https://lore.kernel.org/kvm/20241001050110.3643764-13-xin@zytor.com/
>
>Have you checked if there needs to be any changes to either?

The intersection was already discussed at
https://lore.kernel.org/kvm/496a337d-a20d-4122-93a9-1520779c6d2d@zytor.com/

The plan is to merge the CET KVM series first. Then, the FRED KVM series will
address the intersection by:

1. Allowing guest access to the IA32_FRED_SSP0 MSR if either FRED or CET is
   exposed to the guest.

2. Intercepting the IA32_FRED_SSP0 MSR if CET is not exposed to the guest. This
   way, every access to that MSR is trapped and emulated by KVM when only FRED
   is exposed. Then KVM needn't manually save that MSR or save it through the
   CET_S XSAVE state when the vCPU is being scheduled out.