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:46:46 +0200 Message-ID: <4B5306B6.8050603@redhat.com> References: <1263719028-24765-1-git-send-email-gleb@redhat.com> <1263719028-24765-4-git-send-email-gleb@redhat.com> <4B530090.50506@redhat.com> <20100117123626.GB31692@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]:64785 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750718Ab0AQMqr (ORCPT ); Sun, 17 Jan 2010 07:46:47 -0500 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o0HCkl6R018546 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 17 Jan 2010 07:46:47 -0500 In-Reply-To: <20100117123626.GB31692@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/17/2010 02:36 PM, Gleb Natapov wrote: > + >>> +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). >> >> > This implements part of apic behaviour. It uses internal lapic functions > like apic_reg_read()/apic_reg_write(). Why move it from lapic.c? > The new functions implement hyper-v behaviour. Why scatter them all around? Maybe apic_reg_{read,write} need to be exported. >>> 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(). >> >> > So there is no function to get struct page from user virtual address? > get_user_pages_fast(). -- error compiling committee.c: too many arguments to function