diff mbox

[Altp2m,cleanup,2/3,v9,1/2] Move altp2m specific functions to altp2m files.

Message ID 1476207685-4737-2-git-send-email-paul.c.lai@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Lai Oct. 11, 2016, 5:41 p.m. UTC
This makes the code a little easier to read.
Moving hvm_altp2m_supported() check into functions that use it
for better readability.
Got rid of stray blanks after open paren after function names.
Defining _XEN_ASM_X86_P2M_H instead of _XEN_P2M_H for
xen/include/asm-x86/p2m.h.

Signed-off-by: Paul Lai <paul.c.lai@intel.com>
---
v9
Per request:
o Fixing comments in altp2m_domain_init(struct domain *d)
o Fixing white space (missing blank after NULL in if())
o Reversing the order of
     d->arch.altp2m_eptp = NULL
     p2m_teardown(d->arch.altp2m_p2m[i]);
o coding style within domain_init()
o splitting off the altp2m and ept functions into a seperate patch.
---
 xen/arch/x86/mm/altp2m.c          | 57 +++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/mm/hap/hap.c         | 40 ++++++++-------------------
 xen/include/asm-x86/altp2m.h      |  4 ++-
 xen/include/asm-x86/hvm/vmx/vmx.h |  3 +++
 xen/include/asm-x86/p2m.h         |  9 +++----
 5 files changed, 77 insertions(+), 36 deletions(-)

Comments

Konrad Rzeszutek Wilk Oct. 11, 2016, 7:20 p.m. UTC | #1
On Tue, Oct 11, 2016 at 10:41:24AM -0700, Paul Lai wrote:
> This makes the code a little easier to read.
> Moving hvm_altp2m_supported() check into functions that use it
> for better readability.
> Got rid of stray blanks after open paren after function names.
> Defining _XEN_ASM_X86_P2M_H instead of _XEN_P2M_H for
> xen/include/asm-x86/p2m.h.
> 
> Signed-off-by: Paul Lai <paul.c.lai@intel.com>
> ---
> v9
> Per request:
> o Fixing comments in altp2m_domain_init(struct domain *d)
> o Fixing white space (missing blank after NULL in if())
> o Reversing the order of
>      d->arch.altp2m_eptp = NULL
>      p2m_teardown(d->arch.altp2m_p2m[i]);
> o coding style within domain_init()
> o splitting off the altp2m and ept functions into a seperate patch.
> ---
>  xen/arch/x86/mm/altp2m.c          | 57 +++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/mm/hap/hap.c         | 40 ++++++++-------------------
>  xen/include/asm-x86/altp2m.h      |  4 ++-
>  xen/include/asm-x86/hvm/vmx/vmx.h |  3 +++
>  xen/include/asm-x86/p2m.h         |  9 +++----
>  5 files changed, 77 insertions(+), 36 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
> index 930bdc2..6856a0e 100644
> --- a/xen/arch/x86/mm/altp2m.c
> +++ b/xen/arch/x86/mm/altp2m.c
> @@ -17,6 +17,7 @@
>  
>  #include <asm/hvm/support.h>
>  #include <asm/hvm/hvm.h>
> +#include <asm/domain.h>
>  #include <asm/p2m.h>
>  #include <asm/altp2m.h>
>  
> @@ -66,6 +67,62 @@ altp2m_vcpu_destroy(struct vcpu *v)
>  }
>  
>  /*
> + *  allocate and initialize memory for altp2m portion of domain
> + *
> + *  returns < 0 on error
> + *  returns 0 on no operation & success
> + */
> +int
> +altp2m_domain_init(struct domain *d)
> +{
> +    int rc;
> +    unsigned int i;
> +
> +    if ( d == NULL )
> +        return 0;
> +
> +    if ( !hvm_altp2m_supported() )
> +        return 0;
> +
> +    /* Init alternate p2m data. */
> +    if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL )
> +        return -ENOMEM;
> +
> +    for ( i = 0; i < MAX_EPTP; i++ )
> +        d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
> +
> +    for ( i = 0; i < MAX_ALTP2M; i++ )
> +    {
> +        rc = p2m_alloc_table(d->arch.altp2m_p2m[i]);
> +        if ( rc != 0 ) {

{ should be on its own line.

And you can also do:
	if ( rc )
	{
		...
> +            altp2m_domain_teardown(d);
> +            return rc;
> +        }
> +    }
> +
> +    d->arch.altp2m_active = 0;
> +
> +    return rc;
> +}
> +
> +void
Paul Lai Oct. 11, 2016, 7:26 p.m. UTC | #2
On Tue, Oct 11, 2016 at 03:20:49PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 11, 2016 at 10:41:24AM -0700, Paul Lai wrote:
[snip]
> > +
> > +    for ( i = 0; i < MAX_ALTP2M; i++ )
> > +    {
> > +        rc = p2m_alloc_table(d->arch.altp2m_p2m[i]);
> > +        if ( rc != 0 ) {
> 
> { should be on its own line.
> 
> And you can also do:
> 	if ( rc )
> 	{
> 		...

Thanks... I caught 2-3 like this before sending this out, but... 
Will respin.

-Paul
diff mbox

Patch

diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 930bdc2..6856a0e 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -17,6 +17,7 @@ 
 
 #include <asm/hvm/support.h>
 #include <asm/hvm/hvm.h>
+#include <asm/domain.h>
 #include <asm/p2m.h>
 #include <asm/altp2m.h>
 
@@ -66,6 +67,62 @@  altp2m_vcpu_destroy(struct vcpu *v)
 }
 
 /*
+ *  allocate and initialize memory for altp2m portion of domain
+ *
+ *  returns < 0 on error
+ *  returns 0 on no operation & success
+ */
+int
+altp2m_domain_init(struct domain *d)
+{
+    int rc;
+    unsigned int i;
+
+    if ( d == NULL )
+        return 0;
+
+    if ( !hvm_altp2m_supported() )
+        return 0;
+
+    /* Init alternate p2m data. */
+    if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL )
+        return -ENOMEM;
+
+    for ( i = 0; i < MAX_EPTP; i++ )
+        d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
+
+    for ( i = 0; i < MAX_ALTP2M; i++ )
+    {
+        rc = p2m_alloc_table(d->arch.altp2m_p2m[i]);
+        if ( rc != 0 ) {
+            altp2m_domain_teardown(d);
+            return rc;
+        }
+    }
+
+    d->arch.altp2m_active = 0;
+
+    return rc;
+}
+
+void
+altp2m_domain_teardown(struct domain *d)
+{
+    unsigned int i;
+
+    if ( !hvm_altp2m_supported() )
+        return;
+
+    d->arch.altp2m_active = 0;
+
+    for ( i = 0; i < MAX_ALTP2M; i++ )
+        p2m_teardown(d->arch.altp2m_p2m[i]);
+
+    free_xenheap_page(d->arch.altp2m_eptp);
+    d->arch.altp2m_eptp = NULL;
+}
+
+/*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 3218fa2..4612871 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -37,6 +37,7 @@ 
 #include <asm/hap.h>
 #include <asm/paging.h>
 #include <asm/p2m.h>
+#include <asm/altp2m.h>
 #include <asm/domain.h>
 #include <xen/numa.h>
 #include <asm/hvm/nestedhvm.h>
@@ -493,32 +494,25 @@  int hap_enable(struct domain *d, u32 mode)
             goto out;
     }
 
-    for (i = 0; i < MAX_NESTEDP2M; i++) {
+    for (i = 0; i < MAX_NESTEDP2M; i++)
+    {
         rv = p2m_alloc_table(d->arch.nested_p2m[i]);
         if ( rv != 0 )
            goto out;
     }
 
-    if ( hvm_altp2m_supported() )
+    if ( (rv = altp2m_domain_init(d)) < 0 )
     {
-        /* Init alternate p2m data */
-        if ( (d->arch.altp2m_eptp = alloc_xenheap_page()) == NULL )
+        for (i = 0; i < MAX_NESTEDP2M; i++)
         {
-            rv = -ENOMEM;
-            goto out;
+            p2m_teardown(d->arch.nested_p2m[i]);
         }
 
-        for ( i = 0; i < MAX_EPTP; i++ )
-            d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
-
-        for ( i = 0; i < MAX_ALTP2M; i++ )
-        {
-            rv = p2m_alloc_table(d->arch.altp2m_p2m[i]);
-            if ( rv != 0 )
-               goto out;
-        }
+        if ( d->arch.paging.hap.total_pages != 0 )
+            hap_teardown(d, NULL);
 
-        d->arch.altp2m_active = 0;
+        p2m_teardown(p2m_get_hostp2m(d));
+        goto out;
     }
 
     /* Now let other users see the new mode */
@@ -533,19 +527,7 @@  void hap_final_teardown(struct domain *d)
 {
     unsigned int i;
 
-    if ( hvm_altp2m_supported() )
-    {
-        d->arch.altp2m_active = 0;
-
-        if ( d->arch.altp2m_eptp )
-        {
-            free_xenheap_page(d->arch.altp2m_eptp);
-            d->arch.altp2m_eptp = NULL;
-        }
-
-        for ( i = 0; i < MAX_ALTP2M; i++ )
-            p2m_teardown(d->arch.altp2m_p2m[i]);
-    }
+    altp2m_domain_teardown(d);
 
     /* Destroy nestedp2m's first */
     for (i = 0; i < MAX_NESTEDP2M; i++) {
diff --git a/xen/include/asm-x86/altp2m.h b/xen/include/asm-x86/altp2m.h
index 64c7618..0090c89 100644
--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -18,7 +18,6 @@ 
 #ifndef __ASM_X86_ALTP2M_H
 #define __ASM_X86_ALTP2M_H
 
-#include <xen/types.h>
 #include <xen/sched.h>         /* for struct vcpu, struct domain */
 #include <asm/hvm/vcpu.h>      /* for vcpu_altp2m */
 
@@ -38,4 +37,7 @@  static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
     return vcpu_altp2m(v).p2midx;
 }
 
+int altp2m_domain_init(struct domain *d);
+void altp2m_domain_teardown(struct domain *d);
+
 #endif /* __ASM_X86_ALTP2M_H */
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index 4cdd9b1..9f4c7de 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -561,6 +561,9 @@  void ept_p2m_uninit(struct p2m_domain *p2m);
 void ept_walk_table(struct domain *d, unsigned long gfn);
 bool_t ept_handle_misconfig(uint64_t gpa);
 void setup_ept_dump(void);
+void p2m_init_altp2m_ept(struct domain *d, unsigned int i);
+/* Locate an alternate p2m by its EPTP */
+unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp);
 
 void update_guest_eip(void);
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 7035860..0e72880 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -23,8 +23,8 @@ 
  * along with this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef _XEN_P2M_H
-#define _XEN_P2M_H
+#ifndef _XEN_ASM_X86_P2M_H
+#define _XEN_ASM_X86_P2M_H
 
 #include <xen/config.h>
 #include <xen/paging.h>
@@ -784,9 +784,6 @@  static inline struct p2m_domain *p2m_get_altp2m(struct vcpu *v)
     return v->domain->arch.altp2m_p2m[index];
 }
 
-/* Locate an alternate p2m by its EPTP */
-unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp);
-
 /* Switch alternate p2m for a single vcpu */
 bool_t p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx);
 
@@ -848,7 +845,7 @@  static inline unsigned int p2m_get_iommu_flags(p2m_type_t p2mt)
     return flags;
 }
 
-#endif /* _XEN_P2M_H */
+#endif /* _XEN_ASM_X86_P2M_H */
 
 /*
  * Local variables: