Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-02 Thread Ankur Arora

On 2020-12-02 12:03 a.m., David Woodhouse wrote:

On Tue, 2020-12-01 at 21:19 -0800, Ankur Arora wrote:

+ for (i = 0; i < PAGE_SIZE / sizeof(instructions); i++) {
+ *(u32 *)[1] = i;
+ if (kvm_vcpu_write_guest(vcpu,
+  page_addr + (i * 
sizeof(instructions)),
+  instructions, 
sizeof(instructions)))
+ return 1;
+ }


HYPERVISOR_iret isn't supported on 64bit so should be ud2 instead.


Yeah, I got part way through typing that part but concluded it probably
wasn't a fast path that absolutely needed to be emulated in the kernel.

The VMM can inject the UD# when it receives the hypercall.


That would work as well but if it's a straight ud2 on the hypercall
page, wouldn't the guest just execute it when/if it does a
HYPERVISOR_iret?

Ankur




I appreciate it *is* a guest-visible difference, if we're being really
pedantic, but I don't think we were even going to be able to 100% hide
the fact that it's not actually Xen.



Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-02 Thread David Woodhouse
On Wed, 2020-12-02 at 11:17 +, Joao Martins wrote:
> Xen viridian mode is indeed one thing that needed fixing. And definitely a
> separate patch as you do here. Both this one and the previous is looking good.
> 
> I suppose that because you switch to kvm_vcpu_write_guest() you no longer need
> to validate that the hypercall page is correct neither marking as dirty. 
> Probably
> worth making that explicit in the commit message.

I had intended that to be covered by "open-coding it".

Surely the *point* in using a helper function and not open-coding it is
that we don't *have* to care about the details of what it does ;)



smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-02 Thread Joao Martins
On 12/1/20 11:19 AM, David Woodhouse wrote:
> On Tue, 2020-12-01 at 09:48 +, David Woodhouse wrote:
>> So I switched it to generate the hypercall page directly from the
>> kernel, just like we do for the Hyper-V hypercall page.
> 
> Speaking of Hyper-V... I'll post this one now so you can start heckling
> it.
> 
Xen viridian mode is indeed one thing that needed fixing. And definitely a
separate patch as you do here. Both this one and the previous is looking good.

I suppose that because you switch to kvm_vcpu_write_guest() you no longer need
to validate that the hypercall page is correct neither marking as dirty. 
Probably
worth making that explicit in the commit message.

> I won't post the rest as I go; not much of the rest of the series when
> I eventually post it will be very new and exciting. It'll just be
> rebasing and tweaking your originals and perhaps adding some tests.
> 

Awesome!!


Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-02 Thread David Woodhouse
On Tue, 2020-12-01 at 21:19 -0800, Ankur Arora wrote:
> > + for (i = 0; i < PAGE_SIZE / sizeof(instructions); i++) {
> > + *(u32 *)[1] = i;
> > + if (kvm_vcpu_write_guest(vcpu,
> > +  page_addr + (i * 
> > sizeof(instructions)),
> > +  instructions, 
> > sizeof(instructions)))
> > + return 1;
> > + }
> 
> HYPERVISOR_iret isn't supported on 64bit so should be ud2 instead.

Yeah, I got part way through typing that part but concluded it probably
wasn't a fast path that absolutely needed to be emulated in the kernel.

The VMM can inject the UD# when it receives the hypercall.

I appreciate it *is* a guest-visible difference, if we're being really
pedantic, but I don't think we were even going to be able to 100% hide
the fact that it's not actually Xen.


smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-01 Thread Ankur Arora

On 2020-12-01 1:48 a.m., David Woodhouse wrote:

On Wed, 2019-02-20 at 20:15 +, Joao Martins wrote:

Add a new exit reason for emulator to handle Xen hypercalls.
Albeit these are injected only if guest has initialized the Xen
hypercall page


I've reworked this a little.

I didn't like the inconsistency of allowing userspace to provide the
hypercall pages even though the ABI is now defined by the kernel and it
*has* to be VMCALL/VMMCALL.

So I switched it to generate the hypercall page directly from the
kernel, just like we do for the Hyper-V hypercall page.

I introduced a new flag in the xen_hvm_config struct to enable this
behaviour, and advertised it in the KVM_CAP_XEN_HVM return value.

I also added the cpl and support for 6-argument hypercalls, and made it
check the guest RIP when completing the call as discussed (although I
still think that probably ought to be a generic thing).

I adjusted the test case from my version of the patch, and added
support for actually testing the hypercall page MSR.

https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/xenpv

I'll go through and rebase your patch series at least up to patch 16
and collect them in that tree, then probably post them for real once
I've got everything working locally.


===
 From c037c329c8867b219afe2100e383c62e9db7b06d Mon Sep 17 00:00:00 2001
From: Joao Martins 
Date: Wed, 13 Jun 2018 09:55:44 -0400
Subject: [PATCH] KVM: x86/xen: intercept xen hypercalls if enabled

Add a new exit reason for emulator to handle Xen hypercalls.

Since this means KVM owns the ABI, dispense with the facility for the
VMM to provide its own copy of the hypercall pages; just fill them in
directly using VMCALL/VMMCALL as we do for the Hyper-V hypercall page.

This behaviour is enabled by a new INTERCEPT_HCALL flag in the
KVM_XEN_HVM_CONFIG ioctl structure, and advertised by the same flag
being returned from the KVM_CAP_XEN_HVM check.

Add a test case and shift xen_hvm_config() to the nascent xen.c while
we're at it.

Signed-off-by: Joao Martins 
Signed-off-by: David Woodhouse 
---
  arch/x86/include/asm/kvm_host.h   |   6 +
  arch/x86/kvm/Makefile |   2 +-
  arch/x86/kvm/trace.h  |  36 +
  arch/x86/kvm/x86.c|  46 +++---
  arch/x86/kvm/xen.c| 140 ++
  arch/x86/kvm/xen.h|  21 +++
  include/uapi/linux/kvm.h  |  19 +++
  tools/testing/selftests/kvm/Makefile  |   1 +
  tools/testing/selftests/kvm/lib/kvm_util.c|   1 +
  .../selftests/kvm/x86_64/xen_vmcall_test.c| 123 +++
  10 files changed, 365 insertions(+), 30 deletions(-)
  create mode 100644 arch/x86/kvm/xen.c
  create mode 100644 arch/x86/kvm/xen.h
  create mode 100644 tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c




diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
new file mode 100644
index ..6400a4bc8480
--- /dev/null
+++ b/arch/x86/kvm/xen.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright © 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * KVM Xen emulation
+ */
+
+#include "x86.h"
+#include "xen.h"
+
+#include 
+
+#include 
+
+#include "trace.h"
+
+int kvm_xen_hvm_config(struct kvm_vcpu *vcpu, u64 data)
+   {
+   struct kvm *kvm = vcpu->kvm;
+   u32 page_num = data & ~PAGE_MASK;
+   u64 page_addr = data & PAGE_MASK;
+
+   /*
+* If Xen hypercall intercept is enabled, fill the hypercall
+* page with VMCALL/VMMCALL instructions since that's what
+* we catch. Else the VMM has provided the hypercall pages
+* with instructions of its own choosing, so use those.
+*/
+   if (kvm_xen_hypercall_enabled(kvm)) {
+   u8 instructions[32];
+   int i;
+
+   if (page_num)
+   return 1;
+
+   /* mov imm32, %eax */
+   instructions[0] = 0xb8;
+
+   /* vmcall / vmmcall */
+   kvm_x86_ops.patch_hypercall(vcpu, instructions + 5);
+
+   /* ret */
+   instructions[8] = 0xc3;
+
+   /* int3 to pad */
+   memset(instructions + 9, 0xcc, sizeof(instructions) - 9);
+
+   for (i = 0; i < PAGE_SIZE / sizeof(instructions); i++) {
+   *(u32 *)[1] = i;
+   if (kvm_vcpu_write_guest(vcpu,
+page_addr + (i * 
sizeof(instructions)),
+instructions, 
sizeof(instructions)))
+   return 1;
+   }


HYPERVISOR_iret isn't supported on 64bit so should be ud2 instead.


Ankur


+   } else {
+   int lm = is_long_mode(vcpu);
+   u8 *blob_addr = lm ? 

Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-01 Thread David Woodhouse
On Tue, 2020-12-01 at 09:48 +, David Woodhouse wrote:
> So I switched it to generate the hypercall page directly from the
> kernel, just like we do for the Hyper-V hypercall page.

Speaking of Hyper-V... I'll post this one now so you can start heckling
it.

I won't post the rest as I go; not much of the rest of the series when
I eventually post it will be very new and exciting. It'll just be
rebasing and tweaking your originals and perhaps adding some tests.

===
From db020c521c3a96b698a78d4e62cdd19da3831585 Mon Sep 17 00:00:00 2001
From: David Woodhouse 
Date: Tue, 1 Dec 2020 10:59:26 +
Subject: [PATCH] KVM: x86: Fix coexistence of Xen and Hyper-V hypercalls

Disambiguate Xen vs. Hyper-V calls by adding 'orl $0x8000, %eax'
at the start of the Hyper-V hypercall page when Xen hypercalls are
also enabled.

That bit is reserved in the Hyper-V ABI, and those hypercall numbers
will never be used by Xen (because it does precisely the same trick).

Switch to using kvm_vcpu_write_guest() while we're at it, instead of
open-coding it.

Signed-off-by: David Woodhouse 
---
 arch/x86/kvm/hyperv.c | 40 ++-
 arch/x86/kvm/xen.c|  6 +++
 .../selftests/kvm/x86_64/xen_vmcall_test.c| 39 +++---
 3 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 5c7c4060b45c..347ff9ad70b3 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -23,6 +23,7 @@
 #include "ioapic.h"
 #include "cpuid.h"
 #include "hyperv.h"
+#include "xen.h"
 
 #include 
 #include 
@@ -1139,9 +1140,9 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 data,
hv->hv_hypercall &= ~HV_X64_MSR_HYPERCALL_ENABLE;
break;
case HV_X64_MSR_HYPERCALL: {
-   u64 gfn;
-   unsigned long addr;
-   u8 instructions[4];
+   u8 instructions[9];
+   int i = 0;
+   u64 addr;
 
/* if guest os id is not set hypercall should remain disabled */
if (!hv->hv_guest_os_id)
@@ -1150,16 +1151,33 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 data,
hv->hv_hypercall = data;
break;
}
-   gfn = data >> HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT;
-   addr = gfn_to_hva(kvm, gfn);
-   if (kvm_is_error_hva(addr))
-   return 1;
-   kvm_x86_ops.patch_hypercall(vcpu, instructions);
-   ((unsigned char *)instructions)[3] = 0xc3; /* ret */
-   if (__copy_to_user((void __user *)addr, instructions, 4))
+
+   /*
+* If Xen and Hyper-V hypercalls are both enabled, disambiguate
+* the same way Xen itself does, by setting the bit 31 of EAX
+* which is RsvdZ in the 32-bit Hyper-V hypercall ABI and just
+* going to be clobbered on 64-bit.
+*/
+   if (kvm_xen_hypercall_enabled(kvm)) {
+   /* orl $0x8000, %eax */
+   instructions[i++] = 0x0d;
+   instructions[i++] = 0x00;
+   instructions[i++] = 0x00;
+   instructions[i++] = 0x00;
+   instructions[i++] = 0x80;
+   }
+
+   /* vmcall/vmmcall */
+   kvm_x86_ops.patch_hypercall(vcpu, instructions + i);
+   i += 3;
+
+   /* ret */
+   ((unsigned char *)instructions)[i++] = 0xc3;
+
+   addr = data & HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK;
+   if (kvm_vcpu_write_guest(vcpu, addr, instructions, i))
return 1;
hv->hv_hypercall = data;
-   mark_page_dirty(kvm, gfn);
break;
}
case HV_X64_MSR_REFERENCE_TSC:
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 6400a4bc8480..c3426213a970 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -8,6 +8,7 @@
 
 #include "x86.h"
 #include "xen.h"
+#include "hyperv.h"
 
 #include 
 
@@ -99,6 +100,11 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
 
input = (u64)kvm_register_read(vcpu, VCPU_REGS_RAX);
 
+   /* Hyper-V hypercalls get bit 31 set in EAX */
+   if ((input & 0x8000) &&
+   kvm_hv_hypercall_enabled(vcpu->kvm))
+   return kvm_hv_hypercall(vcpu);
+
longmode = is_64_bit_mode(vcpu);
if (!longmode) {
params[0] = (u32)kvm_rbx_read(vcpu);
diff --git a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
index 3f1dd93626e5..24f279e1a66b 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
@@ -15,6 +15,7 @@

Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2020-12-01 Thread David Woodhouse
On Wed, 2019-02-20 at 20:15 +, Joao Martins wrote:
> Add a new exit reason for emulator to handle Xen hypercalls.
> Albeit these are injected only if guest has initialized the Xen
> hypercall page 

I've reworked this a little.

I didn't like the inconsistency of allowing userspace to provide the
hypercall pages even though the ABI is now defined by the kernel and it
*has* to be VMCALL/VMMCALL.

So I switched it to generate the hypercall page directly from the
kernel, just like we do for the Hyper-V hypercall page.

I introduced a new flag in the xen_hvm_config struct to enable this
behaviour, and advertised it in the KVM_CAP_XEN_HVM return value.

I also added the cpl and support for 6-argument hypercalls, and made it
check the guest RIP when completing the call as discussed (although I
still think that probably ought to be a generic thing).

I adjusted the test case from my version of the patch, and added
support for actually testing the hypercall page MSR.

https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/xenpv

I'll go through and rebase your patch series at least up to patch 16
and collect them in that tree, then probably post them for real once
I've got everything working locally.


===
From c037c329c8867b219afe2100e383c62e9db7b06d Mon Sep 17 00:00:00 2001
From: Joao Martins 
Date: Wed, 13 Jun 2018 09:55:44 -0400
Subject: [PATCH] KVM: x86/xen: intercept xen hypercalls if enabled

Add a new exit reason for emulator to handle Xen hypercalls.

Since this means KVM owns the ABI, dispense with the facility for the
VMM to provide its own copy of the hypercall pages; just fill them in
directly using VMCALL/VMMCALL as we do for the Hyper-V hypercall page.

This behaviour is enabled by a new INTERCEPT_HCALL flag in the
KVM_XEN_HVM_CONFIG ioctl structure, and advertised by the same flag
being returned from the KVM_CAP_XEN_HVM check.

Add a test case and shift xen_hvm_config() to the nascent xen.c while
we're at it.

Signed-off-by: Joao Martins 
Signed-off-by: David Woodhouse 
---
 arch/x86/include/asm/kvm_host.h   |   6 +
 arch/x86/kvm/Makefile |   2 +-
 arch/x86/kvm/trace.h  |  36 +
 arch/x86/kvm/x86.c|  46 +++---
 arch/x86/kvm/xen.c| 140 ++
 arch/x86/kvm/xen.h|  21 +++
 include/uapi/linux/kvm.h  |  19 +++
 tools/testing/selftests/kvm/Makefile  |   1 +
 tools/testing/selftests/kvm/lib/kvm_util.c|   1 +
 .../selftests/kvm/x86_64/xen_vmcall_test.c| 123 +++
 10 files changed, 365 insertions(+), 30 deletions(-)
 create mode 100644 arch/x86/kvm/xen.c
 create mode 100644 arch/x86/kvm/xen.h
 create mode 100644 tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7e5f33a0d0e2..9de3229e91e1 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -520,6 +520,11 @@ struct kvm_vcpu_hv {
cpumask_t tlb_flush;
 };
 
+/* Xen HVM per vcpu emulation context */
+struct kvm_vcpu_xen {
+   u64 hypercall_rip;
+};
+
 struct kvm_vcpu_arch {
/*
 * rip and regs accesses must go through
@@ -717,6 +722,7 @@ struct kvm_vcpu_arch {
unsigned long singlestep_rip;
 
struct kvm_vcpu_hv hyperv;
+   struct kvm_vcpu_xen xen;
 
cpumask_var_t wbinvd_dirty_mask;
 
diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index b80e16d4..8bee4afc1fec 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -13,7 +13,7 @@ kvm-y += $(KVM)/kvm_main.o 
$(KVM)/coalesced_mmio.o \
$(KVM)/eventfd.o $(KVM)/irqchip.o $(KVM)/vfio.o
 kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
 
-kvm-y  += x86.o emulate.o i8259.o irq.o lapic.o \
+kvm-y  += x86.o emulate.o i8259.o irq.o lapic.o xen.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 mmu/tdp_iter.o mmu/tdp_mmu.o
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index aef960f90f26..d28ecb37b62c 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -92,6 +92,42 @@ TRACE_EVENT(kvm_hv_hypercall,
  __entry->outgpa)
 );
 
+/*
+ * Tracepoint for Xen hypercall.
+ */
+TRACE_EVENT(kvm_xen_hypercall,
+   TP_PROTO(unsigned long nr, unsigned long a0, unsigned long a1,
+unsigned long a2, unsigned long a3, unsigned long a4,
+unsigned long a5),
+   TP_ARGS(nr, a0, a1, a2, a3, a4, a5),
+
+   TP_STRUCT__entry(
+   __field(unsigned long, nr)
+   __field(unsigned long, a0)
+   __field(unsigned long, a1)
+   __field(unsigned long, a2)
+

Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2019-02-22 Thread Paolo Bonzini
On 22/02/19 01:30, Sean Christopherson wrote:
> On Thu, Feb 21, 2019 at 08:56:06PM +, Joao Martins wrote:
>> The Xen addition follows the same structure as Hyper-V in kvm (what you 
>> suggest
>> here is probably applicable for both). i.e. there's some Xen specific 
>> routines
>> for vm/vcpu init/teardown, and timer handling. We would have to place some of
>> those functions into a struct that gets registered at modinit.
> 
> Huh.  I never really hooked at the Hyper-V code, for some reason I always
> assumed it was only related to running KVM on Hyper-V.  I agree that this
> extra hurdle only makes sense if it's also applied to Hyper-V.

The difference is that Hyper-V support is more or less mandatory to run
recent Windows guests.  Having a config symbol would be enough for me.

Paolo


Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2019-02-21 Thread Sean Christopherson
On Thu, Feb 21, 2019 at 08:56:06PM +, Joao Martins wrote:
> On 2/21/19 6:29 PM, Sean Christopherson wrote:
> > On Wed, Feb 20, 2019 at 08:15:32PM +, Joao Martins wrote:
> >> Add a new exit reason for emulator to handle Xen hypercalls.
> >> Albeit these are injected only if guest has initialized the Xen
> >> hypercall page - the hypercall is just a convenience but one
> >> that is done by pretty much all guests. Hence if the guest
> >> sets the hypercall page, we assume a Xen guest is going to
> >> be set up.
> >>
> >> Emulator will then panic with:
> >>
> >> KVM: unknown exit reason 28
> >> RAX=0011 RBX=81e03e94 RCX=4000
> >> RDX=
> >> RSI=81e03e70 RDI=0006 RBP=81e03e90
> >> RSP=81e03e68
> >> R8 =73726576206e6558 R9 =81e03e90 R10=81e03e94
> >> R11=2e362e34206e6f69
> >> R12=4004 R13=81e03e8c R14=81e03e88
> >> R15=
> >> RIP=81001228 RFL=0082 [--S] CPL=0 II=0 A20=1 SMM=0 HLT=0
> >> ES =   00c0
> >> CS =0010   00a09b00 DPL=0 CS64 [-RA]
> >> SS =   00c0
> >> DS =   00c0
> >> FS =   00c0
> >> GS = 81f34000  00c0
> >> LDT=   00c0
> >> TR =0020  0fff 00808b00 DPL=0 TSS64-busy
> >> GDT= 81f3c000 007f
> >> IDT= 83265000 0fff
> >> CR0=80050033 CR2=880001fa6ff8 CR3=01fa6000 CR4=000406a0
> >> DR0= DR1= DR2=
> >> DR3=
> >> DR6=0ff0 DR7=0400
> >> EFER=0d01
> >> Code=cc cc cc cc cc cc cc cc cc cc cc cc b8 11 00 00 00 0f 01 c1  cc
> >> cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc b8 12
> >> 00 00 00 0f
> >>
> >> Signed-off-by: Joao Martins 
> >> ---
> >>  arch/x86/include/asm/kvm_host.h | 13 +++
> >>  arch/x86/kvm/Makefile   |  2 +-
> >>  arch/x86/kvm/trace.h| 33 +
> >>  arch/x86/kvm/x86.c  | 12 +++
> >>  arch/x86/kvm/xen.c  | 79 
> >> +
> >>  arch/x86/kvm/xen.h  | 10 ++
> >>  include/uapi/linux/kvm.h| 17 -
> >>  7 files changed, 164 insertions(+), 2 deletions(-)
> >>  create mode 100644 arch/x86/kvm/xen.c
> >>  create mode 100644 arch/x86/kvm/xen.h
> > 
> > ...
> > 
> >> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> >> index 31ecf7a76d5a..2b46c93c9380 100644
> >> --- a/arch/x86/kvm/Makefile
> >> +++ b/arch/x86/kvm/Makefile
> >> @@ -10,7 +10,7 @@ kvm-$(CONFIG_KVM_ASYNC_PF)   += $(KVM)/async_pf.o
> >>  
> >>  kvm-y += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> >>   i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
> >> - hyperv.o page_track.o debugfs.o
> >> + hyperv.o xen.o page_track.o debugfs.o
> > 
> > Can this be wrapped in a config?  Or even better, as a loadable module?
> 
> Turning that into a loadable module might be a little trickier, but I think it
> is doable if that's what folks/maintainers would prefer.
> 
> The Xen addition follows the same structure as Hyper-V in kvm (what you 
> suggest
> here is probably applicable for both). i.e. there's some Xen specific routines
> for vm/vcpu init/teardown, and timer handling. We would have to place some of
> those functions into a struct that gets registered at modinit.

Huh.  I never really hooked at the Hyper-V code, for some reason I always
assumed it was only related to running KVM on Hyper-V.  I agree that this
extra hurdle only makes sense if it's also applied to Hyper-V.


Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2019-02-21 Thread Joao Martins
On 2/21/19 6:29 PM, Sean Christopherson wrote:
> On Wed, Feb 20, 2019 at 08:15:32PM +, Joao Martins wrote:
>> Add a new exit reason for emulator to handle Xen hypercalls.
>> Albeit these are injected only if guest has initialized the Xen
>> hypercall page - the hypercall is just a convenience but one
>> that is done by pretty much all guests. Hence if the guest
>> sets the hypercall page, we assume a Xen guest is going to
>> be set up.
>>
>> Emulator will then panic with:
>>
>> KVM: unknown exit reason 28
>> RAX=0011 RBX=81e03e94 RCX=4000
>> RDX=
>> RSI=81e03e70 RDI=0006 RBP=81e03e90
>> RSP=81e03e68
>> R8 =73726576206e6558 R9 =81e03e90 R10=81e03e94
>> R11=2e362e34206e6f69
>> R12=4004 R13=81e03e8c R14=81e03e88
>> R15=
>> RIP=81001228 RFL=0082 [--S] CPL=0 II=0 A20=1 SMM=0 HLT=0
>> ES =   00c0
>> CS =0010   00a09b00 DPL=0 CS64 [-RA]
>> SS =   00c0
>> DS =   00c0
>> FS =   00c0
>> GS = 81f34000  00c0
>> LDT=   00c0
>> TR =0020  0fff 00808b00 DPL=0 TSS64-busy
>> GDT= 81f3c000 007f
>> IDT= 83265000 0fff
>> CR0=80050033 CR2=880001fa6ff8 CR3=01fa6000 CR4=000406a0
>> DR0= DR1= DR2=
>> DR3=
>> DR6=0ff0 DR7=0400
>> EFER=0d01
>> Code=cc cc cc cc cc cc cc cc cc cc cc cc b8 11 00 00 00 0f 01 c1  cc
>> cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc b8 12
>> 00 00 00 0f
>>
>> Signed-off-by: Joao Martins 
>> ---
>>  arch/x86/include/asm/kvm_host.h | 13 +++
>>  arch/x86/kvm/Makefile   |  2 +-
>>  arch/x86/kvm/trace.h| 33 +
>>  arch/x86/kvm/x86.c  | 12 +++
>>  arch/x86/kvm/xen.c  | 79 
>> +
>>  arch/x86/kvm/xen.h  | 10 ++
>>  include/uapi/linux/kvm.h| 17 -
>>  7 files changed, 164 insertions(+), 2 deletions(-)
>>  create mode 100644 arch/x86/kvm/xen.c
>>  create mode 100644 arch/x86/kvm/xen.h
> 
> ...
> 
>> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
>> index 31ecf7a76d5a..2b46c93c9380 100644
>> --- a/arch/x86/kvm/Makefile
>> +++ b/arch/x86/kvm/Makefile
>> @@ -10,7 +10,7 @@ kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
>>  
>>  kvm-y   += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
>> i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
>> -   hyperv.o page_track.o debugfs.o
>> +   hyperv.o xen.o page_track.o debugfs.o
> 
> Can this be wrapped in a config?  Or even better, as a loadable module?

Turning that into a loadable module might be a little trickier, but I think it
is doable if that's what folks/maintainers would prefer.

The Xen addition follows the same structure as Hyper-V in kvm (what you suggest
here is probably applicable for both). i.e. there's some Xen specific routines
for vm/vcpu init/teardown, and timer handling. We would have to place some of
those functions into a struct that gets registered at modinit.

> 2k+ lines of code is a non-trival amount of baggage for folks that don't
> care about running Xen guests.  I've only glanced through the series, so
> I've no idea if the resulting code would be an abomination.
> 
>>  
>>  kvm-intel-y += vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o 
>> vmx/evmcs.o vmx/nested.o
>>  kvm-amd-y   += svm.o pmu_amd.o


Re: [PATCH RFC 02/39] KVM: x86/xen: intercept xen hypercalls if enabled

2019-02-21 Thread Sean Christopherson
On Wed, Feb 20, 2019 at 08:15:32PM +, Joao Martins wrote:
> Add a new exit reason for emulator to handle Xen hypercalls.
> Albeit these are injected only if guest has initialized the Xen
> hypercall page - the hypercall is just a convenience but one
> that is done by pretty much all guests. Hence if the guest
> sets the hypercall page, we assume a Xen guest is going to
> be set up.
> 
> Emulator will then panic with:
> 
> KVM: unknown exit reason 28
> RAX=0011 RBX=81e03e94 RCX=4000
> RDX=
> RSI=81e03e70 RDI=0006 RBP=81e03e90
> RSP=81e03e68
> R8 =73726576206e6558 R9 =81e03e90 R10=81e03e94
> R11=2e362e34206e6f69
> R12=4004 R13=81e03e8c R14=81e03e88
> R15=
> RIP=81001228 RFL=0082 [--S] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =   00c0
> CS =0010   00a09b00 DPL=0 CS64 [-RA]
> SS =   00c0
> DS =   00c0
> FS =   00c0
> GS = 81f34000  00c0
> LDT=   00c0
> TR =0020  0fff 00808b00 DPL=0 TSS64-busy
> GDT= 81f3c000 007f
> IDT= 83265000 0fff
> CR0=80050033 CR2=880001fa6ff8 CR3=01fa6000 CR4=000406a0
> DR0= DR1= DR2=
> DR3=
> DR6=0ff0 DR7=0400
> EFER=0d01
> Code=cc cc cc cc cc cc cc cc cc cc cc cc b8 11 00 00 00 0f 01 c1  cc
> cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc b8 12
> 00 00 00 0f
> 
> Signed-off-by: Joao Martins 
> ---
>  arch/x86/include/asm/kvm_host.h | 13 +++
>  arch/x86/kvm/Makefile   |  2 +-
>  arch/x86/kvm/trace.h| 33 +
>  arch/x86/kvm/x86.c  | 12 +++
>  arch/x86/kvm/xen.c  | 79 
> +
>  arch/x86/kvm/xen.h  | 10 ++
>  include/uapi/linux/kvm.h| 17 -
>  7 files changed, 164 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/kvm/xen.c
>  create mode 100644 arch/x86/kvm/xen.h

...

> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 31ecf7a76d5a..2b46c93c9380 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -10,7 +10,7 @@ kvm-$(CONFIG_KVM_ASYNC_PF)  += $(KVM)/async_pf.o
>  
>  kvm-y+= x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
>  i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
> -hyperv.o page_track.o debugfs.o
> +hyperv.o xen.o page_track.o debugfs.o

Can this be wrapped in a config?  Or even better, as a loadable module?
2k+ lines of code is a non-trival amount of baggage for folks that don't
care about running Xen guests.  I've only glanced through the series, so
I've no idea if the resulting code would be an abomination.

>  
>  kvm-intel-y  += vmx/vmx.o vmx/vmenter.o vmx/pmu_intel.o vmx/vmcs12.o 
> vmx/evmcs.o vmx/nested.o
>  kvm-amd-y+= svm.o pmu_amd.o