On 01/13/2010 03:59 PM, Gleb Natapov wrote:
Minimum HYPER-V implementation should have GUEST_OS_ID, HYPERCALL and
VP_INDEX MSRs.


diff --git a/arch/x86/include/asm/kvm_hyperv.h 
b/arch/x86/include/asm/kvm_hyperv.h
new file mode 100644
index 0000000..91211f3
--- /dev/null
+++ b/arch/x86/include/asm/kvm_hyperv.h

Please name this asm/hyperv.h, so it can be used for Linux-as-Hyper-V-guest, not just Linux-as-host-impersonating-Hyper-V.+

Also put this in a separate patch.


diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6811e5e..6972b2b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -628,7 +628,8 @@ static u32 msrs_to_save[] = {
  #ifdef CONFIG_X86_64
        MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
  #endif
-       MSR_IA32_TSC, MSR_IA32_PERF_STATUS, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA
+       MSR_IA32_TSC, MSR_IA32_PERF_STATUS, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
+       HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL

End with trailing comma so future patches are nicer.


+static bool kvm_hv_hypercall_enabled(struct kvm *kvm)
+{
+       return !!(kvm->arch.hv_hypercall&  HV_X64_MSR_HYPERCALL_ENABLE);
+}

!! is unnecessary for bool:

_Bool and(unsigned x) { return x & 16; }

0000000000000010 <and>:
  10:   c1 ef 04                shr    $0x4,%edi
  13:   89 f8                   mov    %edi,%eax
  15:   83 e0 01                and    $0x1,%eax
  18:   c3                      retq



+
+static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data)
+{
+       struct kvm *kvm = vcpu->kvm;
+
+       switch (msr) {
+       case HV_X64_MSR_GUEST_OS_ID:
+               kvm->arch.hv_guest_os_id = data;
+               /* setting guest os id to zero disables hypercall page */
+               if (!kvm->arch.hv_guest_os_id)
+                       kvm->arch.hv_hypercall&= ~HV_X64_MSR_HYPERCALL_ENABLE;
+               break;
+       case HV_X64_MSR_HYPERCALL: {
+               u64 gfn;
+               unsigned long addr;
+               /* if guest os id is not set hypercall should remain disabled */
+               if (!kvm->arch.hv_guest_os_id&&  data)
+                       break;
+               kvm->arch.hv_hypercall = data;
+               if(!kvm_hv_hypercall_enabled(kvm))
+                       break;
+               gfn = kvm->arch.hv_hypercall>>
+                       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, (unsigned char*)addr);
+               ((unsigned char*)addr)[3] = 0xc3; /* ret */
+               break;
+       }
+       default:
+               pr_unimpl(vcpu, "HYPER-V unimplemented wrmsr: 0x%x "
+                         "data 0x%llx\n", msr, data);
+               return 1;
+       }
+       return 0;
+}

We need locking in case a malicious guest issues partition-wide msrs from multiple vcpus simultaneously.

  int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
  {
        switch (msr) {
@@ -1117,6 +1181,16 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, 
u64 data)
                pr_unimpl(vcpu, "unimplemented perfctr wrmsr: "
                        "0x%x data 0x%llx\n", msr, data);
                break;
+       case HV_X64_MSR_GUEST_OS_ID ... HV_X64_MSR_SINT15:
+               if (kvm_hv_msr_partition_wide(msr)) {
+                       int r;
+                       mutex_lock(&vcpu->kvm->lock);
+                       r = set_msr_hyperv_pw(vcpu, msr, data);
+                       mutex_unlock(&vcpu->kvm->lock);

We do have locking. Any reason not to put it in set_msr_hyperv_pw? Seems cleaner.

+static int get_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
+{
+       u64 data = 0;
+       struct kvm *kvm = vcpu->kvm;
+
+       switch (msr) {
+       case HV_X64_MSR_GUEST_OS_ID:
+               data = kvm->arch.hv_guest_os_id;
+               break;
+       case HV_X64_MSR_HYPERCALL:
+               data = kvm->arch.hv_hypercall;
+               break;

This could be non-atomic on i386.  I don't think it matters.

  int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
  {
        unsigned long nr, a0, a1, a2, a3, ret;
        int r = 1;

+       if(kvm_hv_hypercall_enabled(vcpu->kvm))
+               return kvm_hv_hypercall(vcpu);
+

Space after if.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to