Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

2017-08-16 Thread Sergej Proskurin 


On 08/16/2017 12:11 PM, Julien Grall wrote:
>
>
> On 16/08/17 10:58, Sergej Proskurin wrote:
>> Hi Julien,
>>
>>
>> On 08/09/2017 10:20 AM, Sergej Proskurin wrote:
>>> This commit moves the function vgic_access_guest_memory to guestcopy.c
>>> and the header asm/guest_access.h. No functional changes are made.
>>> Please note that the function will be renamed in the following commit.
>>>
>>> Signed-off-by: Sergej Proskurin 
>>> Acked-by: Julien Grall 
>>> ---
>>> Cc: Stefano Stabellini 
>>> Cc: Julien Grall 
>>> ---
>>> v6: We added this patch to our patch series.
>>>
>>> v7: Add Acked-by Julien Grall.
>>
>> [...]
>>
>>> diff --git a/xen/include/asm-arm/guest_access.h
>>> b/xen/include/asm-arm/guest_access.h
>>> index 251e935597..49716501a4 100644
>>> --- a/xen/include/asm-arm/guest_access.h
>>> +++ b/xen/include/asm-arm/guest_access.h
>>> @@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void
>>> *to, const void *from,
>>>  unsigned long raw_copy_from_guest(void *to, const void *from,
>>> unsigned len);
>>>  unsigned long raw_clear_guest(void *to, unsigned len);
>>>
>>> +int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
>>> + uint32_t size, bool_t is_write);
>>> +
>>>  #define __raw_copy_to_guest raw_copy_to_guest
>>>  #define __raw_copy_from_guest raw_copy_from_guest
>>>  #define __raw_clear_guest raw_clear_guest
>>> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
>>> index d4ed23df28..e489d0bf21 100644
>>> --- a/xen/include/asm-arm/vgic.h
>>> +++ b/xen/include/asm-arm/vgic.h
>>> @@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d,
>>> const struct vgic_ops *ops);
>>>  int vgic_v2_init(struct domain *d, int *mmio_count);
>>>  int vgic_v3_init(struct domain *d, int *mmio_count);
>>>
>>> -int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
>>> - uint32_t size, bool_t is_write);
>>> -
>>>  extern int domain_vgic_register(struct domain *d, int *mmio_count);
>>>  extern int vcpu_vgic_free(struct vcpu *v);
>>>  extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
>>
>> As Stefano and Andrew mentioned in patch 11/13, due to a recent patch in
>> staging, the upper patch failes building due to a missing declaration of
>> struct domain in . This can be easily fixed by adding a
>> forward declaration to struct domain right above
>> vgic_access_guest_memory in  as you will find in the
>> following patch.
>
> Why the forward declaration and not directly including xen/sched.h?

Yeap, that works too :)

Thanks,
~Sergej

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

Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

2017-08-16 Thread Julien Grall 



On 16/08/17 10:58, Sergej Proskurin wrote:

Hi Julien,


On 08/09/2017 10:20 AM, Sergej Proskurin wrote:

This commit moves the function vgic_access_guest_memory to guestcopy.c
and the header asm/guest_access.h. No functional changes are made.
Please note that the function will be renamed in the following commit.

Signed-off-by: Sergej Proskurin 
Acked-by: Julien Grall 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
v6: We added this patch to our patch series.

v7: Add Acked-by Julien Grall.


[...]


diff --git a/xen/include/asm-arm/guest_access.h 
b/xen/include/asm-arm/guest_access.h
index 251e935597..49716501a4 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void *to, const 
void *from,
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
 unsigned long raw_clear_guest(void *to, unsigned len);

+int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
+ uint32_t size, bool_t is_write);
+
 #define __raw_copy_to_guest raw_copy_to_guest
 #define __raw_copy_from_guest raw_copy_from_guest
 #define __raw_clear_guest raw_clear_guest
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index d4ed23df28..e489d0bf21 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d, const 
struct vgic_ops *ops);
 int vgic_v2_init(struct domain *d, int *mmio_count);
 int vgic_v3_init(struct domain *d, int *mmio_count);

-int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
- uint32_t size, bool_t is_write);
-
 extern int domain_vgic_register(struct domain *d, int *mmio_count);
 extern int vcpu_vgic_free(struct vcpu *v);
 extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,


As Stefano and Andrew mentioned in patch 11/13, due to a recent patch in
staging, the upper patch failes building due to a missing declaration of
struct domain in . This can be easily fixed by adding a
forward declaration to struct domain right above
vgic_access_guest_memory in  as you will find in the
following patch.


Why the forward declaration and not directly including xen/sched.h?

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

2017-08-16 Thread Sergej Proskurin 
Hi Julien,


On 08/09/2017 10:20 AM, Sergej Proskurin wrote:
> This commit moves the function vgic_access_guest_memory to guestcopy.c
> and the header asm/guest_access.h. No functional changes are made.
> Please note that the function will be renamed in the following commit.
>
> Signed-off-by: Sergej Proskurin 
> Acked-by: Julien Grall 
> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> ---
> v6: We added this patch to our patch series.
>
> v7: Add Acked-by Julien Grall.

[...]

> diff --git a/xen/include/asm-arm/guest_access.h 
> b/xen/include/asm-arm/guest_access.h
> index 251e935597..49716501a4 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void *to, 
> const void *from,
>  unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
>  unsigned long raw_clear_guest(void *to, unsigned len);
>  
> +int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
> + uint32_t size, bool_t is_write);
> +
>  #define __raw_copy_to_guest raw_copy_to_guest
>  #define __raw_copy_from_guest raw_copy_from_guest
>  #define __raw_clear_guest raw_clear_guest
> diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
> index d4ed23df28..e489d0bf21 100644
> --- a/xen/include/asm-arm/vgic.h
> +++ b/xen/include/asm-arm/vgic.h
> @@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d, const 
> struct vgic_ops *ops);
>  int vgic_v2_init(struct domain *d, int *mmio_count);
>  int vgic_v3_init(struct domain *d, int *mmio_count);
>  
> -int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
> - uint32_t size, bool_t is_write);
> -
>  extern int domain_vgic_register(struct domain *d, int *mmio_count);
>  extern int vcpu_vgic_free(struct vcpu *v);
>  extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,

As Stefano and Andrew mentioned in patch 11/13, due to a recent patch in
staging, the upper patch failes building due to a missing declaration of
struct domain in . This can be easily fixed by adding a
forward declaration to struct domain right above
vgic_access_guest_memory in  as you will find in the
following patch.

Although this change already fixed the build on my machine, according to
Travis CI one build (XEN_TARGET_ARCH=arm64
CROSS_COMPILE=aarch64-linux-gnu- XEN_CONFIG_EXPERT=y RANDCONFIG=y
debug=n) failed due to missing information of the types paddr_t,
uint32_t, bool_t etc. Which is the reason why I have included
.

The header  already includes . However,
as  gets included from gcov.h before 
it leads to the upper missing type information issues.

Would the following changes be ok with you or shall I remove your
Acked-by in v9?



diff --git a/xen/include/asm-arm/guest_access.h
b/xen/include/asm-arm/guest_access.h
index 251e935597..8038e885f4 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -3,6 +3,7 @@

 #include 
 #include 
+#include 

 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len);
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
@@ -10,6 +11,10 @@ unsigned long raw_copy_to_guest_flush_dcache(void
*to, const void *from,
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned
len);
 unsigned long raw_clear_guest(void *to, unsigned len);

+struct domain;
+int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
+ uint32_t size, bool_t is_write);
+
 #define __raw_copy_to_guest raw_copy_to_guest
 #define __raw_copy_from_guest raw_copy_from_guest
 #define __raw_clear_guest raw_clear_guest
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index d4ed23df28..e489d0bf21 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -217,9 +217,6 @@ extern void register_vgic_ops(struct domain *d,
const struct vgic_ops *ops);
 int vgic_v2_init(struct domain *d, int *mmio_count);
 int vgic_v3_init(struct domain *d, int *mmio_count);

-int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
- uint32_t size, bool_t is_write);
-
 extern int domain_vgic_register(struct domain *d, int *mmio_count);
 extern int vcpu_vgic_free(struct vcpu *v);
 extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
--
2.13.3



Thanks,
~Sergej


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

[Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

2017-08-09 Thread Sergej Proskurin 
This commit moves the function vgic_access_guest_memory to guestcopy.c
and the header asm/guest_access.h. No functional changes are made.
Please note that the function will be renamed in the following commit.

Signed-off-by: Sergej Proskurin 
Acked-by: Julien Grall 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
v6: We added this patch to our patch series.

v7: Add Acked-by Julien Grall.
---
 xen/arch/arm/guestcopy.c   | 50 ++
 xen/arch/arm/vgic-v3-its.c |  1 +
 xen/arch/arm/vgic.c| 49 -
 xen/include/asm-arm/guest_access.h |  3 +++
 xen/include/asm-arm/vgic.h |  3 ---
 5 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 413125f02b..938ffe2668 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -118,6 +118,56 @@ unsigned long raw_copy_from_guest(void *to, const void 
__user *from, unsigned le
 }
 return 0;
 }
+
+/*
+ * Temporarily map one physical guest page and copy data to or from it.
+ * The data to be copied cannot cross a page boundary.
+ */
+int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
+ uint32_t size, bool is_write)
+{
+struct page_info *page;
+uint64_t offset = gpa & ~PAGE_MASK;  /* Offset within the mapped page */
+p2m_type_t p2mt;
+void *p;
+
+/* Do not cross a page boundary. */
+if ( size > (PAGE_SIZE - offset) )
+{
+printk(XENLOG_G_ERR "d%d: vITS: memory access would cross page 
boundary\n",
+   d->domain_id);
+return -EINVAL;
+}
+
+page = get_page_from_gfn(d, paddr_to_pfn(gpa), , P2M_ALLOC);
+if ( !page )
+{
+printk(XENLOG_G_ERR "d%d: vITS: Failed to get table entry\n",
+   d->domain_id);
+return -EINVAL;
+}
+
+if ( !p2m_is_ram(p2mt) )
+{
+put_page(page);
+printk(XENLOG_G_ERR "d%d: vITS: memory used by the ITS should be RAM.",
+   d->domain_id);
+return -EINVAL;
+}
+
+p = __map_domain_page(page);
+
+if ( is_write )
+memcpy(p + offset, buf, size);
+else
+memcpy(buf, p + offset, size);
+
+unmap_domain_page(p);
+put_page(page);
+
+return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 9ef792f479..1af6820cab 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 1e5107b9f8..7a4e3cdc88 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -638,55 +638,6 @@ void vgic_free_virq(struct domain *d, unsigned int virq)
 }
 
 /*
- * Temporarily map one physical guest page and copy data to or from it.
- * The data to be copied cannot cross a page boundary.
- */
-int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
- uint32_t size, bool is_write)
-{
-struct page_info *page;
-uint64_t offset = gpa & ~PAGE_MASK;  /* Offset within the mapped page */
-p2m_type_t p2mt;
-void *p;
-
-/* Do not cross a page boundary. */
-if ( size > (PAGE_SIZE - offset) )
-{
-printk(XENLOG_G_ERR "d%d: vITS: memory access would cross page 
boundary\n",
-   d->domain_id);
-return -EINVAL;
-}
-
-page = get_page_from_gfn(d, paddr_to_pfn(gpa), , P2M_ALLOC);
-if ( !page )
-{
-printk(XENLOG_G_ERR "d%d: vITS: Failed to get table entry\n",
-   d->domain_id);
-return -EINVAL;
-}
-
-if ( !p2m_is_ram(p2mt) )
-{
-put_page(page);
-printk(XENLOG_G_ERR "d%d: vITS: memory used by the ITS should be RAM.",
-   d->domain_id);
-return -EINVAL;
-}
-
-p = __map_domain_page(page);
-
-if ( is_write )
-memcpy(p + offset, buf, size);
-else
-memcpy(buf, p + offset, size);
-
-unmap_domain_page(p);
-put_page(page);
-
-return 0;
-}
-
-/*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
diff --git a/xen/include/asm-arm/guest_access.h 
b/xen/include/asm-arm/guest_access.h
index 251e935597..49716501a4 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -10,6 +10,9 @@ unsigned long raw_copy_to_guest_flush_dcache(void *to, const 
void *from,
 unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len);
 unsigned long raw_clear_guest(void *to, unsigned len);
 
+int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
+ uint32_t size, bool_t is_write);
+
 #define __raw_copy_to_guest raw_copy_to_guest
 #define __raw_copy_from_guest