| Message ID | 20240509075423.156858-2-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/2] KVM: x86: Introduce KVM_{G,S}ET_ONE_REG uAPIs support | expand |
On Thu, May 09, 2024, Yang Weijiang wrote: > Enable guest shadow stack pointer(SSP) access interface with new uAPIs. > CET guest SSP is HW register which has corresponding VMCS field to save > /restore guest values when VM-{Exit,Entry} happens. KVM handles SSP as > a synthetic MSR for userspace access. > > Use a translation helper to set up mapping for SSP synthetic index and > KVM-internal MSR index so that userspace doesn't need to take care of > KVM's management for synthetic MSRs and avoid conflicts. > > Suggested-by: Sean Christopherson <seanjc@google.com> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/include/uapi/asm/kvm.h | 3 +++ > arch/x86/kvm/x86.c | 7 +++++++ > arch/x86/kvm/x86.h | 10 ++++++++++ > 3 files changed, 20 insertions(+) > > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h > index ca2a47a85fa1..81c8d9ea2e58 100644 > --- a/arch/x86/include/uapi/asm/kvm.h > +++ b/arch/x86/include/uapi/asm/kvm.h > @@ -420,6 +420,9 @@ struct kvm_x86_reg_id { > __u16 rsvd16; > }; > > +/* KVM synthetic MSR index staring from 0 */ > +#define MSR_KVM_GUEST_SSP 0 Do we want to have "SYNTHETIC" in the name? E.g. to try and differentiate from KVM's paravirtual MSRs? Hmm, but the PV MSRs are synthetic too. Maybe it's the MSR part that's bad, e.g. the whole point of these shenanigans is to let KVM use its internal MSR framework without exposing those details to userspace. So rather than, KVM_X86_REG_SYNTHETIC_MSR, what if we go with KVM_X86_REG_SYNTHETIC? And then this becomes something like KVM_SYNTHETIC_GUEST_SSP? Aha! And then to prepare for a future where we add synthetic registers that aren't routed through the MSR framework (which seems unlikely, but its trivially easy to handle, so why not): static int kvm_translate_synthetic_reg(struct kvm_x86_reg_id *reg) { switch (reg->index) { case MSR_KVM_GUEST_SSP: reg->type = KVM_X86_REG_MSR; reg->index = MSR_KVM_INTERNAL_GUEST_SSP; break; default: return -EINVAL; } return 0; } and then the caller would have slightly different ordering: if (id->type == KVM_X86_REG_SYNTHETIC_MSR) { r = kvm_translate_synthetic_msr(&id->index); if (r) break; } r = -EINVAL; if (id->type != KVM_X86_REG_MSR) break;
On 6/11/2024 9:17 AM, Sean Christopherson wrote: > On Thu, May 09, 2024, Yang Weijiang wrote: >> Enable guest shadow stack pointer(SSP) access interface with new uAPIs. >> CET guest SSP is HW register which has corresponding VMCS field to save >> /restore guest values when VM-{Exit,Entry} happens. KVM handles SSP as >> a synthetic MSR for userspace access. >> >> Use a translation helper to set up mapping for SSP synthetic index and >> KVM-internal MSR index so that userspace doesn't need to take care of >> KVM's management for synthetic MSRs and avoid conflicts. >> >> Suggested-by: Sean Christopherson <seanjc@google.com> >> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> >> --- >> arch/x86/include/uapi/asm/kvm.h | 3 +++ >> arch/x86/kvm/x86.c | 7 +++++++ >> arch/x86/kvm/x86.h | 10 ++++++++++ >> 3 files changed, 20 insertions(+) >> >> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h >> index ca2a47a85fa1..81c8d9ea2e58 100644 >> --- a/arch/x86/include/uapi/asm/kvm.h >> +++ b/arch/x86/include/uapi/asm/kvm.h >> @@ -420,6 +420,9 @@ struct kvm_x86_reg_id { >> __u16 rsvd16; >> }; >> >> +/* KVM synthetic MSR index staring from 0 */ >> +#define MSR_KVM_GUEST_SSP 0 > Do we want to have "SYNTHETIC" in the name? E.g. to try and differentiate from > KVM's paravirtual MSRs? > > Hmm, but the PV MSRs are synthetic too. Maybe it's the MSR part that's bad, e.g. > the whole point of these shenanigans is to let KVM use its internal MSR framework > without exposing those details to userspace. > > So rather than, KVM_X86_REG_SYNTHETIC_MSR, what if we go with KVM_X86_REG_SYNTHETIC? > And then this becomes something like KVM_SYNTHETIC_GUEST_SSP? Yes, makes sense for me. > > Aha! And then to prepare for a future where we add synthetic registers that > aren't routed through the MSR framework (which seems unlikely, but its trivially > easy to handle, so why not): > > static int kvm_translate_synthetic_reg(struct kvm_x86_reg_id *reg) > { > switch (reg->index) { > case MSR_KVM_GUEST_SSP: > reg->type = KVM_X86_REG_MSR; > reg->index = MSR_KVM_INTERNAL_GUEST_SSP; > break; > default: > return -EINVAL; > } > return 0; > } > > and then the caller would have slightly different ordering: > > if (id->type == KVM_X86_REG_SYNTHETIC_MSR) { > r = kvm_translate_synthetic_msr(&id->index); > if (r) > break; > } > > r = -EINVAL; > if (id->type != KVM_X86_REG_MSR) > break; I assume reg->type translation for GUEST_SSP is due to the fact it relies on CET common checking stuffs underneath for the register, i.e., it goes through existing MSR framework. But for future other synthetic MSRs, it needs to refactor the code here so that it could be routed into new handling. e.g.: if (id->type == KVM_X86_REG_MSR) go through MSR framework; else go through other new handling; But currently the new uAPIs are only for GUEST_SSP, so above suggested id->type check works. Does it make sense?
On Tue, Jun 11, 2024, Weijiang Yang wrote: > On 6/11/2024 9:17 AM, Sean Christopherson wrote: > > On Thu, May 09, 2024, Yang Weijiang wrote: > > Aha! And then to prepare for a future where we add synthetic registers that > > aren't routed through the MSR framework (which seems unlikely, but its trivially > > easy to handle, so why not): > > > > static int kvm_translate_synthetic_reg(struct kvm_x86_reg_id *reg) > > { > > switch (reg->index) { > > case MSR_KVM_GUEST_SSP: > > reg->type = KVM_X86_REG_MSR; > > reg->index = MSR_KVM_INTERNAL_GUEST_SSP; > > break; > > default: > > return -EINVAL; > > } > > return 0; > > } > > > > and then the caller would have slightly different ordering: > > > > if (id->type == KVM_X86_REG_SYNTHETIC_MSR) { > > r = kvm_translate_synthetic_msr(&id->index); > > if (r) > > break; > > } > > > > r = -EINVAL; > > if (id->type != KVM_X86_REG_MSR) > > break; > I assume reg->type translation for GUEST_SSP is due to the fact it relies on > CET common checking stuffs underneath for the register, i.e., it goes through > existing MSR framework. But for future other synthetic MSRs, it needs to Nit, other synthetic *registers*. > refactor the code here so that it could be routed into new handling. e.g.: > > if (id->type == KVM_X86_REG_MSR) > go through MSR framework; > else > go through other new handling; > > But currently the new uAPIs are only for GUEST_SSP, so above suggested > id->type check works. Does it make sense? Yep, we're on the same page.
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index ca2a47a85fa1..81c8d9ea2e58 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -420,6 +420,9 @@ struct kvm_x86_reg_id { __u16 rsvd16; }; +/* KVM synthetic MSR index staring from 0 */ +#define MSR_KVM_GUEST_SSP 0 + #define KVM_SYNC_X86_REGS (1UL << 0) #define KVM_SYNC_X86_SREGS (1UL << 1) #define KVM_SYNC_X86_EVENTS (1UL << 2) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d0054c52f24b..a970bd26ce2c 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5886,6 +5886,13 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu, static int kvm_translate_synthetic_msr(u32 *index) { + switch (*index) { + case MSR_KVM_GUEST_SSP: + *index = MSR_KVM_INTERNAL_GUEST_SSP; + break; + default: + return -EINVAL; + } return 0; } diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index a8b71803777b..6ac86a75aedc 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -57,6 +57,16 @@ void kvm_spurious_fault(void); #define KVM_SVM_DEFAULT_PLE_WINDOW_MAX USHRT_MAX #define KVM_SVM_DEFAULT_PLE_WINDOW 3000 +/* + * KVM's internal, non-ABI indices for synthetic MSRs. The values themselves + * are arbitrary and have no meaning, the only requirement is that they don't + * conflict with "real" MSRs that KVM supports. Use values at the uppper end + * of KVM's reserved paravirtual MSR range to minimize churn, i.e. these values + * will be usable until KVM exhausts its supply of paravirtual MSR indices. + */ + +#define MSR_KVM_INTERNAL_GUEST_SSP 0x4b564dff + static inline unsigned int __grow_ple_window(unsigned int val, unsigned int base, unsigned int modifier, unsigned int max) {
Enable guest shadow stack pointer(SSP) access interface with new uAPIs. CET guest SSP is HW register which has corresponding VMCS field to save /restore guest values when VM-{Exit,Entry} happens. KVM handles SSP as a synthetic MSR for userspace access. Use a translation helper to set up mapping for SSP synthetic index and KVM-internal MSR index so that userspace doesn't need to take care of KVM's management for synthetic MSRs and avoid conflicts. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- arch/x86/include/uapi/asm/kvm.h | 3 +++ arch/x86/kvm/x86.c | 7 +++++++ arch/x86/kvm/x86.h | 10 ++++++++++ 3 files changed, 20 insertions(+)