Re: [PATCH v7 9/9] kvm, mem-hotplug: Unpin and remove kvm_arch->apic_access_page.

2014-09-22 Thread Paolo Bonzini
Il 20/09/2014 12:47, Tang Chen ha scritto:
> @@ -4534,8 +4539,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>   }
>  
>   if (vm_need_virtualize_apic_accesses(vmx->vcpu.kvm))
> - vmcs_write64(APIC_ACCESS_ADDR,
> -  
> page_to_phys(vmx->vcpu.kvm->arch.apic_access_page));
> + kvm_vcpu_reload_apic_access_page(vcpu);
>  
>   if (vmx_vm_has_apicv(vcpu->kvm))
>   memset(>pi_desc, 0, sizeof(struct pi_desc));

Please make the call unconditional in kvm_vcpu_reset.

> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 9c8ae32..99378d7 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3624,6 +3624,11 @@ static bool svm_has_secondary_apic_access(struct 
> kvm_vcpu *vcpu)
>   return false;
>  }
>  
> +static void svm_set_apic_access_page_addr(struct kvm *kvm, hpa_t hpa)
> +{
> + return;
> +}
> +
>  static int svm_vm_has_apicv(struct kvm *kvm)
>  {
>   return 0;

I will ask you to modify the vmx_has_secondary_apic_access callback in
such a way that svm_set_apic_access_page_addr is not needed, so please
remove it from v8.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 9/9] kvm, mem-hotplug: Unpin and remove kvm_arch-apic_access_page.

2014-09-22 Thread Paolo Bonzini
Il 20/09/2014 12:47, Tang Chen ha scritto:
 @@ -4534,8 +4539,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu)
   }
  
   if (vm_need_virtualize_apic_accesses(vmx-vcpu.kvm))
 - vmcs_write64(APIC_ACCESS_ADDR,
 -  
 page_to_phys(vmx-vcpu.kvm-arch.apic_access_page));
 + kvm_vcpu_reload_apic_access_page(vcpu);
  
   if (vmx_vm_has_apicv(vcpu-kvm))
   memset(vmx-pi_desc, 0, sizeof(struct pi_desc));

Please make the call unconditional in kvm_vcpu_reset.

 
 diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
 index 9c8ae32..99378d7 100644
 --- a/arch/x86/kvm/svm.c
 +++ b/arch/x86/kvm/svm.c
 @@ -3624,6 +3624,11 @@ static bool svm_has_secondary_apic_access(struct 
 kvm_vcpu *vcpu)
   return false;
  }
  
 +static void svm_set_apic_access_page_addr(struct kvm *kvm, hpa_t hpa)
 +{
 + return;
 +}
 +
  static int svm_vm_has_apicv(struct kvm *kvm)
  {
   return 0;

I will ask you to modify the vmx_has_secondary_apic_access callback in
such a way that svm_set_apic_access_page_addr is not needed, so please
remove it from v8.

Paolo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v7 9/9] kvm, mem-hotplug: Unpin and remove kvm_arch->apic_access_page.

2014-09-20 Thread Tang Chen
To make apic access page migratable, we do not pin it in memory now.
When it is migrated, we should reload its physical address for all
vmcses. But when we tried to do this, all vcpu will access
kvm_arch->apic_access_page without any locking. This is not safe.

Actually, we do not need kvm_arch->apic_access_page anymore. Since
apic access page is not pinned in memory now, we can remove
kvm_arch->apic_access_page. When we need to write its physical address
into vmcs, use gfn_to_page() to get its page struct, which will also
pin it. And unpin it after then.

Suggested-by: Gleb Natapov 
Signed-off-by: Tang Chen 
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/vmx.c  | 15 +--
 arch/x86/kvm/x86.c  | 16 +++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1a8317e..9fb3d4c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -576,7 +576,7 @@ struct kvm_arch {
struct kvm_apic_map *apic_map;
 
unsigned int tss_addr;
-   struct page *apic_access_page;
+   bool apic_access_page_done;
 
gpa_t wall_clock;
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index baac78a..12f0715 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4002,7 +4002,7 @@ static int alloc_apic_access_page(struct kvm *kvm)
int r = 0;
 
mutex_lock(>slots_lock);
-   if (kvm->arch.apic_access_page)
+   if (kvm->arch.apic_access_page_done)
goto out;
kvm_userspace_mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT;
kvm_userspace_mem.flags = 0;
@@ -4018,7 +4018,12 @@ static int alloc_apic_access_page(struct kvm *kvm)
goto out;
}
 
-   kvm->arch.apic_access_page = page;
+   /*
+* Do not pin apic access page in memory so that memory hotplug
+* process is able to migrate it.
+*/
+   put_page(page);
+   kvm->arch.apic_access_page_done = true;
 out:
mutex_unlock(>slots_lock);
return r;
@@ -4534,8 +4539,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu)
}
 
if (vm_need_virtualize_apic_accesses(vmx->vcpu.kvm))
-   vmcs_write64(APIC_ACCESS_ADDR,
-
page_to_phys(vmx->vcpu.kvm->arch.apic_access_page));
+   kvm_vcpu_reload_apic_access_page(vcpu);
 
if (vmx_vm_has_apicv(vcpu->kvm))
memset(>pi_desc, 0, sizeof(struct pi_desc));
@@ -8003,8 +8007,7 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct 
vmcs12 *vmcs12)
} else if (vm_need_virtualize_apic_accesses(vmx->vcpu.kvm)) {
exec_control |=
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
-   vmcs_write64(APIC_ACCESS_ADDR,
-   page_to_phys(vcpu->kvm->arch.apic_access_page));
+   kvm_vcpu_reload_apic_access_page(vcpu);
}
 
vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7dd4179..996af6e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5991,6 +5991,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
 
 void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu)
 {
+   struct page *page = NULL;
+
/*
 * Only APIC access page shared by L1 and L2 vm is handled. The APIC
 * access page prepared by L1 for L2's execution is still pinned in
@@ -6003,10 +6005,16 @@ void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu 
*vcpu)
 * migrated, GUP will wait till the migrate entry is replaced
 * with the new pte entry pointing to the new page.
 */
-   vcpu->kvm->arch.apic_access_page = gfn_to_page(vcpu->kvm,
-   APIC_DEFAULT_PHYS_BASE >> PAGE_SHIFT);
+   page = gfn_to_page(vcpu->kvm,
+  APIC_DEFAULT_PHYS_BASE >> PAGE_SHIFT);
kvm_x86_ops->set_apic_access_page_addr(vcpu->kvm,
-   page_to_phys(vcpu->kvm->arch.apic_access_page));
+  page_to_phys(page));
+
+   /*
+* Do not pin apic access page in memory so that memory hotplug
+* process is able to migrate it.
+*/
+   put_page(page);
}
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_reload_apic_access_page);
@@ -7272,8 +7280,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
kfree(kvm->arch.vpic);
kfree(kvm->arch.vioapic);
kvm_free_vcpus(kvm);
-   if (kvm->arch.apic_access_page)
-   put_page(kvm->arch.apic_access_page);
kfree(rcu_dereference_check(kvm->arch.apic_map, 1));
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line 

[PATCH v7 9/9] kvm, mem-hotplug: Unpin and remove kvm_arch-apic_access_page.

2014-09-20 Thread Tang Chen
To make apic access page migratable, we do not pin it in memory now.
When it is migrated, we should reload its physical address for all
vmcses. But when we tried to do this, all vcpu will access
kvm_arch-apic_access_page without any locking. This is not safe.

Actually, we do not need kvm_arch-apic_access_page anymore. Since
apic access page is not pinned in memory now, we can remove
kvm_arch-apic_access_page. When we need to write its physical address
into vmcs, use gfn_to_page() to get its page struct, which will also
pin it. And unpin it after then.

Suggested-by: Gleb Natapov g...@kernel.org
Signed-off-by: Tang Chen tangc...@cn.fujitsu.com
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/vmx.c  | 15 +--
 arch/x86/kvm/x86.c  | 16 +++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1a8317e..9fb3d4c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -576,7 +576,7 @@ struct kvm_arch {
struct kvm_apic_map *apic_map;
 
unsigned int tss_addr;
-   struct page *apic_access_page;
+   bool apic_access_page_done;
 
gpa_t wall_clock;
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index baac78a..12f0715 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4002,7 +4002,7 @@ static int alloc_apic_access_page(struct kvm *kvm)
int r = 0;
 
mutex_lock(kvm-slots_lock);
-   if (kvm-arch.apic_access_page)
+   if (kvm-arch.apic_access_page_done)
goto out;
kvm_userspace_mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT;
kvm_userspace_mem.flags = 0;
@@ -4018,7 +4018,12 @@ static int alloc_apic_access_page(struct kvm *kvm)
goto out;
}
 
-   kvm-arch.apic_access_page = page;
+   /*
+* Do not pin apic access page in memory so that memory hotplug
+* process is able to migrate it.
+*/
+   put_page(page);
+   kvm-arch.apic_access_page_done = true;
 out:
mutex_unlock(kvm-slots_lock);
return r;
@@ -4534,8 +4539,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu)
}
 
if (vm_need_virtualize_apic_accesses(vmx-vcpu.kvm))
-   vmcs_write64(APIC_ACCESS_ADDR,
-
page_to_phys(vmx-vcpu.kvm-arch.apic_access_page));
+   kvm_vcpu_reload_apic_access_page(vcpu);
 
if (vmx_vm_has_apicv(vcpu-kvm))
memset(vmx-pi_desc, 0, sizeof(struct pi_desc));
@@ -8003,8 +8007,7 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct 
vmcs12 *vmcs12)
} else if (vm_need_virtualize_apic_accesses(vmx-vcpu.kvm)) {
exec_control |=
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
-   vmcs_write64(APIC_ACCESS_ADDR,
-   page_to_phys(vcpu-kvm-arch.apic_access_page));
+   kvm_vcpu_reload_apic_access_page(vcpu);
}
 
vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7dd4179..996af6e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5991,6 +5991,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
 
 void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu)
 {
+   struct page *page = NULL;
+
/*
 * Only APIC access page shared by L1 and L2 vm is handled. The APIC
 * access page prepared by L1 for L2's execution is still pinned in
@@ -6003,10 +6005,16 @@ void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu 
*vcpu)
 * migrated, GUP will wait till the migrate entry is replaced
 * with the new pte entry pointing to the new page.
 */
-   vcpu-kvm-arch.apic_access_page = gfn_to_page(vcpu-kvm,
-   APIC_DEFAULT_PHYS_BASE  PAGE_SHIFT);
+   page = gfn_to_page(vcpu-kvm,
+  APIC_DEFAULT_PHYS_BASE  PAGE_SHIFT);
kvm_x86_ops-set_apic_access_page_addr(vcpu-kvm,
-   page_to_phys(vcpu-kvm-arch.apic_access_page));
+  page_to_phys(page));
+
+   /*
+* Do not pin apic access page in memory so that memory hotplug
+* process is able to migrate it.
+*/
+   put_page(page);
}
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_reload_apic_access_page);
@@ -7272,8 +7280,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
kfree(kvm-arch.vpic);
kfree(kvm-arch.vioapic);
kvm_free_vcpus(kvm);
-   if (kvm-arch.apic_access_page)
-   put_page(kvm-arch.apic_access_page);
kfree(rcu_dereference_check(kvm-arch.apic_map, 1));
 }
 
-- 
1.8.3.1

--
To unsubscribe from this