* [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h
@ 2020-04-04 13:10 Julien Grall
2020-04-04 13:10 ` [PATCH 1/7] xen/guest_access: Add missing emacs magics Julien Grall
` (7 more replies)
0 siblings, 8 replies; 29+ messages in thread
From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, Stefano Stabellini, julien, Jun Nakajima, Wei Liu,
Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson,
George Dunlap, Jan Beulich, Volodymyr Babchuk,
Roger Pau Monné
From: Julien Grall <jgrall@amazon.com>
Hi all,
A lot of the helpers implemented in asm-*/guest_access.h are implemented
the same way. This series aims to avoid the duplication and implement
them only once in xen/guest_access.h.
Cheers,
Julien Grall (7):
xen/guest_access: Add missing emacs magics
xen/arm: kernel: Re-order the includes
xen/arm: decode: Re-order the includes
xen/arm: guestcopy: Re-order the includes
xen: include xen/guest_access.h rather than asm/guest_access.h
xen/guest_access: Consolidate guest access helpers in
xen/guest_access.h
xen/guest_access: Fix coding style in xen/guest_access.h
xen/arch/arm/decode.c | 7 +-
xen/arch/arm/domain.c | 2 +-
xen/arch/arm/guest_walk.c | 3 +-
xen/arch/arm/guestcopy.c | 5 +-
xen/arch/arm/kernel.c | 12 +--
xen/arch/arm/vgic-v3-its.c | 2 +-
xen/arch/x86/hvm/svm/svm.c | 2 +-
xen/arch/x86/hvm/viridian/viridian.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 2 +-
xen/common/libelf/libelf-loader.c | 2 +-
xen/include/asm-arm/guest_access.h | 132 ------------------------
xen/include/asm-x86/guest_access.h | 129 ++---------------------
xen/include/xen/guest_access.h | 149 +++++++++++++++++++++++++++
xen/lib/x86/private.h | 2 +-
14 files changed, 178 insertions(+), 273 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/7] xen/guest_access: Add missing emacs magics 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-07 8:05 ` Jan Beulich 2020-04-04 13:10 ` [PATCH 2/7] xen/arm: kernel: Re-order the includes Julien Grall ` (6 subsequent siblings) 7 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, julien, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, Jan Beulich, Roger Pau Monné From: Julien Grall <jgrall@amazon.com> Add missing emacs magics for xen/guest_access.h and asm-x86/guest_access.h. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/include/asm-x86/guest_access.h | 8 ++++++++ xen/include/xen/guest_access.h | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h index 0b58f2baee..9ee275d01f 100644 --- a/xen/include/asm-x86/guest_access.h +++ b/xen/include/asm-x86/guest_access.h @@ -175,3 +175,11 @@ }) #endif /* __ASM_X86_GUEST_ACCESS_H__ */ +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h index 09989df819..ef9aaa3efc 100644 --- a/xen/include/xen/guest_access.h +++ b/xen/include/xen/guest_access.h @@ -33,3 +33,11 @@ char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char) u_buf, size_t size, size_t max_size); #endif /* __XEN_GUEST_ACCESS_H__ */ +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] xen/guest_access: Add missing emacs magics 2020-04-04 13:10 ` [PATCH 1/7] xen/guest_access: Add missing emacs magics Julien Grall @ 2020-04-07 8:05 ` Jan Beulich 2020-04-08 21:43 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-07 8:05 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Roger Pau Monné On 04.04.2020 15:10, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Add missing emacs magics for xen/guest_access.h and > asm-x86/guest_access.h. > > Signed-off-by: Julien Grall <jgrall@amazon.com> I don't think these are "missing"; as per ./CODING_STYLE they're permitted, but not required (and I continue to question why one form of such a comment should be preferred over possible other forms other editors may support). Nevertheless, as this is in line with what we have elsewhere: Acked-by: Jan Beulich <jbeulich@suse.com> Jan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] xen/guest_access: Add missing emacs magics 2020-04-07 8:05 ` Jan Beulich @ 2020-04-08 21:43 ` Julien Grall 0 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-08 21:43 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Roger Pau Monné Hi Jan, On 07/04/2020 09:05, Jan Beulich wrote: > On 04.04.2020 15:10, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> Add missing emacs magics for xen/guest_access.h and >> asm-x86/guest_access.h. >> >> Signed-off-by: Julien Grall <jgrall@amazon.com> > > I don't think these are "missing"; as per ./CODING_STYLE they're > permitted, but not required (and I continue to question why one > form of such a comment should be preferred over possible other > forms other editors may support). Nevertheless, as this is in > line with what we have elsewhere: I can remove the "missing" words if you prefer. > > Acked-by: Jan Beulich <jbeulich@suse.com> Thank you! Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/7] xen/arm: kernel: Re-order the includes 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall 2020-04-04 13:10 ` [PATCH 1/7] xen/guest_access: Add missing emacs magics Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-04 13:10 ` [PATCH 3/7] xen/arm: decode: " Julien Grall ` (5 subsequent siblings) 7 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien From: Julien Grall <jgrall@amazon.com> We usually have xen/ includes first and then asm/. They are also ordered alphabetically among themselves. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/arch/arm/kernel.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c index 8eff074836..f95fa392af 100644 --- a/xen/arch/arm/kernel.c +++ b/xen/arch/arm/kernel.c @@ -3,20 +3,20 @@ * * Copyright (C) 2011 Citrix Systems, Inc. */ +#include <xen/domain_page.h> #include <xen/errno.h> +#include <xen/gunzip.h> #include <xen/init.h> #include <xen/lib.h> +#include <xen/libfdt/libfdt.h> #include <xen/mm.h> -#include <xen/domain_page.h> #include <xen/sched.h> -#include <asm/byteorder.h> -#include <asm/setup.h> -#include <xen/libfdt/libfdt.h> -#include <xen/gunzip.h> #include <xen/vmap.h> +#include <asm/byteorder.h> #include <asm/guest_access.h> #include <asm/kernel.h> +#include <asm/setup.h> #define UIMAGE_MAGIC 0x27051956 #define UIMAGE_NMLEN 32 -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/7] xen/arm: decode: Re-order the includes 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall 2020-04-04 13:10 ` [PATCH 1/7] xen/guest_access: Add missing emacs magics Julien Grall 2020-04-04 13:10 ` [PATCH 2/7] xen/arm: kernel: Re-order the includes Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-04 13:10 ` [PATCH 4/7] xen/arm: guestcopy: " Julien Grall ` (4 subsequent siblings) 7 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien From: Julien Grall <jgrall@amazon.com> We usually have xen/ includes first and then asm/. They are also ordered alphabetically among themselves. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/arch/arm/decode.c | 5 +++-- xen/arch/arm/kernel.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c index 8b1e15d118..144793c8ce 100644 --- a/xen/arch/arm/decode.c +++ b/xen/arch/arm/decode.c @@ -17,11 +17,12 @@ * GNU General Public License for more details. */ -#include <xen/types.h> +#include <xen/lib.h> #include <xen/sched.h> +#include <xen/types.h> + #include <asm/current.h> #include <asm/guest_access.h> -#include <xen/lib.h> #include "decode.h" diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c index f95fa392af..032923853f 100644 --- a/xen/arch/arm/kernel.c +++ b/xen/arch/arm/kernel.c @@ -5,6 +5,7 @@ */ #include <xen/domain_page.h> #include <xen/errno.h> +#include <xen/guest_access.h> #include <xen/gunzip.h> #include <xen/init.h> #include <xen/lib.h> @@ -14,7 +15,6 @@ #include <xen/vmap.h> #include <asm/byteorder.h> -#include <asm/guest_access.h> #include <asm/kernel.h> #include <asm/setup.h> -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 4/7] xen/arm: guestcopy: Re-order the includes 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall ` (2 preceding siblings ...) 2020-04-04 13:10 ` [PATCH 3/7] xen/arm: decode: " Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-04 13:10 ` [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall ` (3 subsequent siblings) 7 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien From: Julien Grall <jgrall@amazon.com> We usually have xen/ includes first and then asm/. They are also ordered alphabetically among themselves. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/arch/arm/guestcopy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c index 7a0f3e9d5f..c8023e2bca 100644 --- a/xen/arch/arm/guestcopy.c +++ b/xen/arch/arm/guestcopy.c @@ -1,7 +1,8 @@ -#include <xen/lib.h> #include <xen/domain_page.h> +#include <xen/lib.h> #include <xen/mm.h> #include <xen/sched.h> + #include <asm/current.h> #include <asm/guest_access.h> -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall ` (3 preceding siblings ...) 2020-04-04 13:10 ` [PATCH 4/7] xen/arm: guestcopy: " Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-06 7:40 ` Paul Durrant 2020-04-04 13:10 ` [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall ` (2 subsequent siblings) 7 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel Cc: Kevin Tian, Stefano Stabellini, julien, Jun Nakajima, Wei Liu, Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, Jan Beulich, Volodymyr Babchuk, Roger Pau Monné From: Julien Grall <jgrall@amazon.com> Only a few places are actually including asm/guest_access.h. While this is fine today, a follow-up patch will want to move most of the helpers from asm/guest_access.h to xen/guest_access.h. To prepare the move, everyone should include xen/guest_access.h rather than asm/guest_access.h. Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The inclusion is now removed as no-one but the latter should include the former. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/arch/arm/decode.c | 2 +- xen/arch/arm/domain.c | 2 +- xen/arch/arm/guest_walk.c | 3 ++- xen/arch/arm/guestcopy.c | 2 +- xen/arch/arm/vgic-v3-its.c | 2 +- xen/arch/x86/hvm/svm/svm.c | 2 +- xen/arch/x86/hvm/viridian/viridian.c | 2 +- xen/arch/x86/hvm/vmx/vmx.c | 2 +- xen/common/libelf/libelf-loader.c | 2 +- xen/include/asm-arm/guest_access.h | 1 - xen/include/asm-x86/guest_access.h | 22 ++++++++++++---------- xen/lib/x86/private.h | 2 +- 12 files changed, 23 insertions(+), 21 deletions(-) diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c index 144793c8ce..792c2e92a7 100644 --- a/xen/arch/arm/decode.c +++ b/xen/arch/arm/decode.c @@ -17,12 +17,12 @@ * GNU General Public License for more details. */ +#include <xen/guest_access.h> #include <xen/lib.h> #include <xen/sched.h> #include <xen/types.h> #include <asm/current.h> -#include <asm/guest_access.h> #include "decode.h" diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 2190d908eb..b062c232b6 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -12,6 +12,7 @@ #include <xen/bitops.h> #include <xen/errno.h> #include <xen/grant_table.h> +#include <xen/guest_access.h> #include <xen/hypercall.h> #include <xen/init.h> #include <xen/lib.h> @@ -26,7 +27,6 @@ #include <asm/current.h> #include <asm/event.h> #include <asm/gic.h> -#include <asm/guest_access.h> #include <asm/guest_atomics.h> #include <asm/irq.h> #include <asm/p2m.h> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c index a1cdd7f4af..b4496c4c86 100644 --- a/xen/arch/arm/guest_walk.c +++ b/xen/arch/arm/guest_walk.c @@ -16,8 +16,9 @@ */ #include <xen/domain_page.h> +#include <xen/guest_access.h> #include <xen/sched.h> -#include <asm/guest_access.h> + #include <asm/guest_walk.h> #include <asm/short-desc.h> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c index c8023e2bca..32681606d8 100644 --- a/xen/arch/arm/guestcopy.c +++ b/xen/arch/arm/guestcopy.c @@ -1,10 +1,10 @@ #include <xen/domain_page.h> +#include <xen/guest_access.h> #include <xen/lib.h> #include <xen/mm.h> #include <xen/sched.h> #include <asm/current.h> -#include <asm/guest_access.h> #define COPY_flush_dcache (1U << 0) #define COPY_from_guest (0U << 1) diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c index 6e153c698d..58d939b85f 100644 --- a/xen/arch/arm/vgic-v3-its.c +++ b/xen/arch/arm/vgic-v3-its.c @@ -32,6 +32,7 @@ #include <xen/bitops.h> #include <xen/config.h> #include <xen/domain_page.h> +#include <xen/guest_access.h> #include <xen/lib.h> #include <xen/init.h> #include <xen/softirq.h> @@ -39,7 +40,6 @@ #include <xen/sched.h> #include <xen/sizes.h> #include <asm/current.h> -#include <asm/guest_access.h> #include <asm/mmio.h> #include <asm/gic_v3_defs.h> #include <asm/gic_v3_its.h> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 888f504a94..9e14a451eb 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -16,6 +16,7 @@ * this program; If not, see <http://www.gnu.org/licenses/>. */ +#include <xen/guest_access.h> #include <xen/init.h> #include <xen/lib.h> #include <xen/trace.h> @@ -34,7 +35,6 @@ #include <asm/cpufeature.h> #include <asm/processor.h> #include <asm/amd.h> -#include <asm/guest_access.h> #include <asm/debugreg.h> #include <asm/msr.h> #include <asm/i387.h> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c index 977c1bc54f..dc7183a546 100644 --- a/xen/arch/x86/hvm/viridian/viridian.c +++ b/xen/arch/x86/hvm/viridian/viridian.c @@ -5,12 +5,12 @@ * Hypervisor Top Level Functional Specification for more information. */ +#include <xen/guest_access.h> #include <xen/sched.h> #include <xen/version.h> #include <xen/hypercall.h> #include <xen/domain_page.h> #include <xen/param.h> -#include <asm/guest_access.h> #include <asm/guest/hyperv-tlfs.h> #include <asm/paging.h> #include <asm/p2m.h> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 1c398fdb6e..98e9c91ea3 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -15,6 +15,7 @@ * this program; If not, see <http://www.gnu.org/licenses/>. */ +#include <xen/guest_access.h> #include <xen/init.h> #include <xen/lib.h> #include <xen/param.h> @@ -31,7 +32,6 @@ #include <asm/regs.h> #include <asm/cpufeature.h> #include <asm/processor.h> -#include <asm/guest_access.h> #include <asm/debugreg.h> #include <asm/msr.h> #include <asm/p2m.h> diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c index 0f468727d0..629cc0d3e6 100644 --- a/xen/common/libelf/libelf-loader.c +++ b/xen/common/libelf/libelf-loader.c @@ -16,7 +16,7 @@ */ #ifdef __XEN__ -#include <asm/guest_access.h> +#include <xen/guest_access.h> #endif #include "libelf-private.h" diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h index 4046d50347..93d56868f1 100644 --- a/xen/include/asm-arm/guest_access.h +++ b/xen/include/asm-arm/guest_access.h @@ -1,7 +1,6 @@ #ifndef __ASM_ARM_GUEST_ACCESS_H__ #define __ASM_ARM_GUEST_ACCESS_H__ -#include <xen/guest_access.h> #include <xen/errno.h> #include <xen/sched.h> diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h index 9ee275d01f..5c3dfc47b6 100644 --- a/xen/include/asm-x86/guest_access.h +++ b/xen/include/asm-x86/guest_access.h @@ -54,22 +54,24 @@ /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ #define guest_handle_to_param(hnd, type) ({ \ + typeof((hnd).p) _x = (hnd).p; \ + XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ /* type checking: make sure that the pointers inside \ * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ * the same type, then return hnd */ \ - (void)((typeof(&(hnd).p)) 0 == \ - (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \ - (hnd); \ + (void)(&_x == &_y.p); \ + _y; \ }) /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ -#define guest_handle_from_param(hnd, type) ({ \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)((typeof(&(hnd).p)) 0 == \ - (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \ - (hnd); \ +#define guest_handle_from_param(hnd, type) ({ \ + typeof((hnd).p) _x = (hnd).p; \ + XEN_GUEST_HANDLE(type) _y = { _x }; \ + /* type checking: make sure that the pointers inside \ + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ + * the same type, then return hnd */ \ + (void)(&_x == &_y.p); \ + _y; \ }) #define guest_handle_for_field(hnd, type, fld) \ diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h index b793181464..2d53bd3ced 100644 --- a/xen/lib/x86/private.h +++ b/xen/lib/x86/private.h @@ -4,12 +4,12 @@ #ifdef __XEN__ #include <xen/bitops.h> +#include <xen/guest_access.h> #include <xen/kernel.h> #include <xen/lib.h> #include <xen/nospec.h> #include <xen/types.h> -#include <asm/guest_access.h> #include <asm/msr-index.h> #define copy_to_buffer_offset copy_to_guest_offset -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* RE: [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h 2020-04-04 13:10 ` [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall @ 2020-04-06 7:40 ` Paul Durrant 2020-04-06 8:51 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Paul Durrant @ 2020-04-06 7:40 UTC (permalink / raw) To: 'Julien Grall', xen-devel Cc: 'Kevin Tian', 'Stefano Stabellini', 'Jun Nakajima', 'Wei Liu', 'Andrew Cooper', 'Julien Grall', 'Ian Jackson', 'George Dunlap', 'Jan Beulich', 'Volodymyr Babchuk', 'Roger Pau Monné' > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 04 April 2020 14:10 > To: xen-devel@lists.xenproject.org > Cc: julien@xen.org; Julien Grall <jgrall@amazon.com>; Stefano Stabellini <sstabellini@kernel.org>; > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George > Dunlap <george.dunlap@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Jan Beulich > <jbeulich@suse.com>; Wei Liu <wl@xen.org>; Roger Pau Monné <roger.pau@citrix.com>; Paul Durrant > <paul@xen.org>; Jun Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com> > Subject: [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h > > From: Julien Grall <jgrall@amazon.com> > > Only a few places are actually including asm/guest_access.h. While this > is fine today, a follow-up patch will want to move most of the helpers > from asm/guest_access.h to xen/guest_access.h. > > To prepare the move, everyone should include xen/guest_access.h rather > than asm/guest_access.h. > > Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The > inclusion is now removed as no-one but the latter should include the > former. > > Signed-off-by: Julien Grall <jgrall@amazon.com> > --- > xen/arch/arm/decode.c | 2 +- > xen/arch/arm/domain.c | 2 +- > xen/arch/arm/guest_walk.c | 3 ++- > xen/arch/arm/guestcopy.c | 2 +- > xen/arch/arm/vgic-v3-its.c | 2 +- > xen/arch/x86/hvm/svm/svm.c | 2 +- > xen/arch/x86/hvm/viridian/viridian.c | 2 +- > xen/arch/x86/hvm/vmx/vmx.c | 2 +- > xen/common/libelf/libelf-loader.c | 2 +- > xen/include/asm-arm/guest_access.h | 1 - > xen/include/asm-x86/guest_access.h | 22 ++++++++++++---------- > xen/lib/x86/private.h | 2 +- > 12 files changed, 23 insertions(+), 21 deletions(-) > > diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c > index 144793c8ce..792c2e92a7 100644 > --- a/xen/arch/arm/decode.c > +++ b/xen/arch/arm/decode.c > @@ -17,12 +17,12 @@ > * GNU General Public License for more details. > */ > > +#include <xen/guest_access.h> > #include <xen/lib.h> > #include <xen/sched.h> > #include <xen/types.h> > > #include <asm/current.h> > -#include <asm/guest_access.h> > > #include "decode.h" > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 2190d908eb..b062c232b6 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -12,6 +12,7 @@ > #include <xen/bitops.h> > #include <xen/errno.h> > #include <xen/grant_table.h> > +#include <xen/guest_access.h> > #include <xen/hypercall.h> > #include <xen/init.h> > #include <xen/lib.h> > @@ -26,7 +27,6 @@ > #include <asm/current.h> > #include <asm/event.h> > #include <asm/gic.h> > -#include <asm/guest_access.h> > #include <asm/guest_atomics.h> > #include <asm/irq.h> > #include <asm/p2m.h> > diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c > index a1cdd7f4af..b4496c4c86 100644 > --- a/xen/arch/arm/guest_walk.c > +++ b/xen/arch/arm/guest_walk.c > @@ -16,8 +16,9 @@ > */ > > #include <xen/domain_page.h> > +#include <xen/guest_access.h> > #include <xen/sched.h> > -#include <asm/guest_access.h> > + > #include <asm/guest_walk.h> > #include <asm/short-desc.h> > > diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c > index c8023e2bca..32681606d8 100644 > --- a/xen/arch/arm/guestcopy.c > +++ b/xen/arch/arm/guestcopy.c > @@ -1,10 +1,10 @@ > #include <xen/domain_page.h> > +#include <xen/guest_access.h> > #include <xen/lib.h> > #include <xen/mm.h> > #include <xen/sched.h> > > #include <asm/current.h> > -#include <asm/guest_access.h> > > #define COPY_flush_dcache (1U << 0) > #define COPY_from_guest (0U << 1) > diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c > index 6e153c698d..58d939b85f 100644 > --- a/xen/arch/arm/vgic-v3-its.c > +++ b/xen/arch/arm/vgic-v3-its.c > @@ -32,6 +32,7 @@ > #include <xen/bitops.h> > #include <xen/config.h> > #include <xen/domain_page.h> > +#include <xen/guest_access.h> > #include <xen/lib.h> > #include <xen/init.h> > #include <xen/softirq.h> > @@ -39,7 +40,6 @@ > #include <xen/sched.h> > #include <xen/sizes.h> > #include <asm/current.h> > -#include <asm/guest_access.h> > #include <asm/mmio.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic_v3_its.h> > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 888f504a94..9e14a451eb 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -16,6 +16,7 @@ > * this program; If not, see <http://www.gnu.org/licenses/>. > */ > > +#include <xen/guest_access.h> > #include <xen/init.h> > #include <xen/lib.h> > #include <xen/trace.h> > @@ -34,7 +35,6 @@ > #include <asm/cpufeature.h> > #include <asm/processor.h> > #include <asm/amd.h> > -#include <asm/guest_access.h> > #include <asm/debugreg.h> > #include <asm/msr.h> > #include <asm/i387.h> > diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c > index 977c1bc54f..dc7183a546 100644 > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -5,12 +5,12 @@ > * Hypervisor Top Level Functional Specification for more information. > */ > > +#include <xen/guest_access.h> > #include <xen/sched.h> > #include <xen/version.h> > #include <xen/hypercall.h> > #include <xen/domain_page.h> > #include <xen/param.h> > -#include <asm/guest_access.h> > #include <asm/guest/hyperv-tlfs.h> > #include <asm/paging.h> > #include <asm/p2m.h> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 1c398fdb6e..98e9c91ea3 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -15,6 +15,7 @@ > * this program; If not, see <http://www.gnu.org/licenses/>. > */ > > +#include <xen/guest_access.h> > #include <xen/init.h> > #include <xen/lib.h> > #include <xen/param.h> > @@ -31,7 +32,6 @@ > #include <asm/regs.h> > #include <asm/cpufeature.h> > #include <asm/processor.h> > -#include <asm/guest_access.h> > #include <asm/debugreg.h> > #include <asm/msr.h> > #include <asm/p2m.h> > diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c > index 0f468727d0..629cc0d3e6 100644 > --- a/xen/common/libelf/libelf-loader.c > +++ b/xen/common/libelf/libelf-loader.c > @@ -16,7 +16,7 @@ > */ > > #ifdef __XEN__ > -#include <asm/guest_access.h> > +#include <xen/guest_access.h> > #endif > > #include "libelf-private.h" > diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h > index 4046d50347..93d56868f1 100644 > --- a/xen/include/asm-arm/guest_access.h > +++ b/xen/include/asm-arm/guest_access.h > @@ -1,7 +1,6 @@ > #ifndef __ASM_ARM_GUEST_ACCESS_H__ > #define __ASM_ARM_GUEST_ACCESS_H__ > > -#include <xen/guest_access.h> > #include <xen/errno.h> > #include <xen/sched.h> > > diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h > index 9ee275d01f..5c3dfc47b6 100644 > --- a/xen/include/asm-x86/guest_access.h > +++ b/xen/include/asm-x86/guest_access.h > @@ -54,22 +54,24 @@ > > /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ > #define guest_handle_to_param(hnd, type) ({ \ > + typeof((hnd).p) _x = (hnd).p; \ > + XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ > /* type checking: make sure that the pointers inside \ > * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ > * the same type, then return hnd */ \ > - (void)((typeof(&(hnd).p)) 0 == \ > - (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \ > - (hnd); \ > + (void)(&_x == &_y.p); \ > + _y; \ > }) > > /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ > -#define guest_handle_from_param(hnd, type) ({ \ > - /* type checking: make sure that the pointers inside \ > - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ > - * the same type, then return hnd */ \ > - (void)((typeof(&(hnd).p)) 0 == \ > - (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \ > - (hnd); \ > +#define guest_handle_from_param(hnd, type) ({ \ > + typeof((hnd).p) _x = (hnd).p; \ > + XEN_GUEST_HANDLE(type) _y = { _x }; \ > + /* type checking: make sure that the pointers inside \ > + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ > + * the same type, then return hnd */ \ > + (void)(&_x == &_y.p); \ > + _y; \ > }) > The commit comment would have the reader believe that this patch is just some changes in header file inclusion. These last two hunks are something else so I would suggest they get split out into a separate patch. Paul > #define guest_handle_for_field(hnd, type, fld) \ > diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h > index b793181464..2d53bd3ced 100644 > --- a/xen/lib/x86/private.h > +++ b/xen/lib/x86/private.h > @@ -4,12 +4,12 @@ > #ifdef __XEN__ > > #include <xen/bitops.h> > +#include <xen/guest_access.h> > #include <xen/kernel.h> > #include <xen/lib.h> > #include <xen/nospec.h> > #include <xen/types.h> > > -#include <asm/guest_access.h> > #include <asm/msr-index.h> > > #define copy_to_buffer_offset copy_to_guest_offset > -- > 2.17.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h 2020-04-06 7:40 ` Paul Durrant @ 2020-04-06 8:51 ` Julien Grall 0 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-06 8:51 UTC (permalink / raw) To: paul, xen-devel Cc: 'Kevin Tian', 'Stefano Stabellini', 'Jun Nakajima', 'Wei Liu', 'Andrew Cooper', 'Julien Grall', 'Ian Jackson', 'George Dunlap', 'Jan Beulich', 'Volodymyr Babchuk', 'Roger Pau Monné' Hi Paul, On 06/04/2020 08:40, Paul Durrant wrote: >> diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h >> index 9ee275d01f..5c3dfc47b6 100644 >> --- a/xen/include/asm-x86/guest_access.h >> +++ b/xen/include/asm-x86/guest_access.h >> @@ -54,22 +54,24 @@ >> >> /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ >> #define guest_handle_to_param(hnd, type) ({ \ >> + typeof((hnd).p) _x = (hnd).p; \ >> + XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ >> /* type checking: make sure that the pointers inside \ >> * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ >> * the same type, then return hnd */ \ >> - (void)((typeof(&(hnd).p)) 0 == \ >> - (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \ >> - (hnd); \ >> + (void)(&_x == &_y.p); \ >> + _y; \ >> }) >> >> /* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ >> -#define guest_handle_from_param(hnd, type) ({ \ >> - /* type checking: make sure that the pointers inside \ >> - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ >> - * the same type, then return hnd */ \ >> - (void)((typeof(&(hnd).p)) 0 == \ >> - (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \ >> - (hnd); \ >> +#define guest_handle_from_param(hnd, type) ({ \ >> + typeof((hnd).p) _x = (hnd).p; \ >> + XEN_GUEST_HANDLE(type) _y = { _x }; \ >> + /* type checking: make sure that the pointers inside \ >> + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ >> + * the same type, then return hnd */ \ >> + (void)(&_x == &_y.p); \ >> + _y; \ >> }) >> > > The commit comment would have the reader believe that this patch is just some changes in header file inclusion. These last two hunks are something else so I would suggest they get split out into a separate patch. These two chunks were meant to be squashed in patch #6, but I messed up the rebase. I will fix on the next version. Sorry for that. Cheers, > > Paul > >> #define guest_handle_for_field(hnd, type, fld) \ >> diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h >> index b793181464..2d53bd3ced 100644 >> --- a/xen/lib/x86/private.h >> +++ b/xen/lib/x86/private.h >> @@ -4,12 +4,12 @@ >> #ifdef __XEN__ >> >> #include <xen/bitops.h> >> +#include <xen/guest_access.h> >> #include <xen/kernel.h> >> #include <xen/lib.h> >> #include <xen/nospec.h> >> #include <xen/types.h> >> >> -#include <asm/guest_access.h> >> #include <asm/msr-index.h> >> >> #define copy_to_buffer_offset copy_to_guest_offset >> -- >> 2.17.1 > > -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall ` (4 preceding siblings ...) 2020-04-04 13:10 ` [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-07 8:14 ` Jan Beulich 2020-04-04 13:10 ` [PATCH 7/7] xen/guest_access: Fix coding style " Julien Grall 2020-04-04 13:13 ` [PATCH 0/7] xen: Consolidate asm-*/guest_access.h " Julien Grall 7 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, julien, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, Jan Beulich, Volodymyr Babchuk, Roger Pau Monné From: Julien Grall <jgrall@amazon.com> Most of the helpers to access guest memory are implemented the same way on Arm and x86. The only differences are: - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It is still fine to use the Arm implementation on x86. - __clear_guest_offset(): Interestingly the prototype does not match between the x86 and Arm. However, the Arm one is bogus. So the x86 implementation can be used. - guest_handle{,_subrange}_okay(): They are validly differing because Arm is only supporting auto-translated guest and therefore handles are always valid. While it would be possible to adjust the coding style at the same, this is left for a follow-up patch so 'diff' can be used to check the consolidation was done correctly. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/include/asm-arm/guest_access.h | 131 ---------------------------- xen/include/asm-x86/guest_access.h | 125 -------------------------- xen/include/xen/guest_access.h | 135 +++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 256 deletions(-) diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h index 93d56868f1..53766386d3 100644 --- a/xen/include/asm-arm/guest_access.h +++ b/xen/include/asm-arm/guest_access.h @@ -23,102 +23,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf, #define __raw_copy_from_guest raw_copy_from_guest #define __raw_clear_guest raw_clear_guest -/* Remainder copied from x86 -- could be common? */ - -/* Is the guest handle a NULL reference? */ -#define guest_handle_is_null(hnd) ((hnd).p == NULL) - -/* Offset the given guest handle into the array it refers to. */ -#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) -#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) - -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) - * to the specified type of XEN_GUEST_HANDLE_PARAM. */ -#define guest_handle_cast(hnd, type) ({ \ - type *_x = (hnd).p; \ - (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ -}) - -/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ -#define guest_handle_to_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - - -/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ -#define guest_handle_from_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - -#define guest_handle_for_field(hnd, type, fld) \ - ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) - -#define guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr }) -#define const_guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr }) - -/* - * Copy an array of objects to guest context via a guest handle, - * specifying an offset into the guest array. - * - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \ -}) - -/* - * Clear an array of objects in guest context via a guest handle, - * specifying an offset into the guest array. - */ -#define clear_guest_offset(hnd, off, nr) ({ \ - void *_d = (hnd).p; \ - raw_clear_guest(_d+(off), nr); \ -}) - -/* - * Copy an array of objects from guest context via a guest handle, - * specifying an offset into the guest array. - */ -#define copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -/* Copy sub-field of a structure to guest context via a guest handle. */ -#define copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -/* Copy sub-field of a structure from guest context via a guest handle. */ -#define copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - /* * Pre-validate a guest handle. * Allows use of faster __copy_* functions. @@ -127,41 +31,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf, #define guest_handle_okay(hnd, nr) (1) #define guest_handle_subrange_okay(hnd, first, last) (1) -/* - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ -}) - -#define __clear_guest_offset(hnd, off, ptr, nr) ({ \ - __raw_clear_guest(_d+(off), nr); \ -}) - -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -#define __copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - __raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -#define __copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - __raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - #endif /* __ASM_ARM_GUEST_ACCESS_H__ */ /* * Local variables: diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h index 5c3dfc47b6..08c9fbbc78 100644 --- a/xen/include/asm-x86/guest_access.h +++ b/xen/include/asm-x86/guest_access.h @@ -38,95 +38,6 @@ clear_user_hvm((dst), (len)) : \ clear_user((dst), (len))) -/* Is the guest handle a NULL reference? */ -#define guest_handle_is_null(hnd) ((hnd).p == NULL) - -/* Offset the given guest handle into the array it refers to. */ -#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) -#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) - -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) - * to the specified type of XEN_GUEST_HANDLE_PARAM. */ -#define guest_handle_cast(hnd, type) ({ \ - type *_x = (hnd).p; \ - (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ -}) - -/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ -#define guest_handle_to_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - -/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ -#define guest_handle_from_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - -#define guest_handle_for_field(hnd, type, fld) \ - ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) - -#define guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr }) -#define const_guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr }) - -/* - * Copy an array of objects to guest context via a guest handle, - * specifying an offset into the guest array. - * - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \ -}) - -/* - * Copy an array of objects from guest context via a guest handle, - * specifying an offset into the guest array. - */ -#define copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -#define clear_guest_offset(hnd, off, nr) ({ \ - void *_d = (hnd).p; \ - raw_clear_guest(_d+(off), nr); \ -}) - -/* Copy sub-field of a structure to guest context via a guest handle. */ -#define copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -/* Copy sub-field of a structure from guest context via a guest handle. */ -#define copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - /* * Pre-validate a guest handle. * Allows use of faster __copy_* functions. @@ -140,42 +51,6 @@ (last)-(first)+1, \ sizeof(*(hnd).p))) -/* - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ -}) - -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -#define __clear_guest_offset(hnd, off, nr) ({ \ - void *_d = (hnd).p; \ - __raw_clear_guest(_d+(off), nr); \ -}) - -#define __copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - __raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -#define __copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - __raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - #endif /* __ASM_X86_GUEST_ACCESS_H__ */ /* * Local variables: diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h index ef9aaa3efc..f724f995c3 100644 --- a/xen/include/xen/guest_access.h +++ b/xen/include/xen/guest_access.h @@ -11,6 +11,99 @@ #include <xen/types.h> #include <public/xen.h> +/* Is the guest handle a NULL reference? */ +#define guest_handle_is_null(hnd) ((hnd).p == NULL) + +/* Offset the given guest handle into the array it refers to. */ +#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) +#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) + +/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) + * to the specified type of XEN_GUEST_HANDLE_PARAM. */ +#define guest_handle_cast(hnd, type) ({ \ + type *_x = (hnd).p; \ + (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ +}) + +/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ +#define guest_handle_to_param(hnd, type) ({ \ + typeof((hnd).p) _x = (hnd).p; \ + XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ + /* type checking: make sure that the pointers inside \ + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ + * the same type, then return hnd */ \ + (void)(&_x == &_y.p); \ + _y; \ +}) + +/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ +#define guest_handle_from_param(hnd, type) ({ \ + typeof((hnd).p) _x = (hnd).p; \ + XEN_GUEST_HANDLE(type) _y = { _x }; \ + /* type checking: make sure that the pointers inside \ + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ + * the same type, then return hnd */ \ + (void)(&_x == &_y.p); \ + _y; \ +}) + +#define guest_handle_for_field(hnd, type, fld) \ + ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) + +#define guest_handle_from_ptr(ptr, type) \ + ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr }) +#define const_guest_handle_from_ptr(ptr, type) \ + ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr }) + +/* + * Copy an array of objects to guest context via a guest handle, + * specifying an offset into the guest array. + * + * The variable _t is only here to catch at build time whether we are + * copying back to a const guest handle. + */ +#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \ + const typeof(*(ptr)) *_s = (ptr); \ + char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ + void *__maybe_unused _t = (hnd).p; \ + ((void)((hnd).p == (ptr))); \ + raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \ +}) + +/* + * Clear an array of objects in guest context via a guest handle, + * specifying an offset into the guest array. + */ +#define clear_guest_offset(hnd, off, nr) ({ \ + void *_d = (hnd).p; \ + raw_clear_guest(_d+(off), nr); \ +}) + +/* + * Copy an array of objects from guest context via a guest handle, + * specifying an offset into the guest array. + */ +#define copy_from_guest_offset(ptr, hnd, off, nr) ({ \ + const typeof(*(ptr)) *_s = (hnd).p; \ + typeof(*(ptr)) *_d = (ptr); \ + raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ +}) + +/* Copy sub-field of a structure to guest context via a guest handle. */ +#define copy_field_to_guest(hnd, ptr, field) ({ \ + const typeof(&(ptr)->field) _s = &(ptr)->field; \ + void *_d = &(hnd).p->field; \ + ((void)(&(hnd).p->field == &(ptr)->field)); \ + raw_copy_to_guest(_d, _s, sizeof(*_s)); \ +}) + +/* Copy sub-field of a structure from guest context via a guest handle. */ +#define copy_field_from_guest(ptr, hnd, field) ({ \ + const typeof(&(ptr)->field) _s = &(hnd).p->field; \ + typeof(&(ptr)->field) _d = &(ptr)->field; \ + raw_copy_from_guest(_d, _s, sizeof(*_d)); \ +}) + #define copy_to_guest(hnd, ptr, nr) \ copy_to_guest_offset(hnd, 0, ptr, nr) @@ -20,6 +113,48 @@ #define clear_guest(hnd, nr) \ clear_guest_offset(hnd, 0, nr) +/* + * The __copy_* functions should only be used after the guest handle has + * been pre-validated via guest_handle_okay() and + * guest_handle_subrange_okay(). + */ + +/* + * The variable _t is only here to catch at build time whether we are + * copying back to a const guest handle. + */ +#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ + const typeof(*(ptr)) *_s = (ptr); \ + char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ + void *__maybe_unused _t = (hnd).p; \ + ((void)((hnd).p == (ptr))); \ + __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ +}) + +#define __clear_guest_offset(hnd, off, nr) ({ \ + void *_d = (hnd).p; \ + __raw_clear_guest(_d + (off), nr); \ +}) + +#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ + const typeof(*(ptr)) *_s = (hnd).p; \ + typeof(*(ptr)) *_d = (ptr); \ + __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ +}) + +#define __copy_field_to_guest(hnd, ptr, field) ({ \ + const typeof(&(ptr)->field) _s = &(ptr)->field; \ + void *_d = &(hnd).p->field; \ + ((void)(&(hnd).p->field == &(ptr)->field)); \ + __raw_copy_to_guest(_d, _s, sizeof(*_s)); \ +}) + +#define __copy_field_from_guest(ptr, hnd, field) ({ \ + const typeof(&(ptr)->field) _s = &(hnd).p->field; \ + typeof(&(ptr)->field) _d = &(ptr)->field; \ + __raw_copy_from_guest(_d, _s, sizeof(*_d)); \ +}) + #define __copy_to_guest(hnd, ptr, nr) \ __copy_to_guest_offset(hnd, 0, ptr, nr) -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-04 13:10 ` [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall @ 2020-04-07 8:14 ` Jan Beulich 2020-04-08 22:05 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-07 8:14 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 04.04.2020 15:10, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Most of the helpers to access guest memory are implemented the same way > on Arm and x86. The only differences are: > - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() > and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It > is still fine to use the Arm implementation on x86. > - __clear_guest_offset(): Interestingly the prototype does not match > between the x86 and Arm. However, the Arm one is bogus. So the x86 > implementation can be used. > - guest_handle{,_subrange}_okay(): They are validly differing > because Arm is only supporting auto-translated guest and therefore > handles are always valid. While I'm fine in principle with such consolidation, I'm afraid I really need to ask for some historical background to be added here. It may very well be that there's a reason for the separation (likely to be found in the removed ia64 or ppc ports), which may then provide a hint at why future ports may want to have these separated. If such reasons exist, I'd prefer to avoid the back and forth between headers. What we could do in such a case is borrow Linux'es asm-generic/ concept, and move the "typical" implementation there. (And of course if there were no noticable reasons for the split, the change as it is would be fine in general; saying so without having looked at the details of it, yet). Jan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-07 8:14 ` Jan Beulich @ 2020-04-08 22:05 ` Julien Grall 2020-04-09 6:30 ` Jan Beulich 0 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-08 22:05 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné Hi Jan, On 07/04/2020 09:14, Jan Beulich wrote: > On 04.04.2020 15:10, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> Most of the helpers to access guest memory are implemented the same way >> on Arm and x86. The only differences are: >> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >> is still fine to use the Arm implementation on x86. >> - __clear_guest_offset(): Interestingly the prototype does not match >> between the x86 and Arm. However, the Arm one is bogus. So the x86 >> implementation can be used. >> - guest_handle{,_subrange}_okay(): They are validly differing >> because Arm is only supporting auto-translated guest and therefore >> handles are always valid. > > While I'm fine in principle with such consolidation, I'm afraid I > really need to ask for some historical background to be added > here. It may very well be that there's a reason for the separation > (likely to be found in the removed ia64 or ppc ports), which may > then provide a hint at why future ports may want to have these > separated. If such reasons exist, I'd prefer to avoid the back and > forth between headers. What we could do in such a case is borrow > Linux'es asm-generic/ concept, and move the "typical" > implementation there. (And of course if there were no noticable > reasons for the split, the change as it is would be fine in > general; saying so without having looked at the details of it, > yet). Looking at the history, ia64 and ppc used to include a common header called xen/xencomm.h from asm/guest_access.h. This has now disappeared with the removal of the two ports. Regarding future arch, the fact arm and x86 gives me some confidence we are unlikely going to get a new ABI for an arch. Do you see any reason to? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-08 22:05 ` Julien Grall @ 2020-04-09 6:30 ` Jan Beulich 2020-04-09 8:01 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-09 6:30 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 09.04.2020 00:05, Julien Grall wrote: > Hi Jan, > > On 07/04/2020 09:14, Jan Beulich wrote: >> On 04.04.2020 15:10, Julien Grall wrote: >>> From: Julien Grall <jgrall@amazon.com> >>> >>> Most of the helpers to access guest memory are implemented the same way >>> on Arm and x86. The only differences are: >>> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >>> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >>> is still fine to use the Arm implementation on x86. >>> - __clear_guest_offset(): Interestingly the prototype does not match >>> between the x86 and Arm. However, the Arm one is bogus. So the x86 >>> implementation can be used. >>> - guest_handle{,_subrange}_okay(): They are validly differing >>> because Arm is only supporting auto-translated guest and therefore >>> handles are always valid. >> >> While I'm fine in principle with such consolidation, I'm afraid I >> really need to ask for some historical background to be added >> here. It may very well be that there's a reason for the separation >> (likely to be found in the removed ia64 or ppc ports), which may >> then provide a hint at why future ports may want to have these >> separated. If such reasons exist, I'd prefer to avoid the back and >> forth between headers. What we could do in such a case is borrow >> Linux'es asm-generic/ concept, and move the "typical" >> implementation there. (And of course if there were no noticable >> reasons for the split, the change as it is would be fine in >> general; saying so without having looked at the details of it, >> yet). > > Looking at the history, ia64 and ppc used to include a common > header called xen/xencomm.h from asm/guest_access.h. > > This has now disappeared with the removal of the two ports. > > Regarding future arch, the fact arm and x86 gives me some confidence > we are unlikely going to get a new ABI for an arch. Do you see any > reason to? Well, there surely had be a reason for ia64 and ppc to use a different approach. I don't see why a new port may not also want to go that route instead of the x86/Arm one. Jan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-09 6:30 ` Jan Beulich @ 2020-04-09 8:01 ` Julien Grall 2020-04-09 8:06 ` Jan Beulich 0 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-09 8:01 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné Hi, On 09/04/2020 07:30, Jan Beulich wrote: > On 09.04.2020 00:05, Julien Grall wrote: >> Hi Jan, >> >> On 07/04/2020 09:14, Jan Beulich wrote: >>> On 04.04.2020 15:10, Julien Grall wrote: >>>> From: Julien Grall <jgrall@amazon.com> >>>> >>>> Most of the helpers to access guest memory are implemented the same way >>>> on Arm and x86. The only differences are: >>>> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >>>> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >>>> is still fine to use the Arm implementation on x86. >>>> - __clear_guest_offset(): Interestingly the prototype does not match >>>> between the x86 and Arm. However, the Arm one is bogus. So the x86 >>>> implementation can be used. >>>> - guest_handle{,_subrange}_okay(): They are validly differing >>>> because Arm is only supporting auto-translated guest and therefore >>>> handles are always valid. >>> >>> While I'm fine in principle with such consolidation, I'm afraid I >>> really need to ask for some historical background to be added >>> here. It may very well be that there's a reason for the separation >>> (likely to be found in the removed ia64 or ppc ports), which may >>> then provide a hint at why future ports may want to have these >>> separated. If such reasons exist, I'd prefer to avoid the back and >>> forth between headers. What we could do in such a case is borrow >>> Linux'es asm-generic/ concept, and move the "typical" >>> implementation there. (And of course if there were no noticable >>> reasons for the split, the change as it is would be fine in >>> general; saying so without having looked at the details of it, >>> yet). >> >> Looking at the history, ia64 and ppc used to include a common >> header called xen/xencomm.h from asm/guest_access.h. >> >> This has now disappeared with the removal of the two ports. >> >> Regarding future arch, the fact arm and x86 gives me some confidence >> we are unlikely going to get a new ABI for an arch. Do you see any >> reason to? > > Well, there surely had be a reason for ia64 and ppc to use a > different approach. This was introduced way before my time in Xen. AFAICT, you were already part of the community when 'xencomm' were alive. There are not much information about it only nor in the commits. So do you mind sharing a bit more what 'xencomm' meant to be and why it wasn't introduced on x86? > I don't see why a new port may not also want > to go that route instead of the x86/Arm one. I could accept that someone would want to reinvent a new ABI from scratch for just an hypothetical new arch. However it would be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is RISC-V is only going to re-use what Arm did as Arm already did with x86. I would like to avoid to introduce a new directory asm-generic with just one header in it. Maybe you have some other headers in mind? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-09 8:01 ` Julien Grall @ 2020-04-09 8:06 ` Jan Beulich 2020-04-09 9:28 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-09 8:06 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 09.04.2020 10:01, Julien Grall wrote: > Hi, > > On 09/04/2020 07:30, Jan Beulich wrote: >> On 09.04.2020 00:05, Julien Grall wrote: >>> Hi Jan, >>> >>> On 07/04/2020 09:14, Jan Beulich wrote: >>>> On 04.04.2020 15:10, Julien Grall wrote: >>>>> From: Julien Grall <jgrall@amazon.com> >>>>> >>>>> Most of the helpers to access guest memory are implemented the same way >>>>> on Arm and x86. The only differences are: >>>>> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >>>>> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >>>>> is still fine to use the Arm implementation on x86. >>>>> - __clear_guest_offset(): Interestingly the prototype does not match >>>>> between the x86 and Arm. However, the Arm one is bogus. So the x86 >>>>> implementation can be used. >>>>> - guest_handle{,_subrange}_okay(): They are validly differing >>>>> because Arm is only supporting auto-translated guest and therefore >>>>> handles are always valid. >>>> >>>> While I'm fine in principle with such consolidation, I'm afraid I >>>> really need to ask for some historical background to be added >>>> here. It may very well be that there's a reason for the separation >>>> (likely to be found in the removed ia64 or ppc ports), which may >>>> then provide a hint at why future ports may want to have these >>>> separated. If such reasons exist, I'd prefer to avoid the back and >>>> forth between headers. What we could do in such a case is borrow >>>> Linux'es asm-generic/ concept, and move the "typical" >>>> implementation there. (And of course if there were no noticable >>>> reasons for the split, the change as it is would be fine in >>>> general; saying so without having looked at the details of it, >>>> yet). >>> >>> Looking at the history, ia64 and ppc used to include a common >>> header called xen/xencomm.h from asm/guest_access.h. >>> >>> This has now disappeared with the removal of the two ports. >>> >>> Regarding future arch, the fact arm and x86 gives me some confidence >>> we are unlikely going to get a new ABI for an arch. Do you see any >>> reason to? >> >> Well, there surely had be a reason for ia64 and ppc to use a >> different approach. > > This was introduced way before my time in Xen. AFAICT, you were > already part of the community when 'xencomm' were alive. I was, but I was never actively involved in ia64 or ppc work. > There are not much information about it only nor in the commits. > So do you mind sharing a bit more what 'xencomm' meant to be > and why it wasn't introduced on x86? If I had details, I would have provided at least some hints already. >> I don't see why a new port may not also want >> to go that route instead of the x86/Arm one. > I could accept that someone would want to reinvent a new ABI > from scratch for just an hypothetical new arch. However it would > be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is > RISC-V is only going to re-use what Arm did as Arm already did > with x86. > > I would like to avoid to introduce a new directory asm-generic > with just one header in it. Maybe you have some other headers in > mind? I recall having wondered a few times whether we shouldn't use this concept elsewhere. One case iirc was bitops stuff. Looking over the Linux ones, some atomic and barrier fallback implementations may also sensibly live there, and there are likely more. Jan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-09 8:06 ` Jan Beulich @ 2020-04-09 9:28 ` Julien Grall 2020-04-29 14:04 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-09 9:28 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 09/04/2020 09:06, Jan Beulich wrote: > On 09.04.2020 10:01, Julien Grall wrote: >> Hi, >> >> On 09/04/2020 07:30, Jan Beulich wrote: >>> On 09.04.2020 00:05, Julien Grall wrote: >>> I don't see why a new port may not also want >>> to go that route instead of the x86/Arm one. >> I could accept that someone would want to reinvent a new ABI >> from scratch for just an hypothetical new arch. However it would >> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >> RISC-V is only going to re-use what Arm did as Arm already did >> with x86. >> >> I would like to avoid to introduce a new directory asm-generic >> with just one header in it. Maybe you have some other headers in >> mind? > > I recall having wondered a few times whether we shouldn't use this > concept elsewhere. One case iirc was bitops stuff. Looking over > the Linux ones, some atomic and barrier fallback implementations > may also sensibly live there, and there are likely more. In theory it makes sense but, in the current state of Xen, 'asm-generic' means they are common to Arm and x86. This is basically the same definition as for the directory 'xen'. So how do you draw a line which files go where? To be honest, I don't think we can draw a line without a 3rd architecture. So I would recommend to wait until then to create an asm-generic directory. Meanwhile, I still think the consolidation in xen/ is useful as it makes easier to maintain. It is also going to make easier for RISCv (or a new arch) as they don't have to worry about duplication (if any). In the event they decide to purse their own route, then it is not going to be a massive pain to move part of xen/guest_access.h in asm-generic/guest_access.h and include the latter from {xen, asm} /guest_access.h. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-09 9:28 ` Julien Grall @ 2020-04-29 14:04 ` Julien Grall 2020-04-29 14:07 ` Jan Beulich 2020-05-16 10:25 ` Julien Grall 0 siblings, 2 replies; 29+ messages in thread From: Julien Grall @ 2020-04-29 14:04 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné Hi, Gentle ping. Any comments on the direction to take? On 09/04/2020 10:28, Julien Grall wrote: > > > On 09/04/2020 09:06, Jan Beulich wrote: >> On 09.04.2020 10:01, Julien Grall wrote: >>> Hi, >>> >>> On 09/04/2020 07:30, Jan Beulich wrote: >>>> On 09.04.2020 00:05, Julien Grall wrote: >>>> I don't see why a new port may not also want >>>> to go that route instead of the x86/Arm one. >>> I could accept that someone would want to reinvent a new ABI >>> from scratch for just an hypothetical new arch. However it would >>> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >>> RISC-V is only going to re-use what Arm did as Arm already did >>> with x86. >>> >>> I would like to avoid to introduce a new directory asm-generic >>> with just one header in it. Maybe you have some other headers in >>> mind? >> >> I recall having wondered a few times whether we shouldn't use this >> concept elsewhere. One case iirc was bitops stuff. Looking over >> the Linux ones, some atomic and barrier fallback implementations >> may also sensibly live there, and there are likely more. > > In theory it makes sense but, in the current state of Xen, 'asm-generic' > means they are common to Arm and x86. This is basically the same > definition as for the directory 'xen'. So how do you draw a line which > files go where? > > To be honest, I don't think we can draw a line without a 3rd > architecture. So I would recommend to wait until then to create an > asm-generic directory. > > Meanwhile, I still think the consolidation in xen/ is useful as it makes > easier to maintain. It is also going to make easier for RISCv (or a new > arch) as they don't have to worry about duplication (if any). > > In the event they decide to purse their own route, then it is not going > to be a massive pain to move part of xen/guest_access.h in > asm-generic/guest_access.h and include the latter from {xen, asm} > /guest_access.h. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-29 14:04 ` Julien Grall @ 2020-04-29 14:07 ` Jan Beulich 2020-04-29 14:13 ` Julien Grall 2020-05-16 10:25 ` Julien Grall 1 sibling, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-29 14:07 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 29.04.2020 16:04, Julien Grall wrote: > Gentle ping. Any comments on the direction to take? Just to avoid misunderstanding - while the mail was sent with me as the only one on the To list, I don't think I've been meant, as I've voiced my opinion. I assume you rather meant to ping others to chime in? Jan > On 09/04/2020 10:28, Julien Grall wrote: >> >> >> On 09/04/2020 09:06, Jan Beulich wrote: >>> On 09.04.2020 10:01, Julien Grall wrote: >>>> Hi, >>>> >>>> On 09/04/2020 07:30, Jan Beulich wrote: >>>>> On 09.04.2020 00:05, Julien Grall wrote: >>>>> I don't see why a new port may not also want >>>>> to go that route instead of the x86/Arm one. >>>> I could accept that someone would want to reinvent a new ABI >>>> from scratch for just an hypothetical new arch. However it would >>>> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >>>> RISC-V is only going to re-use what Arm did as Arm already did >>>> with x86. >>>> >>>> I would like to avoid to introduce a new directory asm-generic >>>> with just one header in it. Maybe you have some other headers in >>>> mind? >>> >>> I recall having wondered a few times whether we shouldn't use this >>> concept elsewhere. One case iirc was bitops stuff. Looking over >>> the Linux ones, some atomic and barrier fallback implementations >>> may also sensibly live there, and there are likely more. >> >> In theory it makes sense but, in the current state of Xen, 'asm-generic' means they are common to Arm and x86. This is basically the same definition as for the directory 'xen'. So how do you draw a line which files go where? >> >> To be honest, I don't think we can draw a line without a 3rd architecture. So I would recommend to wait until then to create an asm-generic directory. >> >> Meanwhile, I still think the consolidation in xen/ is useful as it makes easier to maintain. It is also going to make easier for RISCv (or a new arch) as they don't have to worry about duplication (if any). >> >> In the event they decide to purse their own route, then it is not going to be a massive pain to move part of xen/guest_access.h in asm-generic/guest_access.h and include the latter from {xen, asm} /guest_access.h. > > Cheers, > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-29 14:07 ` Jan Beulich @ 2020-04-29 14:13 ` Julien Grall 2020-04-29 14:54 ` Jan Beulich 0 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-29 14:13 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné Hi, On 29/04/2020 15:07, Jan Beulich wrote: > On 29.04.2020 16:04, Julien Grall wrote: >> Gentle ping. Any comments on the direction to take? > > Just to avoid misunderstanding - while the mail was sent with me as > the only one on the To list, I don't think I've been meant, as I've > voiced my opinion. I assume you rather meant to ping others to chime > in? I barely pay attention to CC vs TO. If I am on the list of recipients of an e-mail, then I will at least have a glance. This e-mail is directed to you specifically and also the others. While you may have voiced some of your opinion already, you haven't replied back how you would decide whether an header should be added in xen or asm-generic. So can you please have another and explain how the line can be drawn with just two architectures in place. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-29 14:13 ` Julien Grall @ 2020-04-29 14:54 ` Jan Beulich 2020-04-29 15:03 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-29 14:54 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 29.04.2020 16:13, Julien Grall wrote: > So can you please have another and explain how the line can be drawn with just two architectures in place. There are abstract considerations that can be used to draw the line, as well as knowledge of people on architectures Xen doesn't run on, but where one can - with such knowledge - extrapolate how it would want to be implemented. I don't think the question at this point is where to draw the line, but whether to have asm-generic/ in the first place. Jan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-29 14:54 ` Jan Beulich @ 2020-04-29 15:03 ` Julien Grall 0 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-29 15:03 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk, Roger Pau Monné On 29/04/2020 15:54, Jan Beulich wrote: > On 29.04.2020 16:13, Julien Grall wrote: >> So can you please have another and explain how the line can be drawn with just two architectures in place. > > There are abstract considerations that can be used to draw the > line, as well as knowledge of people on architectures Xen doesn't > run on, but where one can - with such knowledge - extrapolate how > it would want to be implemented. > > I don't think the question at this point is where to draw the > line, but whether to have asm-generic/ in the first place. Well the two come together. You can't add a new directory with no clear view how this is going to be used. At the moment, this would result at best bikeshedding because developpers may have a different opinion on how a new architecture would be implemented in Xen. If you have a 3rd architectures then it would be easier to argue the header should be added in asm-generic/ or xen/: - asm-generic/ should be used if 2 of the architectures are using the same interface - xen/ should be if the 3 architectures are using the same interface Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-04-29 14:04 ` Julien Grall 2020-04-29 14:07 ` Jan Beulich @ 2020-05-16 10:25 ` Julien Grall 2020-05-19 15:05 ` Ian Jackson 1 sibling, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-05-16 10:25 UTC (permalink / raw) To: Stefano Stabellini, Andrew Cooper, George Dunlap, Ian Jackson, Wei Liu, Roger Pau Monné Cc: xen-devel, Julien Grall, Volodymyr Babchuk, Jan Beulich Ping 2. On 29/04/2020 15:04, Julien Grall wrote: > Hi, > > Gentle ping. Any comments on the direction to take? > > On 09/04/2020 10:28, Julien Grall wrote: >> >> >> On 09/04/2020 09:06, Jan Beulich wrote: >>> On 09.04.2020 10:01, Julien Grall wrote: >>>> Hi, >>>> >>>> On 09/04/2020 07:30, Jan Beulich wrote: >>>>> On 09.04.2020 00:05, Julien Grall wrote: >>>>> I don't see why a new port may not also want >>>>> to go that route instead of the x86/Arm one. >>>> I could accept that someone would want to reinvent a new ABI >>>> from scratch for just an hypothetical new arch. However it would >>>> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >>>> RISC-V is only going to re-use what Arm did as Arm already did >>>> with x86. >>>> >>>> I would like to avoid to introduce a new directory asm-generic >>>> with just one header in it. Maybe you have some other headers in >>>> mind? >>> >>> I recall having wondered a few times whether we shouldn't use this >>> concept elsewhere. One case iirc was bitops stuff. Looking over >>> the Linux ones, some atomic and barrier fallback implementations >>> may also sensibly live there, and there are likely more. >> >> In theory it makes sense but, in the current state of Xen, >> 'asm-generic' means they are common to Arm and x86. This is basically >> the same definition as for the directory 'xen'. So how do you draw a >> line which files go where? >> >> To be honest, I don't think we can draw a line without a 3rd >> architecture. So I would recommend to wait until then to create an >> asm-generic directory. >> >> Meanwhile, I still think the consolidation in xen/ is useful as it >> makes easier to maintain. It is also going to make easier for RISCv >> (or a new arch) as they don't have to worry about duplication (if any). >> >> In the event they decide to purse their own route, then it is not >> going to be a massive pain to move part of xen/guest_access.h in >> asm-generic/guest_access.h and include the latter from {xen, asm} >> /guest_access.h. > > Cheers, > -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-05-16 10:25 ` Julien Grall @ 2020-05-19 15:05 ` Ian Jackson 2020-05-29 11:45 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Ian Jackson @ 2020-05-19 15:05 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, George Dunlap, Jan Beulich, xen-devel, Volodymyr Babchuk, Roger Pau Monne Hi. My attention was drawn to this thread. As I understand it, everyone is agreed that deduplicating the implementation is good (I also agree). The debate is only between: 1. Put it in xen/ until an arch comes along that needs something different, at which point maybe introduce an asm-generic-style thing with default implementations. 2. Say, now, that this is a default implementation and it should go in asm-generic. My starting point is that Julien, as the primary author of this cleanup, should be given leeway on a matter of taste like this. (There are as I understand it no wider implications.) Also, ISTM that it can be argued that introducing a new abstraction is an additional piece of work. Doing that is certainly not hampered by Julien's change. So that would be another reason to take Julien's patch as-is. On the merits, I don't have anything to add to the arguments already presented. I am considerably more persuaded by Julien's arguments than Jan's. So on all levels I think this commit should go in, unless there are other concerns that have not been discussed here ? Thanks, Ian. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h 2020-05-19 15:05 ` Ian Jackson @ 2020-05-29 11:45 ` Julien Grall 0 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-05-29 11:45 UTC (permalink / raw) To: Ian Jackson, Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, George Dunlap, Jan Beulich, xen-devel, Volodymyr Babchuk, Roger Pau Monne Hi Ian, On 19/05/2020 16:05, Ian Jackson wrote: > Hi. My attention was drawn to this thread. > > As I understand it, everyone is agreed that deduplicating the > implementation is good (I also agree). The debate is only between: Thank you for stepping in! > > 1. Put it in xen/ until an arch comes along that needs something > different, at which point maybe introduce an asm-generic-style > thing with default implementations. > > 2. Say, now, that this is a default implementation and it should go in > asm-generic. > > My starting point is that Julien, as the primary author of this > cleanup, should be given leeway on a matter of taste like this. > (There are as I understand it no wider implications.) > > Also, ISTM that it can be argued that introducing a new abstraction is > an additional piece of work. Doing that is certainly not hampered by > Julien's change. So that would be another reason to take Julien's > patch as-is. > > On the merits, I don't have anything to add to the arguments already > presented. I am considerably more persuaded by Julien's arguments > than Jan's. > > So on all levels I think this commit should go in, unless there are > other concerns that have not been discussed here ? The major blocker is where the common header lives. The rest are small comments I should address in the next version. I will send a new version (probably post freeze) to address those comments. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 7/7] xen/guest_access: Fix coding style in xen/guest_access.h 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall ` (5 preceding siblings ...) 2020-04-04 13:10 ` [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall @ 2020-04-04 13:10 ` Julien Grall 2020-04-07 8:17 ` Jan Beulich 2020-04-04 13:13 ` [PATCH 0/7] xen: Consolidate asm-*/guest_access.h " Julien Grall 7 siblings, 1 reply; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:10 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, julien, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, Jan Beulich From: Julien Grall <jgrall@amazon.com> * Add space before and after operator * Align \ * Format comments No functional changes expected. Signed-off-by: Julien Grall <jgrall@amazon.com> --- xen/include/xen/guest_access.h | 40 +++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h index f724f995c3..24f03a84ff 100644 --- a/xen/include/xen/guest_access.h +++ b/xen/include/xen/guest_access.h @@ -18,20 +18,24 @@ #define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) #define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) - * to the specified type of XEN_GUEST_HANDLE_PARAM. */ +/* + * Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) + * to the specified type of XEN_GUEST_HANDLE_PARAM. + */ #define guest_handle_cast(hnd, type) ({ \ type *_x = (hnd).p; \ - (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ + (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ }) /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ #define guest_handle_to_param(hnd, type) ({ \ typeof((hnd).p) _x = (hnd).p; \ XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ + /* \ + * type checking: make sure that the pointers inside \ * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ + * the same type, then return hnd. \ + */ \ (void)(&_x == &_y.p); \ _y; \ }) @@ -40,9 +44,11 @@ #define guest_handle_from_param(hnd, type) ({ \ typeof((hnd).p) _x = (hnd).p; \ XEN_GUEST_HANDLE(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ + /* \ + * type checking: make sure that the pointers inside \ * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ + * the same type, then return hnd. \ + */ \ (void)(&_x == &_y.p); \ _y; \ }) @@ -123,12 +129,12 @@ * The variable _t is only here to catch at build time whether we are * copying back to a const guest handle. */ -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ +#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ + const typeof(*(ptr)) *_s = (ptr); \ + char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ + void *__maybe_unused _t = (hnd).p; \ + ((void)((hnd).p == (ptr))); \ + __raw_copy_to_guest(_d + (off), _s, sizeof(*_s) * (nr));\ }) #define __clear_guest_offset(hnd, off, nr) ({ \ @@ -136,10 +142,10 @@ __raw_clear_guest(_d + (off), nr); \ }) -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ +#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ + const typeof(*(ptr)) *_s = (hnd).p; \ + typeof(*(ptr)) *_d = (ptr); \ + __raw_copy_from_guest(_d, _s + (off), sizeof (*_d) * (nr)); \ }) #define __copy_field_to_guest(hnd, ptr, field) ({ \ -- 2.17.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] xen/guest_access: Fix coding style in xen/guest_access.h 2020-04-04 13:10 ` [PATCH 7/7] xen/guest_access: Fix coding style " Julien Grall @ 2020-04-07 8:17 ` Jan Beulich 2020-04-07 9:08 ` Julien Grall 0 siblings, 1 reply; 29+ messages in thread From: Jan Beulich @ 2020-04-07 8:17 UTC (permalink / raw) To: Julien Grall Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel On 04.04.2020 15:10, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > * Add space before and after operator > * Align \ > * Format comments To be honest, despite the reason you give for the split in patch 6, I'd rather see code movement like this to do formatting adjustments right away. But if you're convinced the split is better, I'm not meaning to insist. Jan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] xen/guest_access: Fix coding style in xen/guest_access.h 2020-04-07 8:17 ` Jan Beulich @ 2020-04-07 9:08 ` Julien Grall 0 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-07 9:08 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, xen-devel On 07/04/2020 09:17, Jan Beulich wrote: > On 04.04.2020 15:10, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> * Add space before and after operator >> * Align \ >> * Format comments > > To be honest, despite the reason you give for the split in patch 6, > I'd rather see code movement like this to do formatting adjustments > right away. Thank you for thinking I can move code and also modify coding style at the same time :). However I know mistakes can be easily made and may not be caught during review (possibly leading to an XSA). So I tend to adhere to the KISS principle. > But if you're convinced the split is better, I'm not > meaning to insist. I will keep the code as-is unless someone else think it is bad idea. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall ` (6 preceding siblings ...) 2020-04-04 13:10 ` [PATCH 7/7] xen/guest_access: Fix coding style " Julien Grall @ 2020-04-04 13:13 ` Julien Grall 7 siblings, 0 replies; 29+ messages in thread From: Julien Grall @ 2020-04-04 13:13 UTC (permalink / raw) To: xen-devel Cc: Kevin Tian, Stefano Stabellini, Jun Nakajima, Wei Liu, Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap, Jan Beulich, Volodymyr Babchuk, Roger Pau Monné On 04/04/2020 14:10, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Hi all, > > A lot of the helpers implemented in asm-*/guest_access.h are implemented > the same way. This series aims to avoid the duplication and implement > them only once in xen/guest_access.h. I forgot to mention this is based on "xen/guest_access: Harden *copy_to_guest_offset() to prevent const dest operand" [1]. This will also clash with Jan's patch "guestcopy: evaluate {,__}copy{,_field}_to_guest*() arguments just once" [2]. I am happy to rebase this series on top of it. Cheers, [1] https://lore.kernel.org/xen-devel/20200404130613.26428-1-julien@xen.org/ [2] https://lore.kernel.org/xen-devel/9918b339-e914-7228-5f8e-86c82090b5bd@suse.com/ > > Cheers, > > Julien Grall (7): > xen/guest_access: Add missing emacs magics > xen/arm: kernel: Re-order the includes > xen/arm: decode: Re-order the includes > xen/arm: guestcopy: Re-order the includes > xen: include xen/guest_access.h rather than asm/guest_access.h > xen/guest_access: Consolidate guest access helpers in > xen/guest_access.h > xen/guest_access: Fix coding style in xen/guest_access.h > > xen/arch/arm/decode.c | 7 +- > xen/arch/arm/domain.c | 2 +- > xen/arch/arm/guest_walk.c | 3 +- > xen/arch/arm/guestcopy.c | 5 +- > xen/arch/arm/kernel.c | 12 +-- > xen/arch/arm/vgic-v3-its.c | 2 +- > xen/arch/x86/hvm/svm/svm.c | 2 +- > xen/arch/x86/hvm/viridian/viridian.c | 2 +- > xen/arch/x86/hvm/vmx/vmx.c | 2 +- > xen/common/libelf/libelf-loader.c | 2 +- > xen/include/asm-arm/guest_access.h | 132 ------------------------ > xen/include/asm-x86/guest_access.h | 129 ++--------------------- > xen/include/xen/guest_access.h | 149 +++++++++++++++++++++++++++ > xen/lib/x86/private.h | 2 +- > 14 files changed, 178 insertions(+), 273 deletions(-) > -- Julien Grall ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2020-05-29 11:46 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-04 13:10 [PATCH 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall 2020-04-04 13:10 ` [PATCH 1/7] xen/guest_access: Add missing emacs magics Julien Grall 2020-04-07 8:05 ` Jan Beulich 2020-04-08 21:43 ` Julien Grall 2020-04-04 13:10 ` [PATCH 2/7] xen/arm: kernel: Re-order the includes Julien Grall 2020-04-04 13:10 ` [PATCH 3/7] xen/arm: decode: " Julien Grall 2020-04-04 13:10 ` [PATCH 4/7] xen/arm: guestcopy: " Julien Grall 2020-04-04 13:10 ` [PATCH 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall 2020-04-06 7:40 ` Paul Durrant 2020-04-06 8:51 ` Julien Grall 2020-04-04 13:10 ` [PATCH 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall 2020-04-07 8:14 ` Jan Beulich 2020-04-08 22:05 ` Julien Grall 2020-04-09 6:30 ` Jan Beulich 2020-04-09 8:01 ` Julien Grall 2020-04-09 8:06 ` Jan Beulich 2020-04-09 9:28 ` Julien Grall 2020-04-29 14:04 ` Julien Grall 2020-04-29 14:07 ` Jan Beulich 2020-04-29 14:13 ` Julien Grall 2020-04-29 14:54 ` Jan Beulich 2020-04-29 15:03 ` Julien Grall 2020-05-16 10:25 ` Julien Grall 2020-05-19 15:05 ` Ian Jackson 2020-05-29 11:45 ` Julien Grall 2020-04-04 13:10 ` [PATCH 7/7] xen/guest_access: Fix coding style " Julien Grall 2020-04-07 8:17 ` Jan Beulich 2020-04-07 9:08 ` Julien Grall 2020-04-04 13:13 ` [PATCH 0/7] xen: Consolidate asm-*/guest_access.h " Julien Grall
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.