2021-02-26 10:19:00

by Paolo Bonzini

[permalink] [raw]
Subject: [PATCH] KVM: x86: allow compiling out the Xen hypercall interface

The Xen hypercall interface adds to the attack surface of the hypervisor
and will be used quite rarely. Allow compiling it out.

Suggested-by: Christoph Hellwig <[email protected]>
Cc: David Woodhouse <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
arch/x86/kvm/Kconfig | 9 ++++++++
arch/x86/kvm/Makefile | 3 ++-
arch/x86/kvm/x86.c | 2 ++
arch/x86/kvm/xen.h | 49 ++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 7ac592664c52..5a0d704581bd 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -103,6 +103,15 @@ config KVM_AMD_SEV
Provides support for launching Encrypted VMs (SEV) and Encrypted VMs
with Encrypted State (SEV-ES) on AMD processors.

+config KVM_XEN
+ bool "Support for Xen hypercall interface"
+ depends on KVM && IA32_FEAT_CTL
+ help
+ Provides KVM support for the Xen shared information page and
+ for passing Xen hypercalls to userspace.
+
+ If in doubt, say "N".
+
config KVM_MMU_AUDIT
bool "Audit KVM MMU"
depends on KVM && TRACEPOINTS
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index aeab168c5711..1b4766fe1de2 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -14,11 +14,12 @@ kvm-y += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
$(KVM)/dirty_ring.o
kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o

-kvm-y += x86.o emulate.o i8259.o irq.o lapic.o xen.o \
+kvm-y += x86.o emulate.o i8259.o irq.o lapic.o \
i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
hyperv.o debugfs.o mmu/mmu.o mmu/page_track.o \
mmu/spte.o
kvm-$(CONFIG_X86_64) += mmu/tdp_iter.o mmu/tdp_mmu.o
+kvm-$(CONFIG_KVM_XEN) += xen.o

kvm-intel-y += vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
vmx/evmcs.o vmx/nested.o vmx/posted_intr.o
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bfc928495bd4..9727295b7817 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8039,8 +8039,10 @@ void kvm_arch_exit(void)
kvm_mmu_module_exit();
free_percpu(user_return_msrs);
kmem_cache_destroy(x86_fpu_cache);
+#ifdef CONFIG_KVM_XEN
static_key_deferred_flush(&kvm_xen_enabled);
WARN_ON(static_branch_unlikely(&kvm_xen_enabled.key));
+#endif
}

static int __kvm_vcpu_halt(struct kvm_vcpu *vcpu, int state, int reason)
diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
index b66a921776f4..6abdbebb029b 100644
--- a/arch/x86/kvm/xen.h
+++ b/arch/x86/kvm/xen.h
@@ -9,6 +9,7 @@
#ifndef __ARCH_X86_KVM_XEN_H__
#define __ARCH_X86_KVM_XEN_H__

+#ifdef CONFIG_KVM_XEN
#include <linux/jump_label_ratelimit.h>

extern struct static_key_false_deferred kvm_xen_enabled;
@@ -18,7 +19,6 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data);
int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
int kvm_xen_hvm_get_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
-int kvm_xen_hypercall(struct kvm_vcpu *vcpu);
int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data);
int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc);
void kvm_xen_destroy_vm(struct kvm *kvm);
@@ -38,6 +38,53 @@ static inline int kvm_xen_has_interrupt(struct kvm_vcpu *vcpu)

return 0;
}
+#else
+static inline int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
+{
+ return -EINVAL;
+}
+
+static inline int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
+{
+ return -EINVAL;
+}
+
+static inline int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
+{
+ return -EINVAL;
+}
+
+static inline int kvm_xen_hvm_get_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
+{
+ return -EINVAL;
+}
+
+static inline int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
+{
+ return 1;
+}
+
+static inline int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc)
+{
+ return -EINVAL;
+}
+
+static inline void kvm_xen_destroy_vm(struct kvm *kvm)
+{
+}
+
+static inline bool kvm_xen_hypercall_enabled(struct kvm *kvm)
+{
+ return false;
+}
+
+static inline int kvm_xen_has_interrupt(struct kvm_vcpu *vcpu)
+{
+ return 0;
+}
+#endif
+
+int kvm_xen_hypercall(struct kvm_vcpu *vcpu);

/* 32-bit compatibility definitions, also used natively in 32-bit build */
#include <asm/pvclock-abi.h>
--
2.26.2


2021-02-26 10:43:46

by David Woodhouse

[permalink] [raw]
Subject: Re: [EXTERNAL] [PATCH] KVM: x86: allow compiling out the Xen hypercall interface

On Fri, 2021-02-26 at 05:14 -0500, Paolo Bonzini wrote:
> The Xen hypercall interface adds to the attack surface of the hypervisor
> and will be used quite rarely. Allow compiling it out.
>
> Suggested-by: Christoph Hellwig <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> arch/x86/kvm/Kconfig | 9 ++++++++
> arch/x86/kvm/Makefile | 3 ++-
> arch/x86/kvm/x86.c | 2 ++
> arch/x86/kvm/xen.h | 49 ++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 7ac592664c52..5a0d704581bd 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -103,6 +103,15 @@ config KVM_AMD_SEV
> Provides support for launching Encrypted VMs (SEV) and Encrypted VMs
> with Encrypted State (SEV-ES) on AMD processors.
>
> +config KVM_XEN
> + bool "Support for Xen hypercall interface"
> + depends on KVM && IA32_FEAT_CTL
> + help
> + Provides KVM support for the Xen shared information page and
> + for passing Xen hypercalls to userspace.

I'd be a bit less specific there. Just "support for hosting Xen HVM
guests" perhaps? It already includes basic event channel support, I'm
posted an RFC for the runstate stuff, and more event channel
acceleration is next...


> + If in doubt, say "N".
> +
> config KVM_MMU_AUDIT
> bool "Audit KVM MMU"
> depends on KVM && TRACEPOINTS
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index aeab168c5711..1b4766fe1de2 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -14,11 +14,12 @@ kvm-y += $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o \
> $(KVM)/dirty_ring.o
> kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
>
> -kvm-y += x86.o emulate.o i8259.o irq.o lapic.o xen.o \
> +kvm-y += x86.o emulate.o i8259.o irq.o lapic.o \
> i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
> hyperv.o debugfs.o mmu/mmu.o mmu/page_track.o \
> mmu/spte.o
> kvm-$(CONFIG_X86_64) += mmu/tdp_iter.o mmu/tdp_mmu.o
> +kvm-$(CONFIG_KVM_XEN) += xen.o
>
> kvm-intel-y += vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o \
> vmx/evmcs.o vmx/nested.o vmx/posted_intr.o
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index bfc928495bd4..9727295b7817 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8039,8 +8039,10 @@ void kvm_arch_exit(void)
> kvm_mmu_module_exit();
> free_percpu(user_return_msrs);
> kmem_cache_destroy(x86_fpu_cache);
> +#ifdef CONFIG_KVM_XEN
> static_key_deferred_flush(&kvm_xen_enabled);
> WARN_ON(static_branch_unlikely(&kvm_xen_enabled.key));
> +#endif

We could always just drop that. It's just paranoia.

> }
>
> static int __kvm_vcpu_halt(struct kvm_vcpu *vcpu, int state, int reason)
> diff --git a/arch/x86/kvm/xen.h b/arch/x86/kvm/xen.h
> index b66a921776f4..6abdbebb029b 100644
> --- a/arch/x86/kvm/xen.h
> +++ b/arch/x86/kvm/xen.h
> @@ -9,6 +9,7 @@
> #ifndef __ARCH_X86_KVM_XEN_H__
> #define __ARCH_X86_KVM_XEN_H__
>
> +#ifdef CONFIG_KVM_XEN
> #include <linux/jump_label_ratelimit.h>
>
> extern struct static_key_false_deferred kvm_xen_enabled;
> @@ -18,7 +19,6 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data);
> int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
> int kvm_xen_hvm_get_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
> -int kvm_xen_hypercall(struct kvm_vcpu *vcpu);
> int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data);
> int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc);
> void kvm_xen_destroy_vm(struct kvm *kvm);
> @@ -38,6 +38,53 @@ static inline int kvm_xen_has_interrupt(struct kvm_vcpu *vcpu)
>
> return 0;
> }
> +#else
> +static inline int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_hvm_get_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
> +{
> + return 1;
> +}
> +
> +static inline int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc)
> +{
> + return -EINVAL;
> +}



I wonder if at least for the ioctls, it might be nicer to put the
#ifdef CONFIG_KVM_XEN around the callers for these? If we did it that
way we'd probably spot the fact that we need to stop advertising the
feature too... :)


Attachments:
smime.p7s (5.05 kB)