[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 19/38] arm/p2m: Add HVMOP_altp2m_switch_p2m



Hello Sergej,

On 16/08/2016 23:16, Sergej Proskurin wrote:
Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
---
v3: Extended the function "altp2m_switch_domain_altp2m_by_id" so that if
    the guest domain indirectly calles this function, the current vcpu also
    changes the altp2m view without performing an explicit context switch.

    Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
    "altp2m_p2m[idx] == NULL" in "altp2m_switch_domain_altp2m_by_id".
---
 xen/arch/arm/altp2m.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/hvm.c           |  2 +-
 xen/include/asm-arm/altp2m.h |  4 ++++
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index c14ab0b..ba345b9 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -32,6 +32,54 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
     return v->domain->arch.altp2m_p2m[index];
 }

+int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
+{
+    struct vcpu *v;
+    int rc = -EINVAL;
+
+    if ( idx >= MAX_ALTP2M )
+        return rc;
+
+    domain_pause_except_self(d);
+
+    altp2m_lock(d);
+
+    if ( d->arch.altp2m_p2m[idx] != NULL )
+    {
+        for_each_vcpu( d, v )
+            if ( idx != altp2m_vcpu(v).p2midx )

Could you invert the condition to avoid another layer of nested if?

+            {
+                atomic_dec(&altp2m_get_altp2m(v)->active_vcpus);
+                altp2m_vcpu(v).p2midx = idx;
+                atomic_inc(&altp2m_get_altp2m(v)->active_vcpus);
+
+                /*
+                 * In case it is the guest domain, which indirectly called this
+                 * function, the current vcpu will not switch its context
+                 * within the function "p2m_restore_state". That is, changing
+                 * the altp2m_vcpu(v).p2midx will not lead to the requested
+                 * altp2m switch on that specific vcpu. To achieve the desired
+                 * behavior, we write to VTTBR_EL2 directly.
+                 */
+                if ( v->domain == d && v == current )

v == current is enough.

+                {
+                    struct p2m_domain *ap2m = d->arch.altp2m_p2m[idx];
+
+                    WRITE_SYSREG64(ap2m->vttbr, VTTBR_EL2);
+                    isb();

I don't like the open-coding of VTTBR_EL2. I would much prefer a separate helper to update it.

+                }
+            }
+
+        rc = 0;
+    }
+
+    altp2m_unlock(d);
+
+    domain_unpause_except_self(d);
+
+    return rc;
+}
+
 static void altp2m_vcpu_reset(struct vcpu *v)
 {
     struct altp2mvcpu *av = &altp2m_vcpu(v);
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index df973ef..9ac3422 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -132,7 +132,7 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
         break;

     case HVMOP_altp2m_switch_p2m:
-        rc = -EOPNOTSUPP;
+        rc = altp2m_switch_domain_altp2m_by_id(d, a.u.view.view);
         break;

     case HVMOP_altp2m_set_mem_access:
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 6074079..c2e44ab 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -52,6 +52,10 @@ void altp2m_vcpu_destroy(struct vcpu *v);
 /* Get current alternate p2m table. */
 struct p2m_domain *altp2m_get_altp2m(struct vcpu *v);

+/* Switch alternate p2m for entire domain */
+int altp2m_switch_domain_altp2m_by_id(struct domain *d,
+                                      unsigned int idx);
+
 /* Make a specific alternate p2m valid. */
 int altp2m_init_by_id(struct domain *d,
                       unsigned int idx);


Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.