| 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
| ||
|
Message-ID: <aLIjJ9I1Swf35HPT@google.com>
Date: Fri, 29 Aug 2025 15:01:11 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: Xiaoyao Li <xiaoyao.li@...el.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com, john.allen@....com,
mingo@...hat.com, minipli@...ecurity.net, mlevitsk@...hat.com,
pbonzini@...hat.com, rick.p.edgecombe@...el.com, tglx@...utronix.de,
weijiang.yang@...el.com, x86@...nel.org, xin@...or.com
Subject: Re: [PATCH v13 01/21] KVM: x86: Introduce KVM_{G,S}ET_ONE_REG uAPIs support
On Fri, Aug 29, 2025, Chao Gao wrote:
> On Thu, Aug 28, 2025 at 08:58:07PM +0800, Xiaoyao Li wrote:
> >On 8/21/2025 9:30 PM, Chao Gao wrote:
> >> From: Yang Weijiang <weijiang.yang@...el.com>
> >>
> >> Enable KVM_{G,S}ET_ONE_REG uAPIs so that userspace can access HW MSR or
> >> KVM synthetic MSR through it.
> >>
> >> In CET KVM series [1], KVM "steals" an MSR from PV MSR space and access
> >> it via KVM_{G,S}ET_MSRs uAPIs, but the approach pollutes PV MSR space
> >> and hides the difference of synthetic MSRs and normal HW defined MSRs.
> >>
> >> Now carve out a separate room in KVM-customized MSR address space for
> >> synthetic MSRs. The synthetic MSRs are not exposed to userspace via
> >> KVM_GET_MSR_INDEX_LIST, instead userspace complies with KVM's setup and
> >> composes the uAPI params. KVM synthetic MSR indices start from 0 and
> >> increase linearly. Userspace caller should tag MSR type correctly in
> >> order to access intended HW or synthetic MSR.
> >
> >The old feedback[*]
Good sleuthing! I completely forgot about that...
> was to introduce support for SYNTHETIC registers instead of limiting it to MSR.
>
> GUEST_SSP is a real register but not an MSR,
Eh, yes and no. The vCPU's current SSP is a real register, but there's no
interface to access it. The "synthetic" part here the interface that KVM is
defining. Though I can see where that gets confusing since "synthetic" in KVM
usually means a completely made up register/MSR.
> so it's ok to treat it as a synthetic MSR. But, it's probably
> inappropriate/inaccurate to call it a synthetic register.
>
> I think we need a clear definition for "synthetic register" to determine what
> should be included in this category. "synthetic MSR" is clear - it refers to
> something that isn't an MSR in hardware but is handled by KVM as an MSR.
Ah, but I specifically want to avoid bleeding those details into userspace.
I.e. I don't want userspace to know that internally, KVM handles GUEST_SSP via
the MSR infrastructure, so that if for whatever reason we want to do something
different (very unlikely), then we don't create a contradiction.
> That said, I'm still fine with renaming all "synthetic MSR" to "synthetic
> register" in this series. :)
>
> Sean, which do you prefer with fresh eyes?
>
> If we make this change, I suppose KVM_x86_REG_TYPE_SIZE() should be dropped, as
> we can't guarantee that the size will remain constant for the "synthetic
> register" type, since it could be extended to include registers with different
> sizes in the future.
Ooh, good catch. We can keep it, we just need to be more conservative and only
define a valid size for GUEST_SSP.
To avoid confusion with "synthetic", what if we go with a more literal type of
"KVM"? That's vague enough that it gives KVM a lot of flexibility, but specific
enough that it'll be intuitive for readers, hopefully.
---
arch/x86/include/uapi/asm/kvm.h | 21 +++++++++++++--------
arch/x86/kvm/x86.c | 18 +++++++-----------
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 478d9b63a9db..eeefc03120fc 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -412,28 +412,33 @@ struct kvm_xcrs {
};
#define KVM_X86_REG_TYPE_MSR 2
-#define KVM_X86_REG_TYPE_SYNTHETIC_MSR 3
+#define KVM_X86_REG_TYPE_KVM 3
-#define KVM_X86_REG_TYPE_SIZE(type) \
+#define KVM_X86_KVM_REG_SIZE(reg) \
+({ \
+ reg == KVM_REG_GUEST_SSP ? KVM_REG_SIZE_U64 : 0; \
+})
+
+#define KVM_X86_REG_TYPE_SIZE(type, reg) \
({ \
__u64 type_size = (__u64)type << 32; \
\
type_size |= type == KVM_X86_REG_TYPE_MSR ? KVM_REG_SIZE_U64 : \
- type == KVM_X86_REG_TYPE_SYNTHETIC_MSR ? KVM_REG_SIZE_U64 :\
+ type == KVM_X86_REG_TYPE_KVM ? KVM_X86_KVM_REG_SIZE(reg) : \
0; \
type_size; \
})
#define KVM_X86_REG_ENCODE(type, index) \
- (KVM_REG_X86 | KVM_X86_REG_TYPE_SIZE(type) | index)
+ (KVM_REG_X86 | KVM_X86_REG_TYPE_SIZE(type, index) | index)
#define KVM_X86_REG_MSR(index) \
KVM_X86_REG_ENCODE(KVM_X86_REG_TYPE_MSR, index)
-#define KVM_X86_REG_SYNTHETIC_MSR(index) \
- KVM_X86_REG_ENCODE(KVM_X86_REG_TYPE_SYNTHETIC_MSR, index)
+#define KVM_X86_REG_KVM(index) \
+ KVM_X86_REG_ENCODE(KVM_X86_REG_TYPE_KVM, index)
-/* KVM synthetic MSR index staring from 0 */
-#define KVM_SYNTHETIC_GUEST_SSP 0
+/* KVM-defined registers staring from 0 */
+#define KVM_REG_GUEST_SSP 0
#define KVM_SYNC_X86_REGS (1UL << 0)
#define KVM_SYNC_X86_SREGS (1UL << 1)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9930678f5a3b..e0d7298aea94 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6057,10 +6057,10 @@ struct kvm_x86_reg_id {
__u8 x86;
};
-static int kvm_translate_synthetic_msr(struct kvm_x86_reg_id *reg)
+static int kvm_translate_kvm_reg(struct kvm_x86_reg_id *reg)
{
switch (reg->index) {
- case KVM_SYNTHETIC_GUEST_SSP:
+ case KVM_REG_GUEST_SSP:
reg->type = KVM_X86_REG_TYPE_MSR;
reg->index = MSR_KVM_INTERNAL_GUEST_SSP;
break;
@@ -6204,15 +6204,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
if (id->rsvd || id->rsvd4)
break;
- if (id->type != KVM_X86_REG_TYPE_MSR &&
- id->type != KVM_X86_REG_TYPE_SYNTHETIC_MSR)
- break;
-
- if ((reg.id & KVM_REG_SIZE_MASK) != KVM_REG_SIZE_U64)
- break;
-
- if (id->type == KVM_X86_REG_TYPE_SYNTHETIC_MSR) {
- r = kvm_translate_synthetic_msr(id);
+ if (id->type == KVM_X86_REG_TYPE_KVM) {
+ r = kvm_translate_kvm_reg(id);
if (r)
break;
}
@@ -6221,6 +6214,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
if (id->type != KVM_X86_REG_TYPE_MSR)
break;
+ if ((reg.id & KVM_REG_SIZE_MASK) != KVM_REG_SIZE_U64)
+ break;
+
value = u64_to_user_ptr(reg.addr);
if (ioctl == KVM_GET_ONE_REG)
r = kvm_get_one_msr(vcpu, id->index, value);
base-commit: eb92644efe58ecb8d00a83ef1788871404116001
--
Powered by blists - more mailing lists