On Sun, Jan 17, 2010 at 02:46:46PM +0200, Avi Kivity wrote:
> 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?
>
Each hyper-v extension is pretty much independent one from another, so
why not group things by functionality instead. All apic related code in
lapic.c.
> Maybe apic_reg_{read,write} need to be exported.
>
This is really internal API. It doesn't even check if apic is created.
> >>> 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().
>
Doh.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
