On Sat, Feb 15, 2025, David Woodhouse wrote: > On 15 February 2025 02:14:33 CET, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > >diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c > >index a909b817b9c0..5b94825001a7 100644 > >--- a/arch/x86/kvm/xen.c > >+++ b/arch/x86/kvm/xen.c > >@@ -1324,6 +1324,15 @@ int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc) > > xhc->blob_size_32 || xhc->blob_size_64)) > > return -EINVAL; > > > >+ /* > >+ * Restrict the MSR to the range that is unofficially reserved for > >+ * synthetic, virtualization-defined MSRs, e.g. to prevent confusing > >+ * KVM by colliding with a real MSR that requires special handling. > >+ */ > >+ if (xhc->msr && > >+ (xhc->msr < KVM_XEN_MSR_MIN_INDEX || xhc->msr > KVM_XEN_MSR_MAX_INDEX)) > >+ return -EINVAL; > >+ > > mutex_lock(&kvm->arch.xen.xen_lock); > > > > if (xhc->msr && !kvm->arch.xen_hvm_config.msr) > > I'd still like to restrict this to ensure it doesn't collide with MSRs that > KVM expects to emulate. But that can be a separate patch, as discussed. I think that has to go in userspace. If KVM adds on-by-default, i.e. unguarded, conflicting MSR emulation, then KVM will have broken userspace regardless of whether or not KVM explicitly rejects KVM_XEN_HVM_CONFIG based on emulated MSRs. If we assume future us are somewhat competent and guard new MSR emulation with a feature bit, capability, etc., then rejecting KVM_XEN_HVM_CONFIG isn't obviously better, or even feasible in some cases. E.g. if the opt-in is done via guest CPUID, then KVM is stuck because KVM_XEN_HVM_CONFIG can (and generally should?) be called before vCPUs are even created. Even if the opt-in is VM-scoped, e.g. a capabilitiy, there are still ordering issues as userpace would see different behavior depending on the order between KVM_XEN_HVM_CONFIG and the capability. And if the MSR emulation is guarded, rejecting KVM_XEN_HVM_CONFIG without a precise check is undesirable, because KVM would unnecessarily break userspace. > This patch should probably have a docs update too. Gah, forgot that.
