On 14 December 2020 21:27:19 GMT, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote: >David Woodhouse <dwmw2@xxxxxxxxxxxxx> writes: > >> From: Joao Martins <joao.m.martins@xxxxxxxxxx> >> >> Xen usually places its MSR at 0x40000000 or 0x40000200 depending on >> whether it is running in viridian mode or not. Note that this is not >> ABI guaranteed, so it is possible for Xen to advertise the MSR some >> place else. >> >> Given the way xen_hvm_config() is handled, if the former address is >> selected, this will conflict with Hyper-V's MSR >> (HV_X64_MSR_GUEST_OS_ID) which unconditionally uses the same address. >> >> Given that the MSR location is arbitrary, move the xen_hvm_config() >> handling to the top of kvm_set_msr_common() before falling through. >> > >In case we're making MSR 0x40000000 something different from >HV_X64_MSR_GUEST_OS_ID we can and probably should disable Hyper-V >emulation in KVM completely -- or how else is it going to work? The way Xen itself does this — and the way we have to do it if we want to faithfully emulate Xen and support live migration from it — is to shift the Xen MSRs up to (from memory) 0x40000200 if Hyper-V is enabled. I did look at disabling Hyper-V entirely when it isn't enabled, but the only flag we have for it being enabled is the guest OS ID being set... which is done through that MSR :) My minimal version ended up being so close to Joao's original that it was not longer worth the bikeshedding and so I gave up on it and stuck with the original. >> @@ -3001,6 +3001,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, >struct msr_data *msr_info) >> u32 msr = msr_info->index; >> u64 data = msr_info->data; >> >> + if (msr && (msr == vcpu->kvm->arch.xen_hvm_config.msr)) >> + return xen_hvm_config(vcpu, data); >> + > >Can we generalize this maybe? E.g. before handling KVM and >architectural >MSRs we check that the particular MSR is not overriden by an emulated >hypervisor, > >e.g. > if (kvm_emulating_hyperv(kvm) && kvm_hyperv_msr_overriden(kvm,msr) > return kvm_hyperv_handle_msr(kvm, msr); > if (kvm_emulating_xen(kvm) && kvm_xen_msr_overriden(kvm,msr) > return kvm_xen_handle_msr(kvm, msr); That smells a bit like overengineering. As I said, I did have a play with "improving" Joao's original patch but nothing I tried actually made more sense to me than this once the details were ironed out. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
