[PATCHv2 3/4] Add HYPER-V apic access MSRs.
Signed-off-by: Gleb Natapov g...@redhat.com
Signed-off-by: Vadim Rozenfeld vroze...@redhat.com
---
arch/x86/include/asm/kvm_host.h |2 +
arch/x86/kvm/lapic.c| 31 +
arch/x86/kvm/lapic.h|8 +++
arch/x86/kvm/x86.c | 41 --
include/linux/kvm.h |1 +
5 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 67d19e4..a1f0b5d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -363,6 +363,8 @@ struct kvm_vcpu_arch {
/* used for guest single stepping over the given code position */
u16 singlestep_cs;
unsigned long singlestep_rip;
+ /* fields used by HYPER-V emulation */
+ u64 hv_vapic;
};
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;
+}
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);
+}
#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,
};
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);
+ addr = kmap_atomic(page, KM_USER0);
+ clear_user_page(addr, vaddr, page);
+ kunmap_atomic(addr, KM_USER0);
+ break;
+ }
+ case HV_X64_MSR_EOI:
+ return kvm_hv_vapic_msr_write(vcpu, APIC_EOI, data);
+ case HV_X64_MSR_ICR:
+ return kvm_hv_vapic_msr_write(vcpu, APIC_ICR, data);
+ case HV_X64_MSR_TPR:
+ return kvm_hv_vapic_msr_write(vcpu, APIC_TASKPRI, data);
+ default:
+ pr_unimpl(vcpu, HYPER-V unimplemented wrmsr: 0x%x
+ data 0x%llx\n, msr, data);
+ return 1;
+ }
- return 1;
+ return 0;
}
int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
@@ -1326,6 +1354,12 @@ static int get_msr_hyperv(struct kvm_vcpu *vcpu, u32
msr, u64 *pdata)
data = r;
break;
}
+ case HV_X64_MSR_EOI:
+ return kvm_hv_vapic_msr_read(vcpu, APIC_EOI, pdata);
+ case HV_X64_MSR_ICR:
+ return kvm_hv_vapic_msr_read(vcpu, APIC_ICR,
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On 01/17/2010 11:03 AM, Gleb Natapov wrote:
Signed-off-by: Gleb Natapovg...@redhat.com
Signed-off-by: Vadim Rozenfeldvroze...@redhat.com
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On Sun, Jan 17, 2010 at 02:20:32PM +0200, Avi Kivity wrote: +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). There's a clear_user_highpage helper to take care of it for you. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On Sun, 2010-01-17 at 14:20 +0200, Avi Kivity wrote:
On 01/17/2010 11:03 AM, Gleb Natapov wrote:
Signed-off-by: Gleb Natapovg...@redhat.com
Signed-off-by: Vadim Rozenfeldvroze...@redhat.com
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?
At least, when it is disable, the EOI interception mechanism won't
work.
!! 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).
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On Sun, Jan 17, 2010 at 02:20:32PM +0200, Avi Kivity wrote:
On 01/17/2010 11:03 AM, Gleb Natapov wrote:
Signed-off-by: Gleb Natapovg...@redhat.com
Signed-off-by: Vadim Rozenfeldvroze...@redhat.com
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).
This implements part of apic behaviour. It uses internal lapic functions
like apic_reg_read()/apic_reg_write(). Why move it from lapic.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.
The function is not used to check if vapic is enabled, so the name
should be changed.
#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().
So there is no function to get struct page from user virtual address?
+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
--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On Sun, Jan 17, 2010 at 07:32:32AM -0500, Christoph Hellwig wrote: On Sun, Jan 17, 2010 at 02:20:32PM +0200, Avi Kivity wrote: + 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). There's a clear_user_highpage helper to take care of it for you. I copied code from the instead of using helper faction for some unknown to me reason. Anyway if I can't get struct page from user virtual address I can't use it. Actually I am not sure the page should be zeroed at all. Spec only descries first dword of the page and doesn't require zeroing the reset as far as I see. -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On Sun, Jan 17, 2010 at 02:41:42PM +0200, Gleb Natapov wrote: I copied code from the instead of using helper faction for some unknown to me reason. Anyway if I can't get struct page from user virtual address I can't use it. Actually I am not sure the page should be zeroed at all. Spec only descries first dword of the page and doesn't require zeroing the reset as far as I see. There's a clear_user() function that just takes a user virtual address and a size. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On Sun, Jan 17, 2010 at 07:44:57AM -0500, Christoph Hellwig wrote: On Sun, Jan 17, 2010 at 02:41:42PM +0200, Gleb Natapov wrote: I copied code from the instead of using helper faction for some unknown to me reason. Anyway if I can't get struct page from user virtual address I can't use it. Actually I am not sure the page should be zeroed at all. Spec only descries first dword of the page and doesn't require zeroing the reset as far as I see. There's a clear_user() function that just takes a user virtual address and a size. Precisely what I need. Thanks! -- Gleb. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On 01/17/2010 02:46 PM, Gleb Natapov wrote: On Sun, Jan 17, 2010 at 07:44:57AM -0500, Christoph Hellwig wrote: On Sun, Jan 17, 2010 at 02:41:42PM +0200, Gleb Natapov wrote: I copied code from the instead of using helper faction for some unknown to me reason. Anyway if I can't get struct page from user virtual address I can't use it. Actually I am not sure the page should be zeroed at all. Spec only descries first dword of the page and doesn't require zeroing the reset as far as I see. There's a clear_user() function that just takes a user virtual address and a size. Precisely what I need. Thanks! But clear_user_page() is confusingly named. I assume it deals with virtually tagged caches on archs that have them? -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/4] Add HYPER-V apic access MSRs.
On 01/17/2010 02:50 PM, Gleb Natapov wrote:
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.
Okay. We can rethink it later when the code grows some more.
--
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
