diff mbox series

[02/15] KVM: x86/xen: fix Xen hypercall page msr handling

Message ID 20201204011848.2967588-3-dwmw2@infradead.org (mailing list archive)
State New, archived
Headers show
Series KVM: Add Xen hypercall and shared info pages | expand

Commit Message

David Woodhouse Dec. 4, 2020, 1:18 a.m. UTC
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.

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(-)

Comments

Alexander Graf Dec. 4, 2020, 6:26 p.m. UTC | #1
On 04.12.20 02:18, David Woodhouse wrote:
> 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.
> 
> 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);

How much of this can we handle with the MSR allow listing and MSR 
trapping in user space?


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
David Woodhouse Dec. 4, 2020, 6:54 p.m. UTC | #2
On 4 December 2020 18:26:36 GMT, Alexander Graf <graf@amazon.com> wrote:
>How much of this can we handle with the MSR allow listing and MSR 
>trapping in user space?

We resisted that trapping for a long time but have it now, and both the existing Xen and Hyper-V support — as well as a bunch of other things that KVM has historically done in-kernel on trapping MSRs — could *theoretically* be done in userspace.

But they all already exist in the kernel. This is just a bug fix because the Hyper-V support broke it for Xen due to the conflict.
diff mbox series

Patch

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);
+
 	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;