xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Kevin Tian" <kevin.tian@intel.com>,
	"Jun Nakajima" <jun.nakajima@intel.com>,
	"Julien Grall" <julien@xen.org>
Subject: [PATCH v3 1/2][4.15] VMX: delay p2m insertion of APIC access page
Date: Mon, 22 Feb 2021 11:56:58 +0100	[thread overview]
Message-ID: <90271e69-c07e-a32c-5531-a79b10ef03dd@suse.com> (raw)
In-Reply-To: <4731a3a3-906a-98ac-11ba-6a0723903391@suse.com>

Inserting the mapping at domain creation time leads to a memory leak
when the creation fails later on and the domain uses separate CPU and
IOMMU page tables - the latter requires intermediate page tables to be
allocated, but there's no freeing of them at present in this case. Since
we don't need the p2m insertion to happen this early, avoid the problem
altogether by deferring it until the last possible point. This comes at
the price of not being able to handle an error other than by crashing
the domain.

Reported-by: Julien Grall <julien@xen.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v3: New (split out).
---
Hooking p2m insertion onto arch_domain_creation_finished() isn't very
nice, but I couldn't find any better hook (nor a good place where to
introduce a new one). In particular there look to be no hvm_funcs hooks
being used on a domain-wide basis (except for init/destroy of course).
I did consider connecting this to the setting of HVM_PARAM_IDENT_PT, but
considered this no better, the more that the tool stack could be smarter
and avoid setting that param when not needed.

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1007,6 +1007,8 @@ int arch_domain_soft_reset(struct domain
 
 void arch_domain_creation_finished(struct domain *d)
 {
+    if ( is_hvm_domain(d) )
+        hvm_domain_creation_finished(d);
 }
 
 #define xen_vcpu_guest_context vcpu_guest_context
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -428,6 +428,14 @@ static void vmx_domain_relinquish_resour
     vmx_free_vlapic_mapping(d);
 }
 
+static void domain_creation_finished(struct domain *d)
+{
+    if ( has_vlapic(d) && !mfn_eq(d->arch.hvm.vmx.apic_access_mfn, _mfn(0)) &&
+         set_mmio_p2m_entry(d, gaddr_to_gfn(APIC_DEFAULT_PHYS_BASE),
+                            d->arch.hvm.vmx.apic_access_mfn, PAGE_ORDER_4K) )
+        domain_crash(d);
+}
+
 static void vmx_init_ipt(struct vcpu *v)
 {
     unsigned int size = v->domain->vmtrace_size;
@@ -2408,6 +2416,7 @@ static struct hvm_function_table __initd
     .cpu_dead             = vmx_cpu_dead,
     .domain_initialise    = vmx_domain_initialise,
     .domain_relinquish_resources = vmx_domain_relinquish_resources,
+    .domain_creation_finished = domain_creation_finished,
     .vcpu_initialise      = vmx_vcpu_initialise,
     .vcpu_destroy         = vmx_vcpu_destroy,
     .save_cpu_ctxt        = vmx_save_vmcs_ctxt,
@@ -3234,8 +3243,7 @@ static int vmx_alloc_vlapic_mapping(stru
     clear_domain_page(mfn);
     d->arch.hvm.vmx.apic_access_mfn = mfn;
 
-    return set_mmio_p2m_entry(d, gaddr_to_gfn(APIC_DEFAULT_PHYS_BASE), mfn,
-                              PAGE_ORDER_4K);
+    return 0;
 }
 
 static void vmx_free_vlapic_mapping(struct domain *d)
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -106,6 +106,7 @@ struct hvm_function_table {
      * Initialise/destroy HVM domain/vcpu resources
      */
     int  (*domain_initialise)(struct domain *d);
+    void (*domain_creation_finished)(struct domain *d);
     void (*domain_relinquish_resources)(struct domain *d);
     void (*domain_destroy)(struct domain *d);
     int  (*vcpu_initialise)(struct vcpu *v);
@@ -390,6 +391,12 @@ static inline bool hvm_has_set_descripto
     return hvm_funcs.set_descriptor_access_exiting;
 }
 
+static inline void hvm_domain_creation_finished(struct domain *d)
+{
+    if ( hvm_funcs.domain_creation_finished )
+        alternative_vcall(hvm_funcs.domain_creation_finished, d);
+}
+
 static inline int
 hvm_guest_x86_mode(struct vcpu *v)
 {
@@ -765,6 +772,11 @@ static inline void hvm_invlpg(const stru
 {
     ASSERT_UNREACHABLE();
 }
+
+static inline void hvm_domain_creation_finished(struct domain *d)
+{
+    ASSERT_UNREACHABLE();
+}
 
 /*
  * Shadow code needs further cleanup to eliminate some HVM-only paths. For



  reply	other threads:[~2021-02-22 10:57 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-22 10:55 [PATCH v3 0/2] VMX: apic access page handling adjustments Jan Beulich
2021-02-22 10:56 ` Jan Beulich [this message]
2021-02-22 11:25   ` [PATCH v3 1/2][4.15] VMX: delay p2m insertion of APIC access page Ian Jackson
2021-02-22 14:05     ` Jan Beulich
2021-02-22 17:17       ` Ian Jackson
2021-02-22 12:15   ` Roger Pau Monné
2021-02-25  8:44   ` Jan Beulich
2021-02-26  7:06     ` Tian, Kevin
2021-02-22 10:57 ` [PATCH v3 2/2] VMX: use a single, global " Jan Beulich
2021-03-01  2:34   ` Tian, Kevin
2021-03-01  8:18     ` Jan Beulich
2021-04-12 10:40 ` [PATCH v4] " Jan Beulich
2021-04-12 15:31   ` Roger Pau Monné
2021-04-13  9:24     ` Jan Beulich
2021-04-13 10:18       ` Roger Pau Monné
2021-04-13 12:03         ` Jan Beulich
2021-04-13 13:03           ` Roger Pau Monné
2021-04-17 19:24   ` Tim Deegan
2021-04-19 11:25     ` Jan Beulich
2021-04-22  7:42       ` Tim Deegan
2021-04-22  9:38         ` Jan Beulich
2021-04-22 15:05           ` Tim Deegan
2021-04-23 10:51 ` [PATCH v4 0/3] VMX APIC access page and shadow adjustments Jan Beulich
2021-04-23 10:52   ` [PATCH v4 1/3] VMX: use a single, global APIC access page Jan Beulich
2021-04-23 14:17     ` Roger Pau Monné
2021-04-23 14:42       ` Jan Beulich
2021-04-26 17:55         ` Tim Deegan
2021-04-25  1:27     ` Tian, Kevin
2021-04-26 17:53     ` Tim Deegan
2021-04-23 10:53   ` [PATCH v4 2/3] x86/shadow: re-use variables in shadow_get_page_from_l1e() Jan Beulich
2021-04-23 10:54   ` [PATCH v4 3/3] x86/shadow: streamline shadow_get_page_from_l1e() Jan Beulich
2021-04-23 11:00   ` Really v5 (was: [PATCH v4 0/3] VMX APIC access page and shadow adjustments) Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=90271e69-c07e-a32c-5531-a79b10ef03dd@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=julien@xen.org \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH v3 1/2][4.15] VMX: delay p2m insertion of APIC access page' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).