Re: [Xen-devel] [PATCH v3 31/38] altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

2016-08-18 Thread George Dunlap 
On 16/08/16 23:17, Sergej Proskurin wrote:
> This commit adds the function "altp2m_switch_vcpu_altp2m_by_id" that is
> executed after checking whether the vcpu should be switched to a
> different altp2m within the function "altp2m_check".
> 
> Please note that in this commit, the function "p2m_altp2m_check" is
> renamed to "altp2m_check" and moved from p2m.c to altp2m.c for the x86
> architecuture. This change was perfomed in order to move altp2m related
> functions to one spot (which is altp2m.c). The reason for modifying the
> function's name is due the association of the function with the
> associated .c file.
> 
> Signed-off-by: Sergej Proskurin 

x86/p2m bits:

Acked-by: George Dunlap 

> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Razvan Cojocaru 
> Cc: Tamas K Lengyel 
> ---
> v3: This commit has been moved out of the commit "arm/p2m: Add altp2m
> paging mechanism".
> 
> Moved the function "p2m_altp2m_check" from p2m.c to altp2m.c and
> renamed it to "altp2m_check". This change required the adoption of
> the complementary function in the x86 architecture.
> ---
>  xen/arch/arm/altp2m.c| 32 
>  xen/arch/x86/mm/altp2m.c |  6 ++
>  xen/arch/x86/mm/p2m.c|  6 --
>  xen/common/vm_event.c|  3 ++-
>  xen/include/asm-arm/altp2m.h |  7 ---
>  xen/include/asm-arm/p2m.h|  6 --
>  xen/include/asm-x86/altp2m.h |  3 +++
>  xen/include/asm-x86/p2m.h|  3 ---
>  8 files changed, 47 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
> index b10711e..11272e9 100644
> --- a/xen/arch/arm/altp2m.c
> +++ b/xen/arch/arm/altp2m.c
> @@ -32,6 +32,38 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
>  return v->domain->arch.altp2m_p2m[index];
>  }
>  
> +static bool_t altp2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int 
> idx)
> +{
> +struct domain *d = v->domain;
> +bool_t rc = false;
> +
> +if ( idx >= MAX_ALTP2M )
> +return rc;
> +
> +altp2m_lock(d);
> +
> +if ( d->arch.altp2m_p2m[idx] != NULL )
> +{
> +if ( idx != altp2m_vcpu(v).p2midx )
> +{
> +atomic_dec(_get_altp2m(v)->active_vcpus);
> +altp2m_vcpu(v).p2midx = idx;
> +atomic_inc(_get_altp2m(v)->active_vcpus);
> +}
> +rc = true;
> +}
> +
> +altp2m_unlock(d);
> +
> +return rc;
> +}
> +
> +void altp2m_check(struct vcpu *v, uint16_t idx)
> +{
> +if ( altp2m_active(v->domain) )
> +altp2m_switch_vcpu_altp2m_by_id(v, idx);
> +}
> +
>  int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
>  {
>  struct vcpu *v;
> diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
> index 930bdc2..00abb5a 100644
> --- a/xen/arch/x86/mm/altp2m.c
> +++ b/xen/arch/x86/mm/altp2m.c
> @@ -65,6 +65,12 @@ altp2m_vcpu_destroy(struct vcpu *v)
>  vcpu_unpause(v);
>  }
>  
> +void altp2m_check(struct vcpu *v, uint16_t idx)
> +{
> +if ( altp2m_active(v->domain) )
> +p2m_switch_vcpu_altp2m_by_id(v, idx);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 812dbf6..cb28cc2 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1646,12 +1646,6 @@ void p2m_mem_access_emulate_check(struct vcpu *v,
>  }
>  }
>  
> -void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> -{
> -if ( altp2m_active(v->domain) )
> -p2m_switch_vcpu_altp2m_by_id(v, idx);
> -}
> -
>  bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>  struct npfec npfec,
>  vm_event_request_t **req_ptr)
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index 8398af7..e48d111 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /* for public/io/ring.h macros */
>  #define xen_mb()   mb()
> @@ -423,7 +424,7 @@ void vm_event_resume(struct domain *d, struct 
> vm_event_domain *ved)
>  
>  /* Check for altp2m switch */
>  if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
> -p2m_altp2m_check(v, rsp.altp2m_idx);
> +altp2m_check(v, rsp.altp2m_idx);
>  
>  /* Check flags which apply only when the vCPU is paused */
>  if ( atomic_read(>vm_event_pause_count) )
> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
> index 7f385d9..ef80829 100644
> --- a/xen/include/asm-arm/altp2m.h
> +++ b/xen/include/asm-arm/altp2m.h
> @@ -38,9 +38,7 @@ static inline bool_t altp2m_active(const struct domain *d)
>

Re: [Xen-devel] [PATCH v3 31/38] altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

2016-08-17 Thread Julien Grall 



On 17/08/16 13:37, Sergej Proskurin wrote:

On 08/17/2016 12:05 PM, Jan Beulich wrote:

On 17.08.16 at 00:17,  wrote:

 xen/arch/arm/altp2m.c| 32 
 xen/arch/x86/mm/altp2m.c |  6 ++
 xen/arch/x86/mm/p2m.c|  6 --
 xen/common/vm_event.c|  3 ++-
 xen/include/asm-arm/altp2m.h |  7 ---
 xen/include/asm-arm/p2m.h|  6 --
 xen/include/asm-x86/altp2m.h |  3 +++
 xen/include/asm-x86/p2m.h|  3 ---
 8 files changed, 47 insertions(+), 19 deletions(-)

The x86 parts of the change look really independent of the rest, so
you could have done yourself a favor by splitting this out, as then
the individual patches would each require separate acks rather than
the one patch here requiring an x86/mm one along with the ARM and
VM-event ones.

Jan



Ok, I will split the patch accordingly. Thank you.


I think it is worth to mention again that patches doing one logical 
thing (i.e moving code, renaming function) are easier to review compare 
to patch doing multiple one.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 31/38] altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

2016-08-17 Thread Sergej Proskurin 
Hi Jan,


On 08/17/2016 12:05 PM, Jan Beulich wrote:
 On 17.08.16 at 00:17,  wrote:
>>  xen/arch/arm/altp2m.c| 32 
>>  xen/arch/x86/mm/altp2m.c |  6 ++
>>  xen/arch/x86/mm/p2m.c|  6 --
>>  xen/common/vm_event.c|  3 ++-
>>  xen/include/asm-arm/altp2m.h |  7 ---
>>  xen/include/asm-arm/p2m.h|  6 --
>>  xen/include/asm-x86/altp2m.h |  3 +++
>>  xen/include/asm-x86/p2m.h|  3 ---
>>  8 files changed, 47 insertions(+), 19 deletions(-)
> The x86 parts of the change look really independent of the rest, so
> you could have done yourself a favor by splitting this out, as then
> the individual patches would each require separate acks rather than
> the one patch here requiring an x86/mm one along with the ARM and
> VM-event ones.
>
> Jan
>

Ok, I will split the patch accordingly. Thank you.

Best regards,
~Sergej

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

Re: [Xen-devel] [PATCH v3 31/38] altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

2016-08-17 Thread Razvan Cojocaru 
On 08/17/2016 01:17 AM, Sergej Proskurin wrote:
> This commit adds the function "altp2m_switch_vcpu_altp2m_by_id" that is
> executed after checking whether the vcpu should be switched to a
> different altp2m within the function "altp2m_check".
> 
> Please note that in this commit, the function "p2m_altp2m_check" is
> renamed to "altp2m_check" and moved from p2m.c to altp2m.c for the x86
> architecuture. This change was perfomed in order to move altp2m related
> functions to one spot (which is altp2m.c). The reason for modifying the
> function's name is due the association of the function with the
> associated .c file.
> 
> Signed-off-by: Sergej Proskurin 
> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Razvan Cojocaru 
> Cc: Tamas K Lengyel 
> ---
> v3: This commit has been moved out of the commit "arm/p2m: Add altp2m
> paging mechanism".
> 
> Moved the function "p2m_altp2m_check" from p2m.c to altp2m.c and
> renamed it to "altp2m_check". This change required the adoption of
> the complementary function in the x86 architecture.
> ---
>  xen/arch/arm/altp2m.c| 32 
>  xen/arch/x86/mm/altp2m.c |  6 ++
>  xen/arch/x86/mm/p2m.c|  6 --
>  xen/common/vm_event.c|  3 ++-
>  xen/include/asm-arm/altp2m.h |  7 ---
>  xen/include/asm-arm/p2m.h|  6 --
>  xen/include/asm-x86/altp2m.h |  3 +++
>  xen/include/asm-x86/p2m.h|  3 ---
>  8 files changed, 47 insertions(+), 19 deletions(-)

For the vm_event bits (there are only mechanical changes there):

Acked-by: Razvan Cojocaru 


Thanks,
Razvan


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

Re: [Xen-devel] [PATCH v3 31/38] altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

2016-08-17 Thread Jan Beulich 
>>> On 17.08.16 at 00:17,  wrote:
>  xen/arch/arm/altp2m.c| 32 
>  xen/arch/x86/mm/altp2m.c |  6 ++
>  xen/arch/x86/mm/p2m.c|  6 --
>  xen/common/vm_event.c|  3 ++-
>  xen/include/asm-arm/altp2m.h |  7 ---
>  xen/include/asm-arm/p2m.h|  6 --
>  xen/include/asm-x86/altp2m.h |  3 +++
>  xen/include/asm-x86/p2m.h|  3 ---
>  8 files changed, 47 insertions(+), 19 deletions(-)

The x86 parts of the change look really independent of the rest, so
you could have done yourself a favor by splitting this out, as then
the individual patches would each require separate acks rather than
the one patch here requiring an x86/mm one along with the ARM and
VM-event ones.

Jan


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

[Xen-devel] [PATCH v3 31/38] altp2m: Introduce altp2m_switch_vcpu_altp2m_by_id

2016-08-16 Thread Sergej Proskurin 
This commit adds the function "altp2m_switch_vcpu_altp2m_by_id" that is
executed after checking whether the vcpu should be switched to a
different altp2m within the function "altp2m_check".

Please note that in this commit, the function "p2m_altp2m_check" is
renamed to "altp2m_check" and moved from p2m.c to altp2m.c for the x86
architecuture. This change was perfomed in order to move altp2m related
functions to one spot (which is altp2m.c). The reason for modifying the
function's name is due the association of the function with the
associated .c file.

Signed-off-by: Sergej Proskurin 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Razvan Cojocaru 
Cc: Tamas K Lengyel 
---
v3: This commit has been moved out of the commit "arm/p2m: Add altp2m
paging mechanism".

Moved the function "p2m_altp2m_check" from p2m.c to altp2m.c and
renamed it to "altp2m_check". This change required the adoption of
the complementary function in the x86 architecture.
---
 xen/arch/arm/altp2m.c| 32 
 xen/arch/x86/mm/altp2m.c |  6 ++
 xen/arch/x86/mm/p2m.c|  6 --
 xen/common/vm_event.c|  3 ++-
 xen/include/asm-arm/altp2m.h |  7 ---
 xen/include/asm-arm/p2m.h|  6 --
 xen/include/asm-x86/altp2m.h |  3 +++
 xen/include/asm-x86/p2m.h|  3 ---
 8 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c
index b10711e..11272e9 100644
--- a/xen/arch/arm/altp2m.c
+++ b/xen/arch/arm/altp2m.c
@@ -32,6 +32,38 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v)
 return v->domain->arch.altp2m_p2m[index];
 }
 
+static bool_t altp2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx)
+{
+struct domain *d = v->domain;
+bool_t rc = false;
+
+if ( idx >= MAX_ALTP2M )
+return rc;
+
+altp2m_lock(d);
+
+if ( d->arch.altp2m_p2m[idx] != NULL )
+{
+if ( idx != altp2m_vcpu(v).p2midx )
+{
+atomic_dec(_get_altp2m(v)->active_vcpus);
+altp2m_vcpu(v).p2midx = idx;
+atomic_inc(_get_altp2m(v)->active_vcpus);
+}
+rc = true;
+}
+
+altp2m_unlock(d);
+
+return rc;
+}
+
+void altp2m_check(struct vcpu *v, uint16_t idx)
+{
+if ( altp2m_active(v->domain) )
+altp2m_switch_vcpu_altp2m_by_id(v, idx);
+}
+
 int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx)
 {
 struct vcpu *v;
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 930bdc2..00abb5a 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -65,6 +65,12 @@ altp2m_vcpu_destroy(struct vcpu *v)
 vcpu_unpause(v);
 }
 
+void altp2m_check(struct vcpu *v, uint16_t idx)
+{
+if ( altp2m_active(v->domain) )
+p2m_switch_vcpu_altp2m_by_id(v, idx);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 812dbf6..cb28cc2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1646,12 +1646,6 @@ void p2m_mem_access_emulate_check(struct vcpu *v,
 }
 }
 
-void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
-{
-if ( altp2m_active(v->domain) )
-p2m_switch_vcpu_altp2m_by_id(v, idx);
-}
-
 bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
 struct npfec npfec,
 vm_event_request_t **req_ptr)
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 8398af7..e48d111 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* for public/io/ring.h macros */
 #define xen_mb()   mb()
@@ -423,7 +424,7 @@ void vm_event_resume(struct domain *d, struct 
vm_event_domain *ved)
 
 /* Check for altp2m switch */
 if ( rsp.flags & VM_EVENT_FLAG_ALTERNATE_P2M )
-p2m_altp2m_check(v, rsp.altp2m_idx);
+altp2m_check(v, rsp.altp2m_idx);
 
 /* Check flags which apply only when the vCPU is paused */
 if ( atomic_read(>vm_event_pause_count) )
diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h
index 7f385d9..ef80829 100644
--- a/xen/include/asm-arm/altp2m.h
+++ b/xen/include/asm-arm/altp2m.h
@@ -38,9 +38,7 @@ static inline bool_t altp2m_active(const struct domain *d)
 /* Alternate p2m VCPU */
 static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
 {
-/* Not implemented on ARM, should not be reached. */
-BUG();
-return 0;
+return altp2m_vcpu(v).p2midx;
 }
 
 int altp2m_init(struct domain *d);
@@ -52,6 +50,9 @@ void altp2m_vcpu_destroy(struct vcpu *v);
 /* Get current alternate p2m table. */
 struct p2m_domain *altp2m_get_altp2m(struct