Re: [Xen-devel] [PATCH v8 08/13] arm/guest_access: Move vgic_access_guest_memory to guest_access.h
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
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
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
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), &p2mt, 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), &p2mt, 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 raw_copy_from_guest #define __raw_clear_guest raw_clear_guest diff --git a/xen/include/asm-ar