From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs. Date: Sun, 17 Jan 2010 14:20:32 +0200 Message-ID: <4B530090.50506@redhat.com> References: <1263719028-24765-1-git-send-email-gleb@redhat.com> <1263719028-24765-4-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56745 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751809Ab0AQMUf (ORCPT ); Sun, 17 Jan 2010 07:20:35 -0500 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0HCKYqe030339 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 17 Jan 2010 07:20:35 -0500 In-Reply-To: <1263719028-24765-4-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/17/2010 11:03 AM, Gleb Natapov wrote: > Signed-off-by: Gleb Natapov > Signed-off-by: Vadim Rozenfeld > Changelog entry. > > struct kvm_mem_alias { > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index ba8c045..4b224f9 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -1246,3 +1246,34 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data) > > return 0; > } > + > +int kvm_hv_vapic_msr_write(struct kvm_vcpu *vcpu, u32 reg, u64 data) > +{ > + struct kvm_lapic *apic = vcpu->arch.apic; > + > + if (!irqchip_in_kernel(vcpu->kvm)) > + return 1; > + > + /* if this is ICR write vector before command */ > + if (reg == APIC_ICR) > + apic_reg_write(apic, APIC_ICR2, (u32)(data>> 32)); > + return apic_reg_write(apic, reg, (u32)data); > +} > + > +int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 reg, u64 *data) > +{ > + struct kvm_lapic *apic = vcpu->arch.apic; > + u32 low, high = 0; > + > + if (!irqchip_in_kernel(vcpu->kvm)) > + return 1; > + > + if (apic_reg_read(apic, reg, 4,&low)) > + return 1; > + if (reg == APIC_ICR) > + apic_reg_read(apic, APIC_ICR2, 4,&high); > + > + *data = (((u64)high)<< 32) | low; > + > + return 0; > +} > I prefer putting this in x86.c (maybe later split into hyperv.c). > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 40010b0..d081cb6 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -48,4 +48,12 @@ void kvm_lapic_sync_to_vapic(struct kvm_vcpu *vcpu); > > int kvm_x2apic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data); > int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data); > + > +int kvm_hv_vapic_msr_write(struct kvm_vcpu *vcpu, u32 msr, u64 data); > +int kvm_hv_vapic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data); > + > +static inline bool kvm_hv_vapic_enabled(struct kvm_vcpu *vcpu) > +{ > + return !!(vcpu->arch.hv_vapic& HV_X64_MSR_APIC_ASSIST_PAGE_ENABLE); > +} > Are you sure that vapic enabled is equivalent to apic assist page enable? !! not required. > #endif > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index db0b2b1..2fe555c 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -632,6 +632,7 @@ static u32 msrs_to_save[] = { > #endif > 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, > + HV_X64_MSR_APIC_ASSIST_PAGE, > }; > > Will be dropped by msr validation. > static unsigned num_msrs_to_save; > @@ -1063,10 +1064,37 @@ static int set_msr_hyperv_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data) > > static int set_msr_hyperv(struct kvm_vcpu *vcpu, u32 msr, u64 data) > { > - pr_unimpl(vcpu, "HYPER-V unimplemented wrmsr: 0x%x data 0x%llx\n", > - msr, data); > + switch (msr) { > + case HV_X64_MSR_APIC_ASSIST_PAGE: { > + unsigned long vaddr; > + void *addr; > + struct page *page; > + vcpu->arch.hv_vapic = data; > + if (!kvm_hv_vapic_enabled(vcpu)) > + break; > + vaddr = gfn_to_hva(vcpu->kvm, data>> > + HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT); > + if (kvm_is_error_hva(vaddr)) > + return 1; > + page = virt_to_page(vaddr); > virt_to_page() takes a kernel address, not a user address. This is get_user_pages(). But I think the whole thing is done better with put_user(). > + addr = kmap_atomic(page, KM_USER0); > + clear_user_page(addr, vaddr, page); > + kunmap_atomic(addr, KM_USER0); > Surprising that clear_user_page needs kmap_atomic() (but true). -- error compiling committee.c: too many arguments to function