linux-kernel - Re: [PATCH v10 17/27] KVM: x86: Report KVM supported CET MSRs as to-be-saved

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 8 May 2024 15:00:12 +0800
From: "Yang, Weijiang" <weijiang.yang@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: <pbonzini@...hat.com>, <dave.hansen@...el.com>, <x86@...nel.org>,
	<kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<peterz@...radead.org>, <chao.gao@...el.com>, <rick.p.edgecombe@...el.com>,
	<mlevitsk@...hat.com>, <john.allen@....com>
Subject: Re: [PATCH v10 17/27] KVM: x86: Report KVM supported CET MSRs as
 to-be-saved

On 5/8/2024 1:27 AM, Sean Christopherson wrote:
> On Mon, May 06, 2024, Weijiang Yang wrote:
>> On 5/2/2024 6:40 AM, Sean Christopherson wrote:
>>> On Sun, Feb 18, 2024, Yang Weijiang wrote:

[...]

>> For the latter, IIUC KVM still needs to expose the index within the synthetic
>> namespace so that userspace can read/write the intended MSRs, of course not
>> expose the synthetic MSR index via existing uAPI,  But you said the "index"
>> exposed to userspace can simply  be '0' in this case, then how to distinguish
>> the synthetic MSRs in userspace and KVM? And how userspace can be aware of
>> the synthetic MSR index allocation in KVM?
> The idea is to have a synthetic index that is exposed to userspace, and a separate
> KVM-internal index for emulating accesses.  The value that is exposed to userspace
> can start at 0 and be a simple incrementing value as we add synthetic MSRs, as the
> .type == SYNTHETIC makes it impossible for the value to collide with a "real" MSR.
>
> Translating to a KVM-internal index is a hack to avoid having to plumb a 64-bit
> index into all the MSR code.  We could do that, i.e. pass the full kvm_x86_reg_id
> into the MSR helpers, but I'm not convinced it'd be worth the churn.  That said,
> I'm not opposed to the idea either, if others prefer that approach.
>
> E.g.
>
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 738c449e4f9e..21152796238a 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -420,6 +420,8 @@ struct kvm_x86_reg_id {
>          __u16 rsvd16;
>   };
>   
> +#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 f45cdd9d8c1f..1a9e1e0c9f49 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5990,6 +5990,19 @@ 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;
> +}
> +
>   long kvm_arch_vcpu_ioctl(struct file *filp,
>                           unsigned int ioctl, unsigned long arg)
>   {
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index cc585051d24b..3b5a038f5260 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -49,6 +49,15 @@ void kvm_spurious_fault(void);
>   #define KVM_FIRST_EMULATED_VMX_MSR     MSR_IA32_VMX_BASIC
>   #define KVM_LAST_EMULATED_VMX_MSR      MSR_IA32_VMX_VMFUNC
>   
> +/*
> + * 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
> +
>   #define KVM_DEFAULT_PLE_GAP            128
>   #define KVM_VMX_DEFAULT_PLE_WINDOW     4096
>   #define KVM_DEFAULT_PLE_WINDOW_GROW    2

OK, I'll post an RFC patch for this change, thanks a lot!