diff mbox

[v8,08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h

Message ID 4c768aa1-c556-6243-ca58-172aa700d893@sec.in.tum.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sergej Proskurin Aug. 16, 2017, 9:58 a.m. UTC
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 <proskurin@sec.in.tum.de>
> Acked-by: Julien Grall <julien.grall@arm.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> 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 <xen/types.h>. This can be easily fixed by adding a
forward declaration to struct domain right above
vgic_access_guest_memory in <asm/guest_access.h> 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
<xen/types.h>.

The header <xen/guest_access.h> already includes <xen/types.h>. However,
as <xen/guest_access.h> gets included from gcov.h before <xen/types.h>
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?



 extern bool vgic_to_sgi(struct vcpu *v, register_t sgir,
--
2.13.3



Thanks,
~Sergej

Comments

Julien Grall Aug. 16, 2017, 10:11 a.m. UTC | #1
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 <proskurin@sec.in.tum.de>
>> Acked-by: Julien Grall <julien.grall@arm.com>
>> ---
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Julien Grall <julien.grall@arm.com>
>> ---
>> 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 <xen/types.h>. This can be easily fixed by adding a
> forward declaration to struct domain right above
> vgic_access_guest_memory in <asm/guest_access.h> as you will find in the
> following patch.

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

Cheers,
Sergej Proskurin Aug. 16, 2017, 12:35 p.m. UTC | #2
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 <proskurin@sec.in.tum.de>
>>> Acked-by: Julien Grall <julien.grall@arm.com>
>>> ---
>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>> Cc: Julien Grall <julien.grall@arm.com>
>>> ---
>>> 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 <xen/types.h>. This can be easily fixed by adding a
>> forward declaration to struct domain right above
>> vgic_access_guest_memory in <asm/guest_access.h> 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
diff mbox

Patch

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 <xen/guest_access.h>
 #include <xen/errno.h>
+#include <asm/types.h>

 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);