Re: [Xen-devel] [PATCH v3 16/38] arm/p2m: Add HVMOP_altp2m_set_domain_state

2016-09-14 Thread Julien Grall 

Hello Sergej,

On 16/08/16 23:16, Sergej Proskurin wrote:

diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 180154e..c69da36 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -83,8 +83,40 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
 break;

 case HVMOP_altp2m_set_domain_state:
-rc = -EOPNOTSUPP;
+{
+struct vcpu *v;
+bool_t ostate, nstate;
+
+ostate = d->arch.altp2m_active;
+nstate = !!a.u.domain_state.state;
+
+/* If the alternate p2m state has changed, handle appropriately */
+if ( (nstate != ostate) &&
+ (ostate || !(rc = altp2m_init_by_id(d, 0))) )
+{
+for_each_vcpu( d, v )
+{
+if ( !ostate )
+{
+altp2m_vcpu_initialise(v);
+d->arch.altp2m_active = nstate;


Why do you set d->arch.altp2m_active for each vCPU?


+}
+else
+{
+d->arch.altp2m_active = nstate;
+altp2m_vcpu_destroy(v);
+}
+}
+
+/*
+ * The altp2m_active state has been deactivated. It is now safe to
+ * flush all altp2m views -- including altp2m[0].
+ */
+if ( ostate )
+altp2m_flush(d);
+}
 break;
+}

 case HVMOP_altp2m_vcpu_enable_notify:
 rc = -EOPNOTSUPP;


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 16/38] arm/p2m: Add HVMOP_altp2m_set_domain_state

2016-09-13 Thread Sergej Proskurin 
Hi Julien,


On 09/09/2016 07:14 PM, Julien Grall wrote:
> Hello Sergej,
>
> On 16/08/16 23:16, Sergej Proskurin wrote:
>> +static int altp2m_init_helper(struct domain *d, unsigned int idx)
>> +{
>> +int rc;
>> +struct p2m_domain *p2m = d->arch.altp2m_p2m[idx];
>> +
>> +ASSERT(p2m == NULL);
>> +
>> +/* Allocate a new, zeroed altp2m view. */
>> +p2m = xzalloc(struct p2m_domain);
>> +if ( p2m == NULL)
>> +{
>> +rc = -ENOMEM;
>> +goto err;
>> +}
>
> This could be simplified with just return -ENOMEM.
>

True. I will do that.

>> +
>> +p2m->p2m_class = p2m_alternate;
>> +
>> +/* Initialize the new altp2m view. */
>> +rc = p2m_init_one(d, p2m);
>> +if ( rc )
>> +goto err;
>> +
>> +p2m->access_required = false;
>> +_atomic_set(>active_vcpus, 0);
>
> the p2m is initialized to 0, so both access_required and active_vcpus
> initialization is not necessary.
>

I will remove these initializations in the next patch.

>> +
>> +d->arch.altp2m_p2m[idx] = p2m;
>> +
>> +return rc;
>> +
>> +err:
>> +if ( p2m )
>> +xfree(p2m);
>
> xfree is able to handle NULL pointer.
>

Ok, thank you.

>> +
>> +d->arch.altp2m_p2m[idx] = NULL;
>> +
>> +return rc;
>> +}
>> +
>> +int altp2m_init_by_id(struct domain *d, unsigned int idx)
>> +{
>> +int rc = -EINVAL;
>> +
>> +if ( idx >= MAX_ALTP2M )
>> +return rc;
>> +
>> +altp2m_lock(d);
>> +
>> +if ( d->arch.altp2m_p2m[idx] == NULL )
>> +rc = altp2m_init_helper(d, idx);
>> +
>> +altp2m_unlock(d);
>> +
>> +return rc;
>> +}
>> +

Cheers,
~Sergej

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

Re: [Xen-devel] [PATCH v3 16/38] arm/p2m: Add HVMOP_altp2m_set_domain_state

2016-09-09 Thread Julien Grall 

Hello Sergej,

On 16/08/16 23:16, Sergej Proskurin wrote:

+static int altp2m_init_helper(struct domain *d, unsigned int idx)
+{
+int rc;
+struct p2m_domain *p2m = d->arch.altp2m_p2m[idx];
+
+ASSERT(p2m == NULL);
+
+/* Allocate a new, zeroed altp2m view. */
+p2m = xzalloc(struct p2m_domain);
+if ( p2m == NULL)
+{
+rc = -ENOMEM;
+goto err;
+}


This could be simplified with just return -ENOMEM.


+
+p2m->p2m_class = p2m_alternate;
+
+/* Initialize the new altp2m view. */
+rc = p2m_init_one(d, p2m);
+if ( rc )
+goto err;
+
+p2m->access_required = false;
+_atomic_set(>active_vcpus, 0);


the p2m is initialized to 0, so both access_required and active_vcpus 
initialization is not necessary.



+
+d->arch.altp2m_p2m[idx] = p2m;
+
+return rc;
+
+err:
+if ( p2m )
+xfree(p2m);


xfree is able to handle NULL pointer.


+
+d->arch.altp2m_p2m[idx] = NULL;
+
+return rc;
+}
+
+int altp2m_init_by_id(struct domain *d, unsigned int idx)
+{
+int rc = -EINVAL;
+
+if ( idx >= MAX_ALTP2M )
+return rc;
+
+altp2m_lock(d);
+
+if ( d->arch.altp2m_p2m[idx] == NULL )
+rc = altp2m_init_helper(d, idx);
+
+altp2m_unlock(d);
+
+return rc;
+}
+


Regards,

--
Julien Grall

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

[Xen-devel] [PATCH v3 16/38] arm/p2m: Add HVMOP_altp2m_set_domain_state

2016-08-16 Thread Sergej Proskurin 
The HVMOP_altp2m_set_domain_state allows to activate altp2m on a
specific domain. This commit adopts the x86
HVMOP_altp2m_set_domain_state implementation.

Signed-off-by: Sergej Proskurin 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
v2: Dynamically allocate memory for altp2m views only when needed.
Move altp2m related helpers to altp2m.c.
p2m_flush_tlb is made publicly accessible.

v3: Cosmetic fixes.

Removed call to "p2m_alloc_table" in "altp2m_init_helper" as the
entire p2m allocation is now done within the function
"p2m_init_one". The same applies to the call of the function
"p2m_flush_tlb" from "p2m_init_one".

Removed the "altp2m_enabled" check in HVMOP_altp2m_set_domain_state
case as it has been moved in front of the switch statement in
"do_altp2m_op".

Changed the order of setting the new altp2m state (depending on
setting/resetting the state) in HVMOP_altp2m_set_domain_state case.

Removed the call to altp2m_vcpu_reset from altp2m_vcpu_initialise,
as the p2midx is set right after the call to 0, representing the
default view.

Moved the define "vcpu_altp2m" from domain.h to altp2m.h to avoid
defining altp2m-related functionality in multiple files. Also renamed
"vcpu_altp2m" to "altp2m_vcpu".

Declared the function "p2m_flush_tlb" as static, as it is not called
from altp2m.h anymore.

Exported the function "altp2m_get_altp2m" in altp2m.h.

Exchanged the check "altp2m_vttbr[idx] == INVALID_VTTBR" for
"altp2m_p2m[idx] == NULL" in "altp2m_init_by_id".

Set the field p2m->access_required to false by default.
---
 xen/arch/arm/altp2m.c| 102 +++
 xen/arch/arm/hvm.c   |  34 ++-
 xen/include/asm-arm/altp2m.h |  14 ++
 xen/include/asm-arm/domain.h |   7 +++
 xen/include/asm-arm/p2m.h|   5 +++
 5 files changed, 161 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index 02cffd7..02a52ec 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -20,6 +20,108 @@
 #include 
 #include 
 
+struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
+{
+unsigned int index = altp2m_vcpu(v).p2midx;
+
+if ( index == INVALID_ALTP2M )
+return NULL;
+
+BUG_ON(index >= MAX_ALTP2M);
+
+return v->domain->arch.altp2m_p2m[index];
+}
+
+static void altp2m_vcpu_reset(struct vcpu *v)
+{
+struct altp2mvcpu *av = _vcpu(v);
+
+av->p2midx = INVALID_ALTP2M;
+}
+
+void altp2m_vcpu_initialise(struct vcpu *v)
+{
+if ( v != current )
+vcpu_pause(v);
+
+altp2m_vcpu(v).p2midx = 0;
+atomic_inc(_get_altp2m(v)->active_vcpus);
+
+if ( v != current )
+vcpu_unpause(v);
+}
+
+void altp2m_vcpu_destroy(struct vcpu *v)
+{
+struct p2m_domain *p2m;
+
+if ( v != current )
+vcpu_pause(v);
+
+if ( (p2m = altp2m_get_altp2m(v)) )
+atomic_dec(>active_vcpus);
+
+altp2m_vcpu_reset(v);
+
+if ( v != current )
+vcpu_unpause(v);
+}
+
+static int altp2m_init_helper(struct domain *d, unsigned int idx)
+{
+int rc;
+struct p2m_domain *p2m = d->arch.altp2m_p2m[idx];
+
+ASSERT(p2m == NULL);
+
+/* Allocate a new, zeroed altp2m view. */
+p2m = xzalloc(struct p2m_domain);
+if ( p2m == NULL)
+{
+rc = -ENOMEM;
+goto err;
+}
+
+p2m->p2m_class = p2m_alternate;
+
+/* Initialize the new altp2m view. */
+rc = p2m_init_one(d, p2m);
+if ( rc )
+goto err;
+
+p2m->access_required = false;
+_atomic_set(>active_vcpus, 0);
+
+d->arch.altp2m_p2m[idx] = p2m;
+
+return rc;
+
+err:
+if ( p2m )
+xfree(p2m);
+
+d->arch.altp2m_p2m[idx] = NULL;
+
+return rc;
+}
+
+int altp2m_init_by_id(struct domain *d, unsigned int idx)
+{
+int rc = -EINVAL;
+
+if ( idx >= MAX_ALTP2M )
+return rc;
+
+altp2m_lock(d);
+
+if ( d->arch.altp2m_p2m[idx] == NULL )
+rc = altp2m_init_helper(d, idx);
+
+altp2m_unlock(d);
+
+return rc;
+}
+
 int altp2m_init(struct domain *d)
 {
 unsigned int i;
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 180154e..c69da36 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -83,8 +83,40 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
 break;
 
 case HVMOP_altp2m_set_domain_state:
-rc = -EOPNOTSUPP;
+{
+struct vcpu *v;
+bool_t ostate, nstate;
+
+ostate = d->arch.altp2m_active;
+nstate = !!a.u.domain_state.state;
+
+/* If the alternate p2m state has changed, handle appropriately */
+if ( (nstate != ostate) &&
+ (ostate || !(rc = altp2m_init_by_id(d, 0))) )
+{
+for_each_vcpu( d, v )
+{
+if ( !ostate )
+{
+altp2m_vcpu_initialise(v);
+