On 18 February 2025 17:33:14 CET, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: >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. Well, I just changed QEMU to do KVM_XEN_HVM_CONFIG from the first vCPU init because QEMU doesn't know if it needs to avoid the Hyper-V MSR space at kvm_xen_init() time. But yes, we don't want to depend on ordering either way.
