Re: [PATCH v3 02/17] KVM: x86/xen: fix Xen hypercall page msr handling

[Vitaly Kuznetsov <vkuznets@redhat.com>]

David Woodhouse <dwmw2@infradead.org> writes:

> From: Joao Martins <joao.m.martins@oracle.com>
>
> 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?

> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  arch/x86/kvm/x86.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c7f1ba21212e..13ba4a64f748 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -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);

	switch (msr) {
		...

>  	switch (msr) {
>  	case MSR_AMD64_NB_CFG:
>  	case MSR_IA32_UCODE_WRITE:
> @@ -3288,8 +3291,6 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		vcpu->arch.msr_misc_features_enables = data;
>  		break;
>  	default:
> -		if (msr && (msr == vcpu->kvm->arch.xen_hvm_config.msr))
> -			return xen_hvm_config(vcpu, data);
>  		if (kvm_pmu_is_valid_msr(vcpu, msr))
>  			return kvm_pmu_set_msr(vcpu, msr_info);
>  		return KVM_MSR_RET_INVALID;

-- 
Vitaly