diff mbox

x86/hvm/viridian: fix APIC assist page leak

Message ID 1459248448-13511-1-git-send-email-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Durrant March 29, 2016, 10:47 a.m. UTC 
Commit a6f2cdb6 "keep APIC assist page mapped..." introduced a page
leak because it relied on viridian_vcpu_deinit() always being called
to release the page mapping. This does not happen in the case a normal
domain shutdown.

This patch fixes the problem by introducing a new function,
viridian_domain_deinit(), which will iterate through the vCPUs and
release any page mappings still present.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/hvm.c             |  2 ++
 xen/arch/x86/hvm/viridian.c        | 16 ++++++++++++++++
 xen/include/asm-x86/hvm/viridian.h |  1 +
 3 files changed, 19 insertions(+)

Comments

Jan Beulich March 29, 2016, 11:22 a.m. UTC | #1 
>>> On 29.03.16 at 12:47, <paul.durrant@citrix.com> wrote:
> Commit a6f2cdb6 "keep APIC assist page mapped..." introduced a page
> leak because it relied on viridian_vcpu_deinit() always being called
> to release the page mapping. This does not happen in the case a normal
> domain shutdown.
> 
> This patch fixes the problem by introducing a new function,
> viridian_domain_deinit(), which will iterate through the vCPUs and
> release any page mappings still present.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 80d59ff..611470e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1726,6 +1726,8 @@  void hvm_domain_relinquish_resources(struct domain *d)
     if ( hvm_funcs.nhvm_domain_relinquish_resources )
         hvm_funcs.nhvm_domain_relinquish_resources(d);
 
+    viridian_domain_deinit(d);
+
     hvm_destroy_all_ioreq_servers(d);
 
     msixtbl_pt_cleanup(d);
diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index dceed2c..5c76c1a 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -251,6 +251,14 @@  static void initialize_apic_assist(struct vcpu *v)
 
     if ( viridian_feature_mask(v->domain) & HVMPV_apic_assist )
     {
+        /*
+         * If we overwrite an existing address here then something has
+         * gone wrong and a domain page will leak. Instead crash the
+         * domain to make the problem obvious.
+         */
+        if ( v->arch.hvm_vcpu.viridian.apic_assist.va )
+            domain_crash(d);
+
         v->arch.hvm_vcpu.viridian.apic_assist.va = va;
         return;
     }
@@ -608,6 +616,14 @@  void viridian_vcpu_deinit(struct vcpu *v)
     teardown_apic_assist(v);
 }
 
+void viridian_domain_deinit(struct domain *d)
+{
+    struct vcpu *v;
+
+    for_each_vcpu ( d, v )
+        teardown_apic_assist(v);
+}
+
 static DEFINE_PER_CPU(cpumask_t, ipi_cpumask);
 
 int viridian_hypercall(struct cpu_user_regs *regs)
diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h
index 7f281b2..bdbccd5 100644
--- a/xen/include/asm-x86/hvm/viridian.h
+++ b/xen/include/asm-x86/hvm/viridian.h
@@ -122,6 +122,7 @@  void viridian_time_ref_count_freeze(struct domain *d);
 void viridian_time_ref_count_thaw(struct domain *d);
 
 void viridian_vcpu_deinit(struct vcpu *v);
+void viridian_domain_deinit(struct domain *d);
 
 void viridian_start_apic_assist(struct vcpu *v, int vector);
 int viridian_complete_apic_assist(struct vcpu *v);