* [PATCH v2 1/2] KVM: nVMX: get rid of nested_get_page()
2017-08-03 16:11 [PATCH v2 0/2] KVM: nVMX: remove nested_get_page() David Hildenbrand
@ 2017-08-03 16:11 ` David Hildenbrand
2017-08-03 16:11 ` [PATCH v2 2/2] KVM: nVMX: get rid of nested_release_page* David Hildenbrand
2017-08-03 17:55 ` [PATCH v2 0/2] KVM: nVMX: remove nested_get_page() Radim Krčmář
2 siblings, 0 replies; 4+ messages in thread
From: David Hildenbrand @ 2017-08-03 16:11 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Radim Krčmář, david
nested_get_page() just sounds confusing. All we want is a page from G1.
This is even unrelated to nested.
Let's introduce kvm_vcpu_gpa_to_page() so we don't get too lengthy
lines.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/kvm/vmx.c | 57 ++++++++++++++++++++++--------------------------
include/linux/kvm_host.h | 6 +++++
2 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 39a6222..552acdd 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -894,14 +894,6 @@ static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
return to_vmx(vcpu)->nested.cached_vmcs12;
}
-static struct page *nested_get_page(struct kvm_vcpu *vcpu, gpa_t addr)
-{
- struct page *page = kvm_vcpu_gfn_to_page(vcpu, addr >> PAGE_SHIFT);
- if (is_error_page(page))
- return NULL;
-
- return page;
-}
static void nested_release_page(struct page *page)
{
@@ -7095,8 +7087,8 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
return kvm_skip_emulated_instruction(vcpu);
}
- page = nested_get_page(vcpu, vmptr);
- if (page == NULL) {
+ page = kvm_vcpu_gpa_to_page(vcpu, vmptr);
+ if (is_error_page(page)) {
nested_vmx_failInvalid(vcpu);
return kvm_skip_emulated_instruction(vcpu);
}
@@ -7564,8 +7556,8 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
if (vmx->nested.current_vmptr != vmptr) {
struct vmcs12 *new_vmcs12;
struct page *page;
- page = nested_get_page(vcpu, vmptr);
- if (page == NULL) {
+ page = kvm_vcpu_gpa_to_page(vcpu, vmptr);
+ if (is_error_page(page)) {
nested_vmx_failInvalid(vcpu);
return kvm_skip_emulated_instruction(vcpu);
}
@@ -9524,6 +9516,7 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
+ struct page *page;
u64 hpa;
if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
@@ -9533,17 +9526,19 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
* physical address remains valid. We keep a reference
* to it so we can release it later.
*/
- if (vmx->nested.apic_access_page) /* shouldn't happen */
+ if (vmx->nested.apic_access_page) { /* shouldn't happen */
nested_release_page(vmx->nested.apic_access_page);
- vmx->nested.apic_access_page =
- nested_get_page(vcpu, vmcs12->apic_access_addr);
+ vmx->nested.apic_access_page = NULL;
+ }
+ page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->apic_access_addr);
/*
* If translation failed, no matter: This feature asks
* to exit when accessing the given address, and if it
* can never be accessed, this feature won't do
* anything anyway.
*/
- if (vmx->nested.apic_access_page) {
+ if (!is_error_page(page)) {
+ vmx->nested.apic_access_page = page;
hpa = page_to_phys(vmx->nested.apic_access_page);
vmcs_write64(APIC_ACCESS_ADDR, hpa);
} else {
@@ -9558,10 +9553,11 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
}
if (nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW)) {
- if (vmx->nested.virtual_apic_page) /* shouldn't happen */
+ if (vmx->nested.virtual_apic_page) { /* shouldn't happen */
nested_release_page(vmx->nested.virtual_apic_page);
- vmx->nested.virtual_apic_page =
- nested_get_page(vcpu, vmcs12->virtual_apic_page_addr);
+ vmx->nested.virtual_apic_page = NULL;
+ }
+ page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->apic_access_addr);
/*
* If translation failed, VM entry will fail because
@@ -9576,7 +9572,8 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
* control. But such a configuration is useless, so
* let's keep the code simple.
*/
- if (vmx->nested.virtual_apic_page) {
+ if (!is_error_page(page)) {
+ vmx->nested.virtual_apic_page = page;
hpa = page_to_phys(vmx->nested.virtual_apic_page);
vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, hpa);
}
@@ -9586,15 +9583,13 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
if (vmx->nested.pi_desc_page) { /* shouldn't happen */
kunmap(vmx->nested.pi_desc_page);
nested_release_page(vmx->nested.pi_desc_page);
+ vmx->nested.pi_desc_page = NULL;
}
- vmx->nested.pi_desc_page =
- nested_get_page(vcpu, vmcs12->posted_intr_desc_addr);
- vmx->nested.pi_desc =
- (struct pi_desc *)kmap(vmx->nested.pi_desc_page);
- if (!vmx->nested.pi_desc) {
- nested_release_page_clean(vmx->nested.pi_desc_page);
+ page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->posted_intr_desc_addr);
+ if (is_error_page(page))
return;
- }
+ vmx->nested.pi_desc_page = page;
+ vmx->nested.pi_desc = kmap(vmx->nested.pi_desc_page);
vmx->nested.pi_desc =
(struct pi_desc *)((void *)vmx->nested.pi_desc +
(unsigned long)(vmcs12->posted_intr_desc_addr &
@@ -9676,8 +9671,8 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
if (!nested_cpu_has_virt_x2apic_mode(vmcs12))
return false;
- page = nested_get_page(vcpu, vmcs12->msr_bitmap);
- if (!page)
+ page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->msr_bitmap);
+ if (is_error_page(page))
return false;
msr_bitmap_l1 = (unsigned long *)kmap(page);
@@ -11287,8 +11282,8 @@ static int vmx_write_pml_buffer(struct kvm_vcpu *vcpu)
gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS) & ~0xFFFull;
- page = nested_get_page(vcpu, vmcs12->pml_address);
- if (!page)
+ page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->pml_address);
+ if (is_error_page(page))
return 0;
pml_address = kmap(page);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 890b706..6ed27df 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -983,6 +983,12 @@ static inline hpa_t pfn_to_hpa(kvm_pfn_t pfn)
return (hpa_t)pfn << PAGE_SHIFT;
}
+static inline struct page *kvm_vcpu_gpa_to_page(struct kvm_vcpu *vcpu,
+ gpa_t gpa)
+{
+ return kvm_vcpu_gfn_to_page(vcpu, gpa_to_gfn(gpa));
+}
+
static inline bool kvm_is_error_gpa(struct kvm *kvm, gpa_t gpa)
{
unsigned long hva = gfn_to_hva(kvm, gpa_to_gfn(gpa));
--
2.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/2] KVM: nVMX: get rid of nested_release_page*
2017-08-03 16:11 [PATCH v2 0/2] KVM: nVMX: remove nested_get_page() David Hildenbrand
2017-08-03 16:11 ` [PATCH v2 1/2] KVM: nVMX: get rid of nested_get_page() David Hildenbrand
@ 2017-08-03 16:11 ` David Hildenbrand
2017-08-03 17:55 ` [PATCH v2 0/2] KVM: nVMX: remove nested_get_page() Radim Krčmář
2 siblings, 0 replies; 4+ messages in thread
From: David Hildenbrand @ 2017-08-03 16:11 UTC (permalink / raw)
To: kvm; +Cc: Paolo Bonzini, Radim Krčmář, david
Let's also just use the underlying functions directly here.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/x86/kvm/vmx.c | 41 +++++++++++++++--------------------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 552acdd..877fb51 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -894,17 +894,6 @@ static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
return to_vmx(vcpu)->nested.cached_vmcs12;
}
-
-static void nested_release_page(struct page *page)
-{
- kvm_release_page_dirty(page);
-}
-
-static void nested_release_page_clean(struct page *page)
-{
- kvm_release_page_clean(page);
-}
-
static bool nested_ept_ad_enabled(struct kvm_vcpu *vcpu);
static unsigned long nested_ept_get_cr3(struct kvm_vcpu *vcpu);
static u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa);
@@ -7094,12 +7083,12 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
}
if (*(u32 *)kmap(page) != VMCS12_REVISION) {
kunmap(page);
- nested_release_page_clean(page);
+ kvm_release_page_clean(page);
nested_vmx_failInvalid(vcpu);
return kvm_skip_emulated_instruction(vcpu);
}
kunmap(page);
- nested_release_page_clean(page);
+ kvm_release_page_clean(page);
vmx->nested.vmxon_ptr = vmptr;
ret = enter_vmx_operation(vcpu);
@@ -7151,7 +7140,7 @@ static inline void nested_release_vmcs12(struct vcpu_vmx *vmx)
VMCS12_SIZE);
kunmap(vmx->nested.current_vmcs12_page);
- nested_release_page(vmx->nested.current_vmcs12_page);
+ kvm_release_page_dirty(vmx->nested.current_vmcs12_page);
vmx->nested.current_vmptr = -1ull;
vmx->nested.current_vmcs12 = NULL;
}
@@ -7180,16 +7169,16 @@ static void free_nested(struct vcpu_vmx *vmx)
kfree(vmx->nested.cached_vmcs12);
/* Unpin physical memory we referred to in current vmcs02 */
if (vmx->nested.apic_access_page) {
- nested_release_page(vmx->nested.apic_access_page);
+ kvm_release_page_dirty(vmx->nested.apic_access_page);
vmx->nested.apic_access_page = NULL;
}
if (vmx->nested.virtual_apic_page) {
- nested_release_page(vmx->nested.virtual_apic_page);
+ kvm_release_page_dirty(vmx->nested.virtual_apic_page);
vmx->nested.virtual_apic_page = NULL;
}
if (vmx->nested.pi_desc_page) {
kunmap(vmx->nested.pi_desc_page);
- nested_release_page(vmx->nested.pi_desc_page);
+ kvm_release_page_dirty(vmx->nested.pi_desc_page);
vmx->nested.pi_desc_page = NULL;
vmx->nested.pi_desc = NULL;
}
@@ -7564,7 +7553,7 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
new_vmcs12 = kmap(page);
if (new_vmcs12->revision_id != VMCS12_REVISION) {
kunmap(page);
- nested_release_page_clean(page);
+ kvm_release_page_clean(page);
nested_vmx_failValid(vcpu,
VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
return kvm_skip_emulated_instruction(vcpu);
@@ -9527,7 +9516,7 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
* to it so we can release it later.
*/
if (vmx->nested.apic_access_page) { /* shouldn't happen */
- nested_release_page(vmx->nested.apic_access_page);
+ kvm_release_page_dirty(vmx->nested.apic_access_page);
vmx->nested.apic_access_page = NULL;
}
page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->apic_access_addr);
@@ -9554,7 +9543,7 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
if (nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW)) {
if (vmx->nested.virtual_apic_page) { /* shouldn't happen */
- nested_release_page(vmx->nested.virtual_apic_page);
+ kvm_release_page_dirty(vmx->nested.virtual_apic_page);
vmx->nested.virtual_apic_page = NULL;
}
page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->apic_access_addr);
@@ -9582,7 +9571,7 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu,
if (nested_cpu_has_posted_intr(vmcs12)) {
if (vmx->nested.pi_desc_page) { /* shouldn't happen */
kunmap(vmx->nested.pi_desc_page);
- nested_release_page(vmx->nested.pi_desc_page);
+ kvm_release_page_dirty(vmx->nested.pi_desc_page);
vmx->nested.pi_desc_page = NULL;
}
page = kvm_vcpu_gpa_to_page(vcpu, vmcs12->posted_intr_desc_addr);
@@ -9702,7 +9691,7 @@ static inline bool nested_vmx_merge_msr_bitmap(struct kvm_vcpu *vcpu,
}
}
kunmap(page);
- nested_release_page_clean(page);
+ kvm_release_page_clean(page);
return true;
}
@@ -11097,16 +11086,16 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
/* Unpin physical memory we referred to in vmcs02 */
if (vmx->nested.apic_access_page) {
- nested_release_page(vmx->nested.apic_access_page);
+ kvm_release_page_dirty(vmx->nested.apic_access_page);
vmx->nested.apic_access_page = NULL;
}
if (vmx->nested.virtual_apic_page) {
- nested_release_page(vmx->nested.virtual_apic_page);
+ kvm_release_page_dirty(vmx->nested.virtual_apic_page);
vmx->nested.virtual_apic_page = NULL;
}
if (vmx->nested.pi_desc_page) {
kunmap(vmx->nested.pi_desc_page);
- nested_release_page(vmx->nested.pi_desc_page);
+ kvm_release_page_dirty(vmx->nested.pi_desc_page);
vmx->nested.pi_desc_page = NULL;
vmx->nested.pi_desc = NULL;
}
@@ -11289,7 +11278,7 @@ static int vmx_write_pml_buffer(struct kvm_vcpu *vcpu)
pml_address = kmap(page);
pml_address[vmcs12->guest_pml_index--] = gpa;
kunmap(page);
- nested_release_page_clean(page);
+ kvm_release_page_clean(page);
}
return 0;
--
2.9.4
^ permalink raw reply related [flat|nested] 4+ messages in thread