* [RFC][PATCH 0/5] Add V4V to Xen
@ 2012-05-31 15:07 Jean Guyader
2012-05-31 15:07 ` [PATCH 1/5] xen: add ssize_t to types.h Jean Guyader
` (6 more replies)
0 siblings, 7 replies; 53+ messages in thread
From: Jean Guyader @ 2012-05-31 15:07 UTC (permalink / raw)
To: xen-devel; +Cc: Jean Guyader
[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]
V4V is a copy based inter vm communication system.
Please have a look at this thread for more detail
about V4V.
http://lists.xen.org/archives/html/xen-devel/2012-05/msg01866.html
This patch series is work in progress but I wanted to
post it early enough so I can get feedback from people.
Jean Guyader (5):
xen: add ssize_t to types.h
xen: Add headers to include/Makefile
v4v: Introduce VIRQ_V4V
xen: Enforce casting for guest_handle_cast
xen: Add V4V implementation
xen/arch/x86/hvm/hvm.c | 9 +-
xen/arch/x86/x86_32/entry.S | 2 +
xen/arch/x86/x86_64/compat/entry.S | 2 +
xen/arch/x86/x86_64/entry.S | 2 +
xen/common/Makefile | 1 +
xen/common/domain.c | 11 +-
xen/common/event_channel.c | 1 +
xen/common/v4v.c | 1779 ++++++++++++++++++++++++++++++++++++
xen/include/Makefile | 3 +-
xen/include/asm-x86/guest_access.h | 2 +-
xen/include/public/v4v.h | 544 +++++++++++
xen/include/public/xen.h | 4 +-
xen/include/xen/sched.h | 5 +
xen/include/xen/types.h | 1 +
xen/include/xen/v4v.h | 109 +++
15 files changed, 2467 insertions(+), 8 deletions(-)
create mode 100644 xen/common/v4v.c
create mode 100644 xen/include/public/v4v.h
create mode 100644 xen/include/xen/v4v.h
--
1.7.2.5
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 53+ messages in thread* [PATCH 1/5] xen: add ssize_t to types.h 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader @ 2012-05-31 15:07 ` Jean Guyader 2012-05-31 15:29 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 2/5] xen: Add headers to include/Makefile Jean Guyader ` (5 subsequent siblings) 6 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-05-31 15:07 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 144 bytes --] Signed-off-by: Jean Guyader <jean.guyader@citrix.com> --- xen/include/xen/types.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-xen-add-ssize_t-to-types.h.patch --] [-- Type: text/x-patch; name="0001-xen-add-ssize_t-to-types.h.patch", Size: 302 bytes --] diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h index 8596ded..ec992f8 100644 --- a/xen/include/xen/types.h +++ b/xen/include/xen/types.h @@ -58,5 +58,6 @@ typedef __u64 __le64; typedef __u64 __be64; typedef unsigned long uintptr_t; +typedef int ssize_t; #endif /* __TYPES_H__ */ [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 1/5] xen: add ssize_t to types.h 2012-05-31 15:07 ` [PATCH 1/5] xen: add ssize_t to types.h Jean Guyader @ 2012-05-31 15:29 ` Jan Beulich 0 siblings, 0 replies; 53+ messages in thread From: Jan Beulich @ 2012-05-31 15:29 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: >--- a/xen/include/xen/types.h >+++ b/xen/include/xen/types.h >@@ -58,5 +58,6 @@ typedef __u64 __le64; > typedef __u64 __be64; > > typedef unsigned long uintptr_t; >+typedef int ssize_t; If you add a new type, it needs to be defined correctly and portably - here, it obviously needs to follow the definition of size_t (which isn't a typedef of "unsigned int" unilaterally). With ssize_t in particular I have always been wondering what benefit it provides over ptrdiff_t (i.e. whether there are environments where the two could sensibly be different). Jan > > #endif /* __TYPES_H__ */ ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 2/5] xen: Add headers to include/Makefile 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader 2012-05-31 15:07 ` [PATCH 1/5] xen: add ssize_t to types.h Jean Guyader @ 2012-05-31 15:07 ` Jean Guyader 2012-05-31 15:37 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 3/5] v4v: Introduce VIRQ_V4V Jean Guyader ` (4 subsequent siblings) 6 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-05-31 15:07 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 226 bytes --] Add stdlib.h for size_t Add string.h for memcpy Add sys/types.h for ssize_t Signed-off-by: Jean Guyader <jean.guyader@citrix.com> --- xen/include/Makefile | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-xen-Add-headers-to-include-Makefile.patch --] [-- Type: text/x-patch; name="0002-xen-Add-headers-to-include-Makefile.patch", Size: 739 bytes --] diff --git a/xen/include/Makefile b/xen/include/Makefile index 62846a1..67aaef4 100644 --- a/xen/include/Makefile +++ b/xen/include/Makefile @@ -77,8 +77,9 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH)) all: headers.chk +INCLUDE_LIBS=stdint.h stdlib.h string.h sys/types.h headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile - for i in $(filter %.h,$^); do $(CC) -ansi -include stdint.h -Wall -W -Werror -S -o /dev/null -xc $$i || exit 1; echo $$i; done >$@.new + for i in $(filter %.h,$^); do $(CC) -ansi ${INCLUDE_LIBS:%.h=-include %.h} -Wall -W -Werror -S -o /dev/null -xc $$i || exit 1; echo $$i; done >$@.new mv $@.new $@ endif [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 2/5] xen: Add headers to include/Makefile 2012-05-31 15:07 ` [PATCH 2/5] xen: Add headers to include/Makefile Jean Guyader @ 2012-05-31 15:37 ` Jan Beulich 0 siblings, 0 replies; 53+ messages in thread From: Jan Beulich @ 2012-05-31 15:37 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: > Add stdlib.h for size_t > Add string.h for memcpy > Add sys/types.h for ssize_t > > Signed-off-by: Jean Guyader <jean.guyader@citrix.com> Sorry, I don't think this is correct: What you're modifying is the rule to validate public headers, and public headers should never include any of the three above - the only headers consuming defined symbols of which is half-way acceptable are those that exclusively deal with compiler properties (stddef.h, stdint.h, stdbool.h, stdarg.h, and maybe inttypes.h). Plus I don't see how public headers would legitimately and portably (keep 32-on-64 in mind!) use any of the three symbols mentioned above. Jan ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/5] v4v: Introduce VIRQ_V4V 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader 2012-05-31 15:07 ` [PATCH 1/5] xen: add ssize_t to types.h Jean Guyader 2012-05-31 15:07 ` [PATCH 2/5] xen: Add headers to include/Makefile Jean Guyader @ 2012-05-31 15:07 ` Jean Guyader 2012-05-31 15:44 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 4/5] xen: Enforce casting for guest_handle_cast Jean Guyader ` (3 subsequent siblings) 6 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-05-31 15:07 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 221 bytes --] Introduce the global virq VIRQ_V4V Signed-off-by: Jean Guyader <jean.guyader@citrix.com> --- xen/common/event_channel.c | 1 + xen/include/public/xen.h | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0003-v4v-Introduce-VIRQ_V4V.patch --] [-- Type: text/x-patch; name="0003-v4v-Introduce-VIRQ_V4V.patch", Size: 1123 bytes --] diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c index 53777f8..e138600 100644 --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -107,6 +107,7 @@ static int virq_is_global(uint32_t virq) case VIRQ_TIMER: case VIRQ_DEBUG: case VIRQ_XENOPROF: + case VIRQ_V4V: rc = 0; break; case VIRQ_ARCH_0 ... VIRQ_ARCH_7: diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index b2f6c50..033cbba 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -157,7 +157,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); #define VIRQ_CON_RING 8 /* G. (DOM0) Bytes received on console */ #define VIRQ_PCPU_STATE 9 /* G. (DOM0) PCPU state changed */ #define VIRQ_MEM_EVENT 10 /* G. (DOM0) A memory event has occured */ -#define VIRQ_XC_RESERVED 11 /* G. Reserved for XenClient */ +#define VIRQ_V4V 11 /* G. V4V event has occurred */ #define VIRQ_ENOMEM 12 /* G. (DOM0) Low on heap memory */ /* Architecture-specific VIRQ definitions. */ [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 3/5] v4v: Introduce VIRQ_V4V 2012-05-31 15:07 ` [PATCH 3/5] v4v: Introduce VIRQ_V4V Jean Guyader @ 2012-05-31 15:44 ` Jan Beulich 0 siblings, 0 replies; 53+ messages in thread From: Jan Beulich @ 2012-05-31 15:44 UTC (permalink / raw) To: Jean Guyader, xen-devel >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: >--- a/xen/common/event_channel.c >+++ b/xen/common/event_channel.c >@@ -107,6 +107,7 @@ static int virq_is_global(uint32_t virq) > case VIRQ_TIMER: > case VIRQ_DEBUG: > case VIRQ_XENOPROF: >+ case VIRQ_V4V: Either the placement here is wrong (the vIRQ being per-vCPU), ... > rc = 0; > break; > case VIRQ_ARCH_0 ... VIRQ_ARCH_7: >--- a/xen/include/public/xen.h >+++ b/xen/include/public/xen.h >@@ -157,7 +157,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); > #define VIRQ_CON_RING 8 /* G. (DOM0) Bytes received on console */ > #define VIRQ_PCPU_STATE 9 /* G. (DOM0) PCPU state changed */ > #define VIRQ_MEM_EVENT 10 /* G. (DOM0) A memory event has occured */ >-#define VIRQ_XC_RESERVED 11 /* G. Reserved for XenClient */ >+#define VIRQ_V4V 11 /* G. V4V event has occurred */ ... or the comment here is (and was before). This is an ABI property, end hence you can't really convert a vIRQ defined to be global to a per-vCPU one. So if it turns out the comment was wrong, I would argue whether the change here is really acceptable - our kernel, for example, has built-in knowledge of which vIRQ-s are per-vCPU (but of course this should be benign when the only consumer of the vIRQ lives in userland; otoh, a userland consumer can hardly really make use of a per-vCPU one). Jan > #define VIRQ_ENOMEM 12 /* G. (DOM0) Low on heap memory */ > > /* Architecture-specific VIRQ definitions. */ ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader ` (2 preceding siblings ...) 2012-05-31 15:07 ` [PATCH 3/5] v4v: Introduce VIRQ_V4V Jean Guyader @ 2012-05-31 15:07 ` Jean Guyader 2012-05-31 15:47 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 5/5] xen: Add V4V implementation Jean Guyader ` (2 subsequent siblings) 6 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-05-31 15:07 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 298 bytes --] This is useful if you want to cast a guest handle to char * to do pointer aritmetics afterwards with functions like guest_handle_add_offset. Signed-off-by: Jean Guyader <jean.guyader@citrix.com> --- xen/include/asm-x86/guest_access.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0004-xen-Enforce-casting-for-guest_handle_cast.patch --] [-- Type: text/x-patch; name="0004-xen-Enforce-casting-for-guest_handle_cast.patch", Size: 487 bytes --] diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h index 2b429c2..7e95da3 100644 --- a/xen/include/asm-x86/guest_access.h +++ b/xen/include/asm-x86/guest_access.h @@ -47,7 +47,7 @@ /* Cast a guest handle to the specified type of handle. */ #define guest_handle_cast(hnd, type) ({ \ - type *_x = (hnd).p; \ + type *_x = (type *)(hnd).p; \ (XEN_GUEST_HANDLE(type)) { _x }; \ }) [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-05-31 15:07 ` [PATCH 4/5] xen: Enforce casting for guest_handle_cast Jean Guyader @ 2012-05-31 15:47 ` Jan Beulich 2012-06-14 14:08 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Jan Beulich @ 2012-05-31 15:47 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: >--- a/xen/include/asm-x86/guest_access.h >+++ b/xen/include/asm-x86/guest_access.h >@@ -47,7 +47,7 @@ > > /* Cast a guest handle to the specified type of handle. */ > #define guest_handle_cast(hnd, type) ({ \ >- type *_x = (hnd).p; \ >+ type *_x = (type *)(hnd).p; \ You would have to explain how this is safe: Without the cast, we get compiler warnings (and hence build failures due to -Werror) if "type *" and typeof((hnd).p) are incompatible. Adding an explicit cast removes that intentional check. Jan > (XEN_GUEST_HANDLE(type)) { _x }; \ > }) > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-05-31 15:47 ` Jan Beulich @ 2012-06-14 14:08 ` Jean Guyader 2012-06-14 14:23 ` Jan Beulich 2012-06-14 14:26 ` Tim Deegan 0 siblings, 2 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-14 14:08 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel@lists.xen.org On 31/05 04:47, Jan Beulich wrote: > >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: > >--- a/xen/include/asm-x86/guest_access.h > >+++ b/xen/include/asm-x86/guest_access.h > >@@ -47,7 +47,7 @@ > > > > /* Cast a guest handle to the specified type of handle. */ > > #define guest_handle_cast(hnd, type) ({ \ > >- type *_x = (hnd).p; \ > >+ type *_x = (type *)(hnd).p; \ > > > You would have to explain how this is safe: Without the cast, we > get compiler warnings (and hence build failures due to -Werror) > if "type *" and typeof((hnd).p) are incompatible. Adding an > explicit cast removes that intentional check. > I can't realy explain how this is safe because I agree it make this function less safe. Maybe I should put here the reason that led me to do something like that. Here is what I'm trying to do: XEN_GUEST_HANDLE (uint8_t) slop_hnd = guest_handle_cast (pfn_list_hnd, uint8_t); guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); I need to cast to uint8_t first to get the add_offset to behave correctly. Maybe what I need would need a new macro that would do those two operations. What would be the proper way to doing something like this? Thanks, Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 14:08 ` Jean Guyader @ 2012-06-14 14:23 ` Jan Beulich 2012-06-14 14:26 ` Tim Deegan 1 sibling, 0 replies; 53+ messages in thread From: Jan Beulich @ 2012-06-14 14:23 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel@lists.xen.org >>> On 14.06.12 at 16:08, Jean Guyader <jean.guyader@citrix.com> wrote: > On 31/05 04:47, Jan Beulich wrote: >> >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: >> >--- a/xen/include/asm-x86/guest_access.h >> >+++ b/xen/include/asm-x86/guest_access.h >> >@@ -47,7 +47,7 @@ >> > >> > /* Cast a guest handle to the specified type of handle. */ >> > #define guest_handle_cast(hnd, type) ({ \ >> >- type *_x = (hnd).p; \ >> >+ type *_x = (type *)(hnd).p; \ >> >> >> You would have to explain how this is safe: Without the cast, we >> get compiler warnings (and hence build failures due to -Werror) >> if "type *" and typeof((hnd).p) are incompatible. Adding an >> explicit cast removes that intentional check. >> > > I can't realy explain how this is safe because I agree it make > this function less safe. > > Maybe I should put here the reason that led me to do something > like that. Here is what I'm trying to do: > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > guest_handle_cast (pfn_list_hnd, uint8_t); > guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); > pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); > > I need to cast to uint8_t first to get the add_offset to behave > correctly. Maybe what I need would need a new macro that would > do those two operations. > > What would be the proper way to doing something like this? Depends on what pfn_list_hnd's type is. Maybe going through XEN_GUEST_HANDLE(void) would do (perhaps you could even replace uint8_t with void in your code above, making use of gcc's extension allowing void pointer arithmetic). If that doesn't help, maybe you could post a complete example, compilable with your modification, and I could look into making this work with what is there already. Jan ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 14:08 ` Jean Guyader 2012-06-14 14:23 ` Jan Beulich @ 2012-06-14 14:26 ` Tim Deegan 2012-06-14 14:27 ` Tim Deegan 1 sibling, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-14 14:26 UTC (permalink / raw) To: Jean Guyader; +Cc: Jan Beulich, xen-devel@lists.xen.org At 15:08 +0100 on 14 Jun (1339686495), Jean Guyader wrote: > On 31/05 04:47, Jan Beulich wrote: > > >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: > > >--- a/xen/include/asm-x86/guest_access.h > > >+++ b/xen/include/asm-x86/guest_access.h > > >@@ -47,7 +47,7 @@ > > > > > > /* Cast a guest handle to the specified type of handle. */ > > > #define guest_handle_cast(hnd, type) ({ \ > > >- type *_x = (hnd).p; \ > > >+ type *_x = (type *)(hnd).p; \ > > > > > > You would have to explain how this is safe: Without the cast, we > > get compiler warnings (and hence build failures due to -Werror) > > if "type *" and typeof((hnd).p) are incompatible. Adding an > > explicit cast removes that intentional check. > > > > I can't realy explain how this is safe because I agree it make > this function less safe. > > Maybe I should put here the reason that led me to do something > like that. Here is what I'm trying to do: > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > guest_handle_cast (pfn_list_hnd, uint8_t); > guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); > pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); > > I need to cast to uint8_t first to get the add_offset to behave > correctly. Maybe what I need would need a new macro that would > do those two operations. > > What would be the proper way to doing something like this? You could avoid it altogether by dropping struct v4v_ring_data, and passing a v4v_pfn_t array directly with the 'npage' as a separate hypercall argument. AFAICS struct v4v_ring_data has no other useful fields. Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 14:26 ` Tim Deegan @ 2012-06-14 14:27 ` Tim Deegan 2012-06-14 14:40 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-14 14:27 UTC (permalink / raw) To: Jean Guyader; +Cc: Jan Beulich, xen-devel@lists.xen.org At 15:26 +0100 on 14 Jun (1339687574), Tim Deegan wrote: > At 15:08 +0100 on 14 Jun (1339686495), Jean Guyader wrote: > > Maybe I should put here the reason that led me to do something > > like that. Here is what I'm trying to do: > > > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > > guest_handle_cast (pfn_list_hnd, uint8_t); > > guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); > > pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); > > > > I need to cast to uint8_t first to get the add_offset to behave > > correctly. Maybe what I need would need a new macro that would > > do those two operations. > > > > What would be the proper way to doing something like this? > > You could avoid it altogether by dropping struct v4v_ring_data, and > passing a v4v_pfn_t array directly with the 'npage' as a separate > hypercall argument. AFAICS struct v4v_ring_data has no other useful > fields. Excuse me, I meant struct v4v_pfn_list_t. Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 14:27 ` Tim Deegan @ 2012-06-14 14:40 ` Jean Guyader 2012-06-14 15:39 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-14 14:40 UTC (permalink / raw) To: Tim Deegan; +Cc: Jan Beulich, xen-devel@lists.xen.org On 14/06 03:27, Tim Deegan wrote: > At 15:26 +0100 on 14 Jun (1339687574), Tim Deegan wrote: > > At 15:08 +0100 on 14 Jun (1339686495), Jean Guyader wrote: > > > Maybe I should put here the reason that led me to do something > > > like that. Here is what I'm trying to do: > > > > > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > > > guest_handle_cast (pfn_list_hnd, uint8_t); > > > guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); > > > pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); > > > > > > I need to cast to uint8_t first to get the add_offset to behave > > > correctly. Maybe what I need would need a new macro that would > > > do those two operations. > > > > > > What would be the proper way to doing something like this? > > > > You could avoid it altogether by dropping struct v4v_ring_data, and > > passing a v4v_pfn_t array directly with the 'npage' as a separate > > hypercall argument. AFAICS struct v4v_ring_data has no other useful > > fields. > > Excuse me, I meant struct v4v_pfn_list_t. > You probably mean both, because we doing the same thing with v4v_ring_data_t and v4v_ring_data_ent_t. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 14:40 ` Jean Guyader @ 2012-06-14 15:39 ` Jean Guyader 2012-06-14 15:50 ` Tim Deegan ` (2 more replies) 0 siblings, 3 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-14 15:39 UTC (permalink / raw) To: Tim (Xen.org); +Cc: Jan Beulich, xen-devel@lists.xen.org On 14/06 03:40, Jean Guyader wrote: > On 14/06 03:27, Tim Deegan wrote: > > At 15:26 +0100 on 14 Jun (1339687574), Tim Deegan wrote: > > > At 15:08 +0100 on 14 Jun (1339686495), Jean Guyader wrote: > > > > Maybe I should put here the reason that led me to do something > > > > like that. Here is what I'm trying to do: > > > > > > > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > > > > guest_handle_cast (pfn_list_hnd, uint8_t); > > > > guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); > > > > pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); > > > > > > > > I need to cast to uint8_t first to get the add_offset to behave > > > > correctly. Maybe what I need would need a new macro that would > > > > do those two operations. > > > > > > > > What would be the proper way to doing something like this? > > > > > > You could avoid it altogether by dropping struct v4v_ring_data, and > > > passing a v4v_pfn_t array directly with the 'npage' as a separate > > > hypercall argument. AFAICS struct v4v_ring_data has no other useful > > > fields. > > > > Excuse me, I meant struct v4v_pfn_list_t. > > > > You probably mean both, because we doing the same thing with > v4v_ring_data_t and v4v_ring_data_ent_t. > Actually I don't want to get rid of the v4v_ring_data_t struct. The idea being this struct is that we might want to extend it in the future so having a wrapper arround with a magic is important to detect which version of the struct is being used. Here are the structs: typedef struct v4v_ring_data_ent { struct v4v_addr ring; uint16_t flags; uint16_t pad0; uint32_t space_required; uint32_t max_message_size; } v4v_ring_data_ent_t; DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_ent_t); typedef struct v4v_ring_data { uint64_t magic; uint32_t nent; uint32_t padding; uint64_t reserved[4]; v4v_ring_data_ent_t ring[0]; } v4v_ring_data_t; DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_t); I get a XEN_GUEST_HANDLE(v4v_ring_data_t) as argument of my hypercall and I would like to access the ring data inside it which is a XEN_GUEST_HANDLE as well. Here is the code that I use for doing that (with explicte cast in guest_handle_cast): XEN_GUEST_HANDLE (v4v_ring_data_ent_t) ring_data_ent_hnd; XEN_GUEST_HANDLE (uint8_t) slop_hnd = guest_handle_cast (ring_data_hnd, uint8_t); guest_handle_add_offset (slop_hnd, sizeof (v4v_ring_data_t)); ring_data_ent_hnd = guest_handle_cast (slop_hnd, v4v_ring_data_ent_t); ret = v4v_fill_ring_datas (d, ring_data.nent, ring_data_ent_hnd); Thanks for your help. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 15:39 ` Jean Guyader @ 2012-06-14 15:50 ` Tim Deegan 2012-06-14 16:00 ` Jan Beulich 2012-06-18 11:36 ` Jan Beulich 2 siblings, 0 replies; 53+ messages in thread From: Tim Deegan @ 2012-06-14 15:50 UTC (permalink / raw) To: Jean Guyader; +Cc: Jan Beulich, xen-devel@lists.xen.org At 16:39 +0100 on 14 Jun (1339691953), Jean Guyader wrote: > On 14/06 03:40, Jean Guyader wrote: > > On 14/06 03:27, Tim Deegan wrote: > > > At 15:26 +0100 on 14 Jun (1339687574), Tim Deegan wrote: > > > > At 15:08 +0100 on 14 Jun (1339686495), Jean Guyader wrote: > > > > > Maybe I should put here the reason that led me to do something > > > > > like that. Here is what I'm trying to do: > > > > > > > > > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > > > > > guest_handle_cast (pfn_list_hnd, uint8_t); > > > > > guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); > > > > > pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); > > > > > > > > > > I need to cast to uint8_t first to get the add_offset to behave > > > > > correctly. Maybe what I need would need a new macro that would > > > > > do those two operations. > > > > > > > > > > What would be the proper way to doing something like this? > > > > > > > > You could avoid it altogether by dropping struct v4v_ring_data, and > > > > passing a v4v_pfn_t array directly with the 'npage' as a separate > > > > hypercall argument. AFAICS struct v4v_ring_data has no other useful > > > > fields. > > > > > > Excuse me, I meant struct v4v_pfn_list_t. > > > > > > > You probably mean both, because we doing the same thing with > > v4v_ring_data_t and v4v_ring_data_ent_t. > > > > Actually I don't want to get rid of the v4v_ring_data_t struct. > > The idea being this struct is that we might want to extend it in the future > so having a wrapper arround with a magic is important to detect which > version of the struct is being used. Do you have concrete ideas for how you might want to extend the header, or is this just generic futureproofing? Since each of these structs is only used for one command you can allocate new command numbers if you need to add more arguments later. That's worked well enough for other hypercalls in the past. Cheers, Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 15:39 ` Jean Guyader 2012-06-14 15:50 ` Tim Deegan @ 2012-06-14 16:00 ` Jan Beulich 2012-06-14 21:19 ` Jean Guyader 2012-06-18 11:36 ` Jan Beulich 2 siblings, 1 reply; 53+ messages in thread From: Jan Beulich @ 2012-06-14 16:00 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), xen-devel@lists.xen.org >>> On 14.06.12 at 17:39, Jean Guyader <jean.guyader@citrix.com> wrote: > Here are the structs: > > typedef struct v4v_ring_data_ent > { > struct v4v_addr ring; > uint16_t flags; > uint16_t pad0; > uint32_t space_required; > uint32_t max_message_size; > } v4v_ring_data_ent_t; > DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_ent_t); > > typedef struct v4v_ring_data > { > uint64_t magic; > uint32_t nent; > uint32_t padding; > uint64_t reserved[4]; > v4v_ring_data_ent_t ring[0]; > } v4v_ring_data_t; > DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_t); > > I get a XEN_GUEST_HANDLE(v4v_ring_data_t) as argument of my hypercall and I > would like to access the ring data inside it which is a XEN_GUEST_HANDLE as well. > > Here is the code that I use for doing that (with explicte cast in > guest_handle_cast): > XEN_GUEST_HANDLE (v4v_ring_data_ent_t) ring_data_ent_hnd; > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > guest_handle_cast (ring_data_hnd, uint8_t); > guest_handle_add_offset (slop_hnd, sizeof (v4v_ring_data_t)); > ring_data_ent_hnd = > guest_handle_cast (slop_hnd, v4v_ring_data_ent_t); > ret = v4v_fill_ring_datas (d, ring_data.nent, ring_data_ent_hnd); So you really don't want to add anything (as not being type-safe), but instead want to get a guest handle representation for accessing the ring[0] member. That is, I'd introduce a guest_handle_for_field() for this purpose. Let me see whether I can put something together for you early next week. Jan ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 16:00 ` Jan Beulich @ 2012-06-14 21:19 ` Jean Guyader 0 siblings, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-14 21:19 UTC (permalink / raw) To: Jan Beulich; +Cc: Tim (Xen.org), xen-devel@lists.xen.org On 14/06 05:00, Jan Beulich wrote: > >>> On 14.06.12 at 17:39, Jean Guyader <jean.guyader@citrix.com> wrote: > > Here are the structs: > > > > typedef struct v4v_ring_data_ent > > { > > struct v4v_addr ring; > > uint16_t flags; > > uint16_t pad0; > > uint32_t space_required; > > uint32_t max_message_size; > > } v4v_ring_data_ent_t; > > DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_ent_t); > > > > typedef struct v4v_ring_data > > { > > uint64_t magic; > > uint32_t nent; > > uint32_t padding; > > uint64_t reserved[4]; > > v4v_ring_data_ent_t ring[0]; > > } v4v_ring_data_t; > > DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_t); > > > > I get a XEN_GUEST_HANDLE(v4v_ring_data_t) as argument of my hypercall and I > > would like to access the ring data inside it which is a XEN_GUEST_HANDLE as well. > > > > Here is the code that I use for doing that (with explicte cast in > > guest_handle_cast): > > XEN_GUEST_HANDLE (v4v_ring_data_ent_t) ring_data_ent_hnd; > > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > > guest_handle_cast (ring_data_hnd, uint8_t); > > guest_handle_add_offset (slop_hnd, sizeof (v4v_ring_data_t)); > > ring_data_ent_hnd = > > guest_handle_cast (slop_hnd, v4v_ring_data_ent_t); > > ret = v4v_fill_ring_datas (d, ring_data.nent, ring_data_ent_hnd); > > So you really don't want to add anything (as not being type-safe), > but instead want to get a guest handle representation for accessing > the ring[0] member. That is, I'd introduce a guest_handle_for_field() > for this purpose. Let me see whether I can put something together > for you early next week. > Thanks but I'll follow Tim's advice and get rid of the wrapper structure. I still think having some macro to get a handle from a field can be quite useful in general. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-14 15:39 ` Jean Guyader 2012-06-14 15:50 ` Tim Deegan 2012-06-14 16:00 ` Jan Beulich @ 2012-06-18 11:36 ` Jan Beulich 2012-06-18 12:50 ` Jean Guyader 2 siblings, 1 reply; 53+ messages in thread From: Jan Beulich @ 2012-06-18 11:36 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), xen-devel@lists.xen.org >>> On 14.06.12 at 17:39, Jean Guyader <jean.guyader@citrix.com> wrote: > Here are the structs: > > typedef struct v4v_ring_data_ent > > { > > struct v4v_addr ring; > > uint16_t flags; > > uint16_t pad0; > > uint32_t space_required; > > uint32_t max_message_size; > > } v4v_ring_data_ent_t; > > DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_ent_t); > > > > typedef struct v4v_ring_data > > { > > uint64_t magic; > > uint32_t nent; > > uint32_t padding; > > uint64_t reserved[4]; > > v4v_ring_data_ent_t ring[0]; > > } v4v_ring_data_t; > > DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_t); > > I get a XEN_GUEST_HANDLE(v4v_ring_data_t) as argument of my hypercall and I > would like to access the ring data inside it which is a XEN_GUEST_HANDLE as well. > > Here is the code that I use for doing that (with explicte cast in > guest_handle_cast): > XEN_GUEST_HANDLE (v4v_ring_data_ent_t) ring_data_ent_hnd; > XEN_GUEST_HANDLE (uint8_t) slop_hnd = > guest_handle_cast (ring_data_hnd, uint8_t); > guest_handle_add_offset (slop_hnd, sizeof (v4v_ring_data_t)); > ring_data_ent_hnd = > guest_handle_cast (slop_hnd, v4v_ring_data_ent_t); > ret = v4v_fill_ring_datas (d, ring_data.nent, ring_data_ent_hnd); Something as simple as #define guest_handle_for_field(hnd, type, fld) \ ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) works quite fine for me is an example like int v4v_test(struct domain *d, XEN_GUEST_HANDLE(v4v_ring_data_t) urp) { v4v_ring_data_t ring_data; XEN_GUEST_HANDLE(v4v_ring_data_ent_t) ring_data_ent_hnd; copy_from_guest(&ring_data, urp, 1); ring_data_ent_hnd = guest_handle_for_field(urp, v4v_ring_data_ent_t, ring[0]); return v4v_fill_ring_datas(d, ring_data.nent, ring_data_ent_hnd); } Jan ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [PATCH 4/5] xen: Enforce casting for guest_handle_cast 2012-06-18 11:36 ` Jan Beulich @ 2012-06-18 12:50 ` Jean Guyader 0 siblings, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-18 12:50 UTC (permalink / raw) To: Jan Beulich; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On 18 June 2012 12:36, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 14.06.12 at 17:39, Jean Guyader <jean.guyader@citrix.com> wrote: >> Here are the structs: >> >> typedef struct v4v_ring_data_ent >> >> { >> >> struct v4v_addr ring; >> >> uint16_t flags; >> >> uint16_t pad0; >> >> uint32_t space_required; >> >> uint32_t max_message_size; >> >> } v4v_ring_data_ent_t; >> >> DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_ent_t); >> >> >> >> typedef struct v4v_ring_data >> >> { >> >> uint64_t magic; >> >> uint32_t nent; >> >> uint32_t padding; >> >> uint64_t reserved[4]; >> >> v4v_ring_data_ent_t ring[0]; >> >> } v4v_ring_data_t; >> >> DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_t); >> >> I get a XEN_GUEST_HANDLE(v4v_ring_data_t) as argument of my hypercall and I >> would like to access the ring data inside it which is a XEN_GUEST_HANDLE as well. >> >> Here is the code that I use for doing that (with explicte cast in >> guest_handle_cast): >> XEN_GUEST_HANDLE (v4v_ring_data_ent_t) ring_data_ent_hnd; >> XEN_GUEST_HANDLE (uint8_t) slop_hnd = >> guest_handle_cast (ring_data_hnd, uint8_t); >> guest_handle_add_offset (slop_hnd, sizeof (v4v_ring_data_t)); >> ring_data_ent_hnd = >> guest_handle_cast (slop_hnd, v4v_ring_data_ent_t); >> ret = v4v_fill_ring_datas (d, ring_data.nent, ring_data_ent_hnd); > > Something as simple as > > #define guest_handle_for_field(hnd, type, fld) \ > ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) > > works quite fine for me is an example like > > int v4v_test(struct domain *d, XEN_GUEST_HANDLE(v4v_ring_data_t) urp) { > v4v_ring_data_t ring_data; > XEN_GUEST_HANDLE(v4v_ring_data_ent_t) ring_data_ent_hnd; > > copy_from_guest(&ring_data, urp, 1); > ring_data_ent_hnd = guest_handle_for_field(urp, v4v_ring_data_ent_t, ring[0]); > return v4v_fill_ring_datas(d, ring_data.nent, ring_data_ent_hnd); > } > Thanks! That works for me too. I'll replace the code in my patch series. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 5/5] xen: Add V4V implementation 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader ` (3 preceding siblings ...) 2012-05-31 15:07 ` [PATCH 4/5] xen: Enforce casting for guest_handle_cast Jean Guyader @ 2012-05-31 15:07 ` Jean Guyader 2012-05-31 15:59 ` Jan Beulich 2012-06-01 12:41 ` [RFC][PATCH 0/5] Add V4V to Xen Jan Beulich 2012-06-01 13:47 ` Ian Campbell 6 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-05-31 15:07 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 927 bytes --] Setup of v4v domains a domain gets created and cleanup when a domain die. Wire up the v4v hypercall. Include v4v internal and public headers. Signed-off-by: Jean Guyader <jean.guyader@citrix.com> --- xen/arch/x86/hvm/hvm.c | 9 +- xen/arch/x86/x86_32/entry.S | 2 + xen/arch/x86/x86_64/compat/entry.S | 2 + xen/arch/x86/x86_64/entry.S | 2 + xen/common/Makefile | 1 + xen/common/domain.c | 11 +- xen/common/v4v.c | 1779 ++++++++++++++++++++++++++++++++++++ xen/include/public/v4v.h | 544 +++++++++++ xen/include/public/xen.h | 2 +- xen/include/xen/sched.h | 5 + xen/include/xen/v4v.h | 109 +++ 11 files changed, 2461 insertions(+), 5 deletions(-) create mode 100644 xen/common/v4v.c create mode 100644 xen/include/public/v4v.h create mode 100644 xen/include/xen/v4v.h [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0005-xen-Add-V4V-implementation.patch --] [-- Type: text/x-patch; name="0005-xen-Add-V4V-implementation.patch", Size: 70641 bytes --] diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index e0d495d..6f2d70e 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3124,7 +3124,8 @@ static hvm_hypercall_t *hvm_hypercall32_table[NR_hypercalls] = { HYPERCALL(set_timer_op), HYPERCALL(hvm_op), HYPERCALL(sysctl), - HYPERCALL(tmem_op) + HYPERCALL(tmem_op), + HYPERCALL(v4v_op) }; #else /* defined(__x86_64__) */ @@ -3209,7 +3210,8 @@ static hvm_hypercall_t *hvm_hypercall64_table[NR_hypercalls] = { HYPERCALL(set_timer_op), HYPERCALL(hvm_op), HYPERCALL(sysctl), - HYPERCALL(tmem_op) + HYPERCALL(tmem_op), + HYPERCALL(v4v_op) }; #define COMPAT_CALL(x) \ @@ -3226,7 +3228,8 @@ static hvm_hypercall_t *hvm_hypercall32_table[NR_hypercalls] = { COMPAT_CALL(set_timer_op), HYPERCALL(hvm_op), HYPERCALL(sysctl), - HYPERCALL(tmem_op) + HYPERCALL(tmem_op), + HYPERCALL(v4v_op) }; #endif /* defined(__x86_64__) */ diff --git a/xen/arch/x86/x86_32/entry.S b/xen/arch/x86/x86_32/entry.S index 2982679..b3e0da4 100644 --- a/xen/arch/x86/x86_32/entry.S +++ b/xen/arch/x86/x86_32/entry.S @@ -700,6 +700,7 @@ ENTRY(hypercall_table) .long do_domctl .long do_kexec_op .long do_tmem_op + .long do_v4v_op .rept __HYPERVISOR_arch_0-((.-hypercall_table)/4) .long do_ni_hypercall .endr @@ -748,6 +749,7 @@ ENTRY(hypercall_args_table) .byte 1 /* do_domctl */ .byte 2 /* do_kexec_op */ .byte 1 /* do_tmem_op */ + .byte 6 /* do_v4v_op */ .rept __HYPERVISOR_arch_0-(.-hypercall_args_table) .byte 0 /* do_ni_hypercall */ .endr diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S index f49ff2d..28615f9 100644 --- a/xen/arch/x86/x86_64/compat/entry.S +++ b/xen/arch/x86/x86_64/compat/entry.S @@ -414,6 +414,7 @@ ENTRY(compat_hypercall_table) .quad do_domctl .quad compat_kexec_op .quad do_tmem_op + .quad do_v4v_op .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8) .quad compat_ni_hypercall .endr @@ -462,6 +463,7 @@ ENTRY(compat_hypercall_args_table) .byte 1 /* do_domctl */ .byte 2 /* compat_kexec_op */ .byte 1 /* do_tmem_op */ + .byte 6 /* do_v4v_op */ .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table) .byte 0 /* compat_ni_hypercall */ .endr diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index 3836260..918fa59 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -699,6 +699,7 @@ ENTRY(hypercall_table) .quad do_domctl .quad do_kexec_op .quad do_tmem_op + .quad do_v4v_op .rept __HYPERVISOR_arch_0-((.-hypercall_table)/8) .quad do_ni_hypercall .endr @@ -747,6 +748,7 @@ ENTRY(hypercall_args_table) .byte 1 /* do_domctl */ .byte 2 /* do_kexec */ .byte 1 /* do_tmem_op */ + .byte 6 /* do_v4v_op */ .rept __HYPERVISOR_arch_0-(.-hypercall_args_table) .byte 0 /* do_ni_hypercall */ .endr diff --git a/xen/common/Makefile b/xen/common/Makefile index 9eba8bc..fe3c72c 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -45,6 +45,7 @@ obj-y += tmem_xen.o obj-y += radix-tree.o obj-y += rbtree.o obj-y += lzo.o +obj-y += v4v.o obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo,$(n).init.o) diff --git a/xen/common/domain.c b/xen/common/domain.c index 8840202..9539d88 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -195,7 +195,8 @@ struct domain *domain_create( { struct domain *d, **pd; enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2, - INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 }; + INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5, + INIT_v4v = 1u<<6 }; int init_status = 0; int poolid = CPUPOOLID_NONE; @@ -219,6 +220,7 @@ struct domain *domain_create( spin_lock_init(&d->hypercall_deadlock_mutex); INIT_PAGE_LIST_HEAD(&d->page_list); INIT_PAGE_LIST_HEAD(&d->xenpage_list); + rwlock_init(&d->v4v_lock); spin_lock_init(&d->node_affinity_lock); d->node_affinity = NODE_MASK_ALL; @@ -274,6 +276,10 @@ struct domain *domain_create( goto fail; init_status |= INIT_gnttab; + if ( v4v_init(d) != 0 ) + goto fail; + init_status |= INIT_v4v; + poolid = 0; d->mem_event = xzalloc(struct mem_event_per_domain); @@ -313,6 +319,8 @@ struct domain *domain_create( xfree(d->mem_event); if ( init_status & INIT_arch ) arch_domain_destroy(d); + if ( init_status & INIT_v4v ) + v4v_destroy(d); if ( init_status & INIT_gnttab ) grant_table_destroy(d); if ( init_status & INIT_evtchn ) @@ -466,6 +474,7 @@ int domain_kill(struct domain *d) domain_pause(d); d->is_dying = DOMDYING_dying; spin_barrier(&d->domain_lock); + v4v_destroy(d); evtchn_destroy(d); gnttab_release_mappings(d); tmem_destroy(d->tmem); diff --git a/xen/common/v4v.c b/xen/common/v4v.c new file mode 100644 index 0000000..ed846ba --- /dev/null +++ b/xen/common/v4v.c @@ -0,0 +1,1779 @@ +/****************************************************************************** + * V4V + * + * Version 2 of v2v (Virtual-to-Virtual) + * + * Copyright (c) 2010, Citrix Systems + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include <xen/config.h> +#include <xen/mm.h> +#include <xen/compat.h> +#include <xen/init.h> +#include <xen/lib.h> +#include <xen/errno.h> +#include <xen/sched.h> +#include <xen/domain.h> +#include <xen/v4v.h> +#include <xen/event.h> +#include <xen/guest_access.h> +#include <asm/paging.h> +#include <asm/p2m.h> +#include <xen/keyhandler.h> + +#ifdef V4V_DEBUG +#define MY_FILE "v4v.c" +#define v4v_dprintk(format, args...) \ + do { \ + printk("%s:%d " format, \ + MY_FILE, __LINE__, ## args ); \ + } while ( 1 == 0 ) +#else +#define v4v_dprintk(format, ... ) (void)0 +#endif + + + +DEFINE_XEN_GUEST_HANDLE (uint8_t); +static struct v4v_ring_info *v4v_ring_find_info (struct domain *d, + struct v4v_ring_id *id); + +static struct v4v_ring_info *v4v_ring_find_info_by_addr (struct domain *d, + struct v4v_addr *a, + domid_t p); + +/* + * locks + */ + +/* + * locking is organized as follows: + * + * the global lock v4v_lock: L1 protects the v4v elements + * of all struct domain *d in the system, it does not + * protect any of the elements of d->v4v, just their + * addresses. By extension since the destruction of + * a domain with a non-NULL d->v4v will need to free + * the d->v4v pointer, holding this lock gauruntees + * that no domains pointers in which v4v is interested + * become invalid whilst this lock is held. + */ + +static DEFINE_RWLOCK (v4v_lock); /* L1 */ + +/* + * the lock d->v4v->lock: L2: Read on protects the hash table and + * the elements in the hash_table d->v4v->ring_hash, and + * the node and id fields in struct v4v_ring_info in the + * hash table. Write on L2 protects all of the elements of + * struct v4v_ring_info. To take L2 you must already have R(L1) + * W(L1) implies W(L2) and L3 + * + * the lock v4v_ring_info *ringinfo; ringinfo->lock: L3: + * protects len,tx_ptr the guest ring, the + * guest ring_data and the pending list. To take L3 you must + * already have R(L2). W(L2) implies L3 + */ + + +/* + * Debugs + */ + +#ifdef V4V_DEBUG +static void +v4v_hexdump (void *_p, int len) +{ + uint8_t *buf = (uint8_t *) _p; + int i, j; + + for (i = 0; i < len; i += 16) + { + printk (KERN_ERR "%p:", &buf[i]); + for (j = 0; j < 16; ++j) + { + int k = i + j; + if (k < len) + printk (" %02x", buf[k]); + else + printk (" "); + } + printk (" "); + + for (j = 0; j < 16; ++j) + { + int k = i + j; + if (k < len) + printk ("%c", ((buf[k] > 32) && (buf[k] < 127)) ? buf[k] : '.'); + else + printk (" "); + } + printk ("\n"); + } +} +#endif + + +/* + * Event channel + */ + +static void +v4v_signal_domain (struct domain *d) +{ + v4v_dprintk("send guest VIRQ_V4V domid:%d\n", d->domain_id); + send_guest_vcpu_virq (d->vcpu[0], VIRQ_V4V); +} + +static void +v4v_signal_domid (domid_t id) +{ + struct domain *d = get_domain_by_id (id); + if (!d) + return; + v4v_signal_domain (d); + put_domain (d); +} + + +/* + * ring buffer + */ + +/* called must have L3 */ +static void +v4v_ring_unmap (struct v4v_ring_info *ring_info) +{ + int i; + for (i = 0; i < ring_info->npage; ++i) + { + if (!ring_info->mfn_mapping[i]) + continue; + v4v_dprintk(""); + v4v_dprintk("unmapping page %p from %p\n", + (void*) mfn_x (ring_info->mfns[i]), + ring_info->mfn_mapping[i]); + + unmap_domain_page (ring_info->mfn_mapping[i]); + ring_info->mfn_mapping[i] = NULL; + } +} + +/* called must have L3 */ +static uint8_t * +v4v_ring_map_page (struct v4v_ring_info *ring_info, int i) +{ + if (i >= ring_info->npage) + return NULL; + if (ring_info->mfn_mapping[i]) + return ring_info->mfn_mapping[i]; + ring_info->mfn_mapping[i] = map_domain_page (mfn_x (ring_info->mfns[i])); + + v4v_dprintk("mapping page %p to %p\n", + (void *) mfn_x (ring_info->mfns[i]), + ring_info->mfn_mapping[i]); + return ring_info->mfn_mapping[i]; +} + +/* called must have L3 */ +static int +v4v_memcpy_from_guest_ring (void *_dst, struct v4v_ring_info *ring_info, + uint32_t offset, uint32_t len) +{ + int page = offset >> PAGE_SHIFT; + uint8_t *src; + uint8_t *dst = _dst; + + offset &= PAGE_SIZE - 1; + + while ((offset + len) > PAGE_SIZE) + { + src = v4v_ring_map_page (ring_info, page); + + if (!src) + { + return -EFAULT; + } + + v4v_dprintk("memcpy(%p,%p+%d,%d)\n", + dst, src, offset, + (int) (PAGE_SIZE - offset)); + memcpy (dst, src + offset, PAGE_SIZE - offset); + + page++; + len -= PAGE_SIZE - offset; + dst += PAGE_SIZE - offset; + offset = 0; + } + + src = v4v_ring_map_page (ring_info, page); + if (!src) + { + return -EFAULT; + } + + v4v_dprintk("memcpy(%p,%p+%d,%d)\n", dst, src, offset, len); + memcpy (dst, src + offset, len); + + return 0; +} + + +/* called must have L3 */ +static int +v4v_update_tx_ptr (struct v4v_ring_info *ring_info, uint32_t tx_ptr) +{ + uint8_t *dst = v4v_ring_map_page (ring_info, 0); + volatile uint32_t *p = (uint32_t *)(dst + offsetof (v4v_ring_t, tx_ptr)); + + if (!dst) + return -EFAULT; + *p = tx_ptr; + return 0; +} + +/* called must have L3 */ +static int +v4v_memcpy_to_guest_ring (struct v4v_ring_info *ring_info, uint32_t offset, + void *_src, uint32_t len) +{ + int page = offset >> PAGE_SHIFT; + uint8_t *dst; + uint8_t *src = _src; + + offset &= PAGE_SIZE - 1; + + while ((offset + len) > PAGE_SIZE) + { + dst = v4v_ring_map_page (ring_info, page); + + if (!dst) + { + v4v_dprintk("!dst\n"); + return -EFAULT; + } + +#ifdef V4V_DEBUG + v4v_dprintk("memcpy(%p+%d,%p,%d)\n", + dst, offset, src, + (int) (PAGE_SIZE - offset)); + v4v_hexdump (src, PAGE_SIZE - offset); + v4v_hexdump (dst + offset, PAGE_SIZE - offset); +#endif + memcpy (dst + offset, src, PAGE_SIZE - offset); + + page++; + len -= (PAGE_SIZE - offset); + src += (PAGE_SIZE - offset); + offset = 0; + } + + dst = v4v_ring_map_page (ring_info, page); + + if (!dst) + { + v4v_dprintk("attempted to map page %d of %d\n", page, ring_info->npage); + return -EFAULT; + } + +#ifdef V4V_DEBUG + v4v_dprintk("memcpy(%p+%d,%p,%d)\n", + dst, offset, src, len); + v4v_hexdump (src, len); + v4v_hexdump (dst + offset, len); +#endif + memcpy (dst + offset, src, len); + + return 0; +} + +/*called must have L3*/ +static int +v4v_memcpy_to_guest_ring_from_guest(struct v4v_ring_info *ring_info, + uint32_t offset, + XEN_GUEST_HANDLE (uint8_t) src_hnd, + uint32_t len) +{ + int page = offset >> PAGE_SHIFT; + uint8_t *dst; + + offset &= PAGE_SIZE - 1; + + while ( (offset + len) > PAGE_SIZE ) + { + dst = v4v_ring_map_page (ring_info, page); + + if ( !dst ) + { + v4v_dprintk("!dst\n"); + return -EFAULT; + } + + v4v_dprintk("copy_from_guest(%p+%d,%p,%d)\n", + dst, offset, (void *) src_hnd.p, + (int) (PAGE_SIZE - offset)); + if ( copy_from_guest ((dst + offset), src_hnd, PAGE_SIZE - offset) ) + { + v4v_dprintk("copy_from_guest failed\n"); + return -EFAULT; + } + + page++; + len -= PAGE_SIZE - offset; + guest_handle_add_offset (src_hnd, PAGE_SIZE - offset); + offset = 0; + } + + dst = v4v_ring_map_page (ring_info, page); + if (!dst) + { + v4v_dprintk("v4v_ring_map failed\n"); + return -EFAULT; + } + + v4v_dprintk("copy_from_guest(%p+%d,%p,%d)\n", + dst, offset, (void *) src_hnd.p, len); + if ( copy_from_guest ((dst + offset), src_hnd, len) ) + { + v4v_dprintk("copy_from_guest failed\n"); + return -EFAULT; + } + + return 0; +} + +static int +v4v_ringbuf_get_rx_ptr (struct domain *d, struct v4v_ring_info *ring_info, + uint32_t * rx_ptr) +{ + v4v_ring_t *ringp; + + if ( ring_info->npage == 0 ) + return -1; + + ringp = map_domain_page (mfn_x (ring_info->mfns[0])); + + v4v_dprintk("v4v_ringbuf_payload_space: mapped %p to %p\n", + (void *) mfn_x (ring_info->mfns[0]), ringp); + if ( !ringp ) + return -1; + + *rx_ptr = *(volatile uint32_t *) &ringp->rx_ptr; + + unmap_domain_page (mfn_x (ring_info->mfns[0])); + return 0; +} + +uint32_t +v4v_ringbuf_payload_space (struct domain * d, struct v4v_ring_info * ring_info) +{ + v4v_ring_t ring; + int32_t ret; + + ring.tx_ptr = ring_info->tx_ptr; + ring.len = ring_info->len; + + if ( v4v_ringbuf_get_rx_ptr (d, ring_info, &ring.rx_ptr) ) + return 0; + + v4v_dprintk("v4v_ringbuf_payload_space:tx_ptr=%d rx_ptr=%d\n", + (int) ring.tx_ptr, (int) ring.rx_ptr); + if ( ring.rx_ptr == ring.tx_ptr ) + return ring.len - sizeof (struct v4v_ring_message_header); + + ret = ring.rx_ptr - ring.tx_ptr; + if ( ret < 0 ) + ret += ring.len; + + ret -= sizeof (struct v4v_ring_message_header); + ret -= V4V_ROUNDUP (1); + + return (ret < 0) ? 0 : ret; +} + +/*caller must have L3*/ +static size_t +v4v_ringbuf_insert (struct domain *d, + struct v4v_ring_info *ring_info, + struct v4v_ring_id *src_id, uint32_t proto, + XEN_GUEST_HANDLE (void) buf_hnd_void, uint32_t len) +{ + XEN_GUEST_HANDLE (uint8_t) buf_hnd = + guest_handle_cast (buf_hnd_void, uint8_t); + v4v_ring_t ring; + struct v4v_ring_message_header mh = { 0 }; + int32_t sp; + int32_t happy_ret = len; + int32_t ret = 0; + + if ( (V4V_ROUNDUP (len) + sizeof (struct v4v_ring_message_header)) >= + ring_info->len ) + { + v4v_dprintk("EMSGSIZE\n"); + return -EMSGSIZE; + } + + do + { + if ( (ret = v4v_memcpy_from_guest_ring (&ring, ring_info, + 0, sizeof (ring))) ) + break; + + ring.tx_ptr = ring_info->tx_ptr; + ring.len = ring_info->len; + + v4v_dprintk("ring.tx_ptr=%d ring.rx_ptr=%d ring.len=%d ring_info->tx_ptr=%d\n", + ring.tx_ptr, ring.rx_ptr, ring.len, ring_info->tx_ptr); + + if ( ring.rx_ptr == ring.tx_ptr ) + sp = ring_info->len; + else + { + sp = ring.rx_ptr - ring.tx_ptr; + if (sp < 0) + sp += ring.len; + } + + if ( (V4V_ROUNDUP (len) + sizeof (struct v4v_ring_message_header)) >= sp ) + { + v4v_dprintk("EAGAIN\n"); + ret = -EAGAIN; + break; + } + + mh.len = len + sizeof (struct v4v_ring_message_header); + mh.source = src_id->addr; + mh.pad = 0; + mh.protocol = proto; + + if ( (ret = v4v_memcpy_to_guest_ring (ring_info, + ring.tx_ptr + sizeof (v4v_ring_t), + &mh, sizeof (mh))) ) + break; + + ring.tx_ptr += sizeof (mh); + if ( ring.tx_ptr == ring_info->len ) + ring.tx_ptr = 0; + + sp = ring.len - ring.tx_ptr; + + if ( len > sp ) + { + if ((ret = v4v_memcpy_to_guest_ring_from_guest (ring_info, + ring.tx_ptr + sizeof (v4v_ring_t), + buf_hnd, sp))) + break; + + ring.tx_ptr = 0; + len -= sp; + guest_handle_add_offset (buf_hnd, sp); + } + + if ( (ret = v4v_memcpy_to_guest_ring_from_guest (ring_info, + ring.tx_ptr + sizeof (v4v_ring_t), + buf_hnd, len)) ) + break; + + ring.tx_ptr += V4V_ROUNDUP (len); + + if ( ring.tx_ptr == ring_info->len ) + ring.tx_ptr = 0; + + mb (); + ring_info->tx_ptr = ring.tx_ptr; + + if ( (ret = v4v_update_tx_ptr(ring_info, ring.tx_ptr)) ) + break; + + } + while ( 0 ); + + v4v_ring_unmap (ring_info); + + return ret ? ret : happy_ret; + +} + +static ssize_t +v4v_iov_count (XEN_GUEST_HANDLE (v4v_iov_t) iovs, int niov) +{ + v4v_iov_t iov; + size_t ret = 0; + + while ( niov-- ) + { + if ( copy_from_guest (&iov, iovs, 1) ) + return -EFAULT; + + ret += iov.iov_len; + guest_handle_add_offset (iovs, 1); + } + + return ret; +} + +/*caller must have L3*/ +static ssize_t +v4v_ringbuf_insertv (struct domain *d, + struct v4v_ring_info *ring_info, + struct v4v_ring_id *src_id, uint32_t proto, + XEN_GUEST_HANDLE (v4v_iov_t) iovs, uint32_t niov, + uint32_t len) +{ + v4v_ring_t ring; + struct v4v_ring_message_header mh = { 0 }; + int32_t sp; + int32_t happy_ret; + int32_t ret = 0; + + happy_ret = len; + + if ( (V4V_ROUNDUP (len) + sizeof (struct v4v_ring_message_header) ) >= + ring_info->len) + return -EMSGSIZE; + + do + { + if ( (ret = v4v_memcpy_from_guest_ring (&ring, ring_info, 0, + sizeof (ring))) ) + break; + + ring.tx_ptr = ring_info->tx_ptr; + ring.len = ring_info->len; + + v4v_dprintk("ring.tx_ptr=%d ring.rx_ptr=%d ring.len=%d ring_info->tx_ptr=%d\n", + ring.tx_ptr, ring.rx_ptr, ring.len, ring_info->tx_ptr); + + if ( ring.rx_ptr == ring.tx_ptr ) + sp = ring_info->len; + else + { + sp = ring.rx_ptr - ring.tx_ptr; + if (sp < 0) + sp += ring.len; + } + + if ( (V4V_ROUNDUP (len) + sizeof (struct v4v_ring_message_header) ) >= sp) + { + v4v_dprintk("EAGAIN\n"); + ret = -EAGAIN; + break; + } + + mh.len = len + sizeof (struct v4v_ring_message_header); + mh.source = src_id->addr; + mh.pad = 0; + mh.protocol = proto; + + if ( (ret = v4v_memcpy_to_guest_ring (ring_info, + ring.tx_ptr + sizeof (v4v_ring_t), + &mh, sizeof (mh))) ) + break; + + ring.tx_ptr += sizeof (mh); + if ( ring.tx_ptr == ring_info->len ) + ring.tx_ptr = 0; + + while ( niov-- ) + { + XEN_GUEST_HANDLE (uint8_t) buf_hnd; + v4v_iov_t iov; + + if ( copy_from_guest (&iov, iovs, 1) ) + { + ret = -EFAULT; + break; + } + + buf_hnd.p = (uint8_t *) iov.iov_base; //FIXME + len = iov.iov_len; + + if ( unlikely (!guest_handle_okay (buf_hnd, len)) ) + { + ret = -EFAULT; + break; + } + + sp = ring.len - ring.tx_ptr; + + if ( len > sp ) + { + if ( (ret = v4v_memcpy_to_guest_ring_from_guest (ring_info, + ring.tx_ptr + + sizeof (v4v_ring_t), + buf_hnd, sp)) ) + break; + + ring.tx_ptr = 0; + len -= sp; + guest_handle_add_offset (buf_hnd, sp); + } + + if ( (ret = v4v_memcpy_to_guest_ring_from_guest (ring_info, + ring.tx_ptr + + sizeof (v4v_ring_t), + buf_hnd, len)) ) + break; + + ring.tx_ptr += len; + + if (ring.tx_ptr == ring_info->len) + ring.tx_ptr = 0; + + guest_handle_add_offset (iovs, 1); + } + if ( ret ) + break; + + ring.tx_ptr = V4V_ROUNDUP (ring.tx_ptr); + + if ( ring.tx_ptr >= ring_info->len ) + ring.tx_ptr -= ring_info->len; + + mb (); + ring_info->tx_ptr = ring.tx_ptr; + if ( (ret = v4v_update_tx_ptr(ring_info, ring.tx_ptr)) ) + break; + } + while ( 0 ); + + v4v_ring_unmap (ring_info); + + return ret ? ret : happy_ret; +} + + + +/* pending */ +static void +v4v_pending_remove_ent (struct v4v_pending_ent *ent) +{ + hlist_del (&ent->node); + xfree (ent); +} + +/*caller must have L3 */ +static void +v4v_pending_remove_all (struct v4v_ring_info *info) +{ + struct hlist_node *node, *next; + struct v4v_pending_ent *pending_ent; + + hlist_for_each_entry_safe (pending_ent, node, next, &info->pending, + node) v4v_pending_remove_ent (pending_ent); +} + +/*Caller must hold L1 */ +static void +v4v_pending_notify (struct domain *caller_d, struct hlist_head *to_notify) +{ + struct hlist_node *node, *next; + struct v4v_pending_ent *pending_ent; + + hlist_for_each_entry_safe (pending_ent, node, next, to_notify, node) + { + hlist_del (&pending_ent->node); + v4v_signal_domid (pending_ent->id); + xfree (pending_ent); + } + +} + +/*caller must have R(L2) */ +static void +v4v_pending_find (struct v4v_ring_info *ring_info, uint32_t payload_space, + struct hlist_head *to_notify) +{ + struct hlist_node *node, *next; + struct v4v_pending_ent *ent; + + spin_lock (&ring_info->lock); + hlist_for_each_entry_safe (ent, node, next, &ring_info->pending, node) + { + if (payload_space >= ent->len) + { + hlist_del (&ent->node); + hlist_add_head (&ent->node, to_notify); + } + } + spin_unlock (&ring_info->lock); +} + +/*caller must have L3 */ +static int +v4v_pending_queue (struct v4v_ring_info *ring_info, domid_t src_id, int len) +{ + struct v4v_pending_ent *ent = xmalloc (struct v4v_pending_ent); + + if ( !ent ) + { + v4v_dprintk("ENOMEM\n"); + return -ENOMEM; + } + + ent->len = len; + ent->id = src_id; + + hlist_add_head (&ent->node, &ring_info->pending); + + return 0; +} + +/* L3 */ +static int +v4v_pending_requeue (struct v4v_ring_info *ring_info, domid_t src_id, int len) +{ + struct hlist_node *node; + struct v4v_pending_ent *ent; + + hlist_for_each_entry (ent, node, &ring_info->pending, node) + { + if ( ent->id == src_id ) + { + if ( ent->len < len ) + ent->len = len; + return 0; + } + } + + return v4v_pending_queue (ring_info, src_id, len); +} + + +/* L3 */ +static void +v4v_pending_cancel (struct v4v_ring_info *ring_info, domid_t src_id) +{ + struct hlist_node *node, *next; + struct v4v_pending_ent *ent; + + hlist_for_each_entry_safe (ent, node, next, &ring_info->pending, node) + { + if ( ent->id == src_id) + { + hlist_del (&ent->node); + xfree (ent); + } + } +} + +/* + * ring data + */ + +/*Caller should hold R(L1)*/ +static int +v4v_fill_ring_data (struct domain *src_d, + XEN_GUEST_HANDLE (v4v_ring_data_ent_t) data_ent_hnd) +{ + v4v_ring_data_ent_t ent; + struct domain *dst_d; + struct v4v_ring_info *ring_info; + + if ( copy_from_guest (&ent, data_ent_hnd, 1) ) + { + v4v_dprintk("EFAULT\n"); + return -EFAULT; + } + + v4v_dprintk("v4v_fill_ring_data: ent.ring.domain=%d,ent.ring.port=%u\n", + (int) ent.ring.domain, (int) ent.ring.port); + + ent.flags = 0; + + dst_d = get_domain_by_id (ent.ring.domain); + + if ( dst_d && dst_d->v4v ) + { + read_lock (&dst_d->v4v->lock); + ring_info = v4v_ring_find_info_by_addr (dst_d, &ent.ring, + src_d->domain_id); + + if ( ring_info ) + { + uint32_t space_avail; + + ent.flags |= V4V_RING_DATA_F_EXISTS; + ent.max_message_size = + ring_info->len - sizeof (struct v4v_ring_message_header) - + V4V_ROUNDUP (1); + spin_lock (&ring_info->lock); + + space_avail = v4v_ringbuf_payload_space (dst_d, ring_info); + + if ( space_avail >= ent.space_required ) + { + v4v_pending_cancel (ring_info, src_d->domain_id); + ent.flags |= V4V_RING_DATA_F_SUFFICIENT; + } + else + { + v4v_pending_requeue (ring_info, src_d->domain_id, + ent.space_required); + ent.flags |= V4V_RING_DATA_F_PENDING; + } + + spin_unlock (&ring_info->lock); + + if ( space_avail == ent.max_message_size ) + ent.flags |= V4V_RING_DATA_F_EMPTY; + + } + read_unlock (&dst_d->v4v->lock); + } + + if ( dst_d ) + put_domain (dst_d); + + if ( copy_field_to_guest (data_ent_hnd, &ent, flags) ) + { + v4v_dprintk("EFAULT\n"); + return -EFAULT; + } + return 0; +} + +/*Called should hold no more than R(L1) */ +static int +v4v_fill_ring_datas (struct domain *d, int nent, + XEN_GUEST_HANDLE (v4v_ring_data_ent_t) data_ent_hnd) +{ + int ret = 0; + + read_lock (&v4v_lock); + while ( !ret && nent-- ) + { + ret = v4v_fill_ring_data (d, data_ent_hnd); + guest_handle_add_offset (data_ent_hnd, 1); + } + read_unlock (&v4v_lock); + return ret; +} + +/* + * ring + */ +static int +v4v_find_ring_mfns (struct domain *d, struct v4v_ring_info *ring_info, + XEN_GUEST_HANDLE (v4v_pfn_list_t) pfn_list_hnd) +{ + XEN_GUEST_HANDLE (v4v_pfn_t) pfn_hnd; + v4v_pfn_list_t pfn_list; + int i,j; + mfn_t *mfns; + uint8_t **mfn_mapping; + unsigned long mfn; + struct page_info *page; + int ret = 0; + + if ( copy_from_guest (&pfn_list, pfn_list_hnd, 1) ) + { + v4v_dprintk("EFAULT\n"); + return -EFAULT; + } + + if (pfn_list.magic != V4V_PFN_LIST_MAGIC) + { + v4v_dprintk("EINVAL\n"); + return -EINVAL; + } + + if ((pfn_list.npage << PAGE_SHIFT) < ring_info->len) + { + v4v_dprintk("EINVAL\n"); + return -EINVAL; + } + + { + XEN_GUEST_HANDLE (uint8_t) slop_hnd = + guest_handle_cast (pfn_list_hnd, uint8_t); + guest_handle_add_offset (slop_hnd, sizeof (v4v_pfn_list_t)); + pfn_hnd = guest_handle_cast (slop_hnd, v4v_pfn_t); + } + + mfns = xmalloc_array (mfn_t, pfn_list.npage); + if ( !mfns ) + { + v4v_dprintk("ENOMEM\n"); + return -ENOMEM; + } + + mfn_mapping = xmalloc_array (uint8_t *, pfn_list.npage); + if ( !mfn_mapping ) + { + xfree (mfns); + return -ENOMEM; + } + + for (i = 0; i < pfn_list.npage; ++i) + { + unsigned long pfn; + p2m_type_t p2mt; + + if ( copy_from_guest_offset (&pfn, pfn_hnd, i, 1) ) + { + ret = -EFAULT; + v4v_dprintk("EFAULT\n"); + break; + } + + mfn = mfn_x(get_gfn(d, pfn, &p2mt)); + if ( !mfn_valid(mfn) ) + { + printk(KERN_ERR "v4v domain %d passed invalid mfn %"PRI_mfn" ring %p seq %d\n", + d->domain_id, mfn, ring_info, i); + ret = -EINVAL; + break; + } + page = mfn_to_page(mfn); + if ( !get_page_and_type(page, d, PGT_writable_page) ) + { + printk(KERN_ERR "v4v domain %d passed wrong type mfn %"PRI_mfn" ring %p seq %d\n", + d->domain_id, mfn, ring_info, i); + ret = -EINVAL; + break; + } + mfns[i] = _mfn(mfn); + v4v_dprintk("v4v_find_ring_mfns: %d: %lx -> %lx\n", + i, (unsigned long) pfn, (unsigned long) mfn_x (mfns[i])); + mfn_mapping[i] = NULL; + put_gfn(d, pfn); + } + + if ( !ret ) + { + ring_info->npage = pfn_list.npage; + ring_info->mfns = mfns; + ring_info->mfn_mapping = mfn_mapping; + } + else + { + j = i; + for ( i = 0; i < j; ++i ) + if ( mfn_x(mfns[i]) != 0 ) + put_page_and_type(mfn_to_page(mfn_x(mfns[i]))); + xfree (mfn_mapping); + xfree (mfns); + v4v_dprintk(""); + } + return ret; +} + + +/* caller must hold R(L2) */ +static struct v4v_ring_info * +v4v_ring_find_info (struct domain *d, struct v4v_ring_id *id) +{ + uint16_t hash; + struct hlist_node *node; + struct v4v_ring_info *ring_info; + + hash = v4v_hash_fn (id); + + v4v_dprintk("ring_find_info: d->v4v=%p, d->v4v->ring_hash[%d]=%p id=%p\n", + d->v4v, (int) hash, d->v4v->ring_hash[hash].first, id); + v4v_dprintk("ring_find_info: id.addr.port=%d id.addr.domain=%d id.addr.partner=%d\n", + id->addr.port, id->addr.domain, id->partner); + + hlist_for_each_entry (ring_info, node, &d->v4v->ring_hash[hash], node) + { + if ( !memcmp (id, &ring_info->id, sizeof (*id)) ) + { + v4v_dprintk("ring_find_info: ring_info=%p\n", ring_info); + return ring_info; + } + } + v4v_dprintk("ring_find_info: no ring_info found\n"); + return NULL; +} + +/* caller must hold R(L2) */ +static struct v4v_ring_info * +v4v_ring_find_info_by_addr (struct domain *d, struct v4v_addr *a, domid_t p) +{ + struct v4v_ring_id id; + struct v4v_ring_info *ret; + + if ( !a ) + return NULL; + + id.addr.port = a->port; + id.addr.domain = d->domain_id; + id.partner = p; + + ret = v4v_ring_find_info (d, &id); + if ( ret ) + return ret; + + id.partner = V4V_DOMID_NONE; + + return v4v_ring_find_info (d, &id); +} + +/*caller must hold W(L2) */ +static void v4v_ring_remove_mfns (struct v4v_ring_info *ring_info) +{ + int i; + + if ( ring_info->mfns ) + { + for ( i=0; i < ring_info->npage; ++i ) + if (mfn_x(ring_info->mfns[i]) != 0) + put_page_and_type(mfn_to_page(mfn_x(ring_info->mfns[i]))); + xfree (ring_info->mfns); + } + ring_info->mfns = NULL; +} + +/*caller must hold W(L2) */ +static void +v4v_ring_remove_info (struct v4v_ring_info *ring_info) +{ + v4v_pending_remove_all (ring_info); + + hlist_del (&ring_info->node); + v4v_ring_remove_mfns(ring_info); + xfree (ring_info); +} + +/* Call from guest to unpublish a ring */ +static long +v4v_ring_remove (struct domain *d, XEN_GUEST_HANDLE (v4v_ring_t) ring_hnd) +{ + struct v4v_ring ring; + struct v4v_ring_info *ring_info; + int ret = 0; + + read_lock (&v4v_lock); + + do + { + if ( !d->v4v ) + { + v4v_dprintk("EINVAL\n"); + ret = -EINVAL; + break; + } + + if ( copy_from_guest (&ring, ring_hnd, 1) ) + { + v4v_dprintk("EFAULT\n"); + ret = -EFAULT; + break; + } + + if ( ring.magic != V4V_RING_MAGIC ) + { + v4v_dprintk("ring.magic(%lx) != V4V_RING_MAGIC(%lx), EINVAL\n", + ring.magic, V4V_RING_MAGIC); + ret = -EINVAL; + break; + } + + ring.id.addr.domain = d->domain_id; + + write_lock (&d->v4v->lock); + ring_info = v4v_ring_find_info (d, &ring.id); + + if ( ring_info ) + v4v_ring_remove_info (ring_info); + + write_unlock (&d->v4v->lock); + + if ( !ring_info ) + { + v4v_dprintk( "ENOENT\n" ); + ret = -ENOENT; + break; + } + + } + while ( 0 ); + + read_unlock (&v4v_lock); + return ret; +} + +/* call from guest to publish a ring */ +static long +v4v_ring_add (struct domain *d, XEN_GUEST_HANDLE (v4v_ring_t) ring_hnd, + XEN_GUEST_HANDLE (v4v_pfn_list_t) pfn_list_hnd) +{ + struct v4v_ring ring; + struct v4v_ring_info *ring_info; + int need_to_insert = 0; + int ret = 0; + + if ( (long) ring_hnd.p & (PAGE_SIZE - 1) ) + { + v4v_dprintk("EINVAL\n"); + return -EINVAL; + } + + read_lock (&v4v_lock); + do + { + if ( !d->v4v ) + { + v4v_dprintk(" !d->v4v, EINVAL\n"); + ret = -EINVAL; + break; + } + + if ( copy_from_guest (&ring, ring_hnd, 1) ) + { + v4v_dprintk(" copy_from_guest failed, EFAULT\n"); + ret = -EFAULT; + break; + } + + if ( ring.magic != V4V_RING_MAGIC ) + { + v4v_dprintk("ring.magic(%lx) != V4V_RING_MAGIC(%lx), EINVAL\n", + ring.magic, V4V_RING_MAGIC); + ret = -EINVAL; + break; + } + + if ( (ring.len < + (sizeof (struct v4v_ring_message_header) + V4V_ROUNDUP (1) + + V4V_ROUNDUP (1))) || (V4V_ROUNDUP (ring.len) != ring.len) ) + { + v4v_dprintk("EINVAL\n"); + ret = -EINVAL; + break; + } + + ring.id.addr.domain = d->domain_id; + if ( copy_field_to_guest (ring_hnd, &ring, id) ) + { + v4v_dprintk("EFAULT\n"); + ret = -EFAULT; + break; + } + + /* + * no need for a lock yet, because only we know about this + * set the tx pointer if it looks bogus (we don't reset it + * because this might be a re-register after S4) + */ + if ( (ring.tx_ptr >= ring.len) + || (V4V_ROUNDUP (ring.tx_ptr) != ring.tx_ptr) ) + { + ring.tx_ptr = ring.rx_ptr; + } + copy_field_to_guest (ring_hnd, &ring, tx_ptr); + + read_lock (&d->v4v->lock); + ring_info = v4v_ring_find_info (d, &ring.id); + + if ( !ring_info ) + { + read_unlock (&d->v4v->lock); + ring_info = xmalloc (struct v4v_ring_info); + if ( !ring_info ) + { + v4v_dprintk("ENOMEM\n"); + ret = -ENOMEM; + break; + } + need_to_insert++; + spin_lock_init (&ring_info->lock); + INIT_HLIST_HEAD (&ring_info->pending); + ring_info->mfns = NULL; + + } + else + { + /* + * Ring info already existed. If mfn list was already + * populated remove the MFN's from list and then add + * the new list. + */ + printk(KERN_INFO "v4v: dom%d re-registering existing ring, clearing MFN list\n", + current->domain->domain_id); + v4v_ring_remove_mfns(ring_info); + } + + spin_lock (&ring_info->lock); + ring_info->id = ring.id; + ring_info->len = ring.len; + ring_info->tx_ptr = ring.tx_ptr; + ring_info->ring = ring_hnd; + if ( ring_info->mfns ) + xfree (ring_info->mfns); + ret = v4v_find_ring_mfns (d, ring_info, pfn_list_hnd); + spin_unlock (&ring_info->lock); + if ( ret ) + break; + + if ( !need_to_insert ) + { + read_unlock (&d->v4v->lock); + } + else + { + uint16_t hash = v4v_hash_fn (&ring.id); + write_lock (&d->v4v->lock); + hlist_add_head (&ring_info->node, &d->v4v->ring_hash[hash]); + write_unlock (&d->v4v->lock); + } + } + while ( 0 ); + + read_unlock (&v4v_lock); + return ret; +} + + +/* + * io + */ + +/*Caller must hold v4v_lock and hash_lock*/ +static void +v4v_notify_ring (struct domain *d, struct v4v_ring_info *ring_info, + struct hlist_head *to_notify) +{ + uint32_t space; + + spin_lock (&ring_info->lock); + space = v4v_ringbuf_payload_space (d, ring_info); + spin_unlock (&ring_info->lock); + + v4v_pending_find (ring_info, space, to_notify); +} + +/*notify hypercall*/ +static long +v4v_notify (struct domain *d, + XEN_GUEST_HANDLE (v4v_ring_data_t) ring_data_hnd) +{ + v4v_ring_data_t ring_data; + HLIST_HEAD (to_notify); + int i; + int ret = 0; + + read_lock (&v4v_lock); + + if ( !d->v4v ) + { + read_unlock (&v4v_lock); + v4v_dprintk("!d->v4v, ENODEV\n"); + return -ENODEV; + } + + read_lock (&d->v4v->lock); + for ( i = 0; i < V4V_HTABLE_SIZE; ++i ) + { + struct hlist_node *node, *next; + struct v4v_ring_info *ring_info; + + hlist_for_each_entry_safe (ring_info, node, + next, &d->v4v->ring_hash[i], + node) + { + v4v_notify_ring (d, ring_info, &to_notify); + } + } + read_unlock (&d->v4v->lock); + + if ( !hlist_empty (&to_notify) ) + { + v4v_pending_notify (d, &to_notify); + } + + do + { + if ( !guest_handle_is_null (ring_data_hnd) ) + { + /* Quick sanity check on ring_data_hnd */ + if ( copy_field_from_guest (&ring_data, ring_data_hnd, magic) ) + { + v4v_dprintk("copy_field_from_guest failed\n"); + ret = -EFAULT; + break; + } + + if ( ring_data.magic != V4V_RING_DATA_MAGIC ) + { + v4v_dprintk("ring.magic(%lx) != V4V_RING_MAGIC(%lx), EINVAL\n", + ring_data.magic, V4V_RING_MAGIC); + ret = -EINVAL; + break; + } + + if (copy_from_guest (&ring_data, ring_data_hnd, 1)) + { + v4v_dprintk("copy_from_guest failed\n"); + ret = -EFAULT; + break; + } + + { + XEN_GUEST_HANDLE (v4v_ring_data_ent_t) ring_data_ent_hnd; + XEN_GUEST_HANDLE (uint8_t) slop_hnd = + guest_handle_cast (ring_data_hnd, uint8_t); + guest_handle_add_offset (slop_hnd, sizeof (v4v_ring_data_t)); + ring_data_ent_hnd = + guest_handle_cast (slop_hnd, v4v_ring_data_ent_t); + ret = v4v_fill_ring_datas (d, ring_data.nent, ring_data_ent_hnd); + } + } + } + while ( 0 ); + + read_unlock (&v4v_lock); + + return ret; +} + + + +/*Hypercall to do the send*/ +static size_t +v4v_send (struct domain *src_d, v4v_addr_t * src_addr, + v4v_addr_t * dst_addr, uint32_t proto, + XEN_GUEST_HANDLE (void) buf, size_t len) +{ + struct domain *dst_d; + struct v4v_ring_id src_id; + struct v4v_ring_info *ring_info; + int ret = 0; + + if ( !dst_addr ) + { + v4v_dprintk("!dst_addr\n"); + return -EINVAL; + } + + read_lock (&v4v_lock); + if ( !src_d->v4v ) + { + read_unlock (&v4v_lock); + v4v_dprintk("!src_d->v4v\n"); + return -EINVAL; + } + + src_id.addr.port = src_addr->port; + src_id.addr.domain = src_d->domain_id; + src_id.partner = dst_addr->domain; + + dst_d = get_domain_by_id (dst_addr->domain); + if ( !dst_d ) + { + read_unlock (&v4v_lock); + v4v_dprintk("!dst_d, ECONNREFUSED\n"); + return -ECONNREFUSED; + } + + do + { + if ( !dst_d->v4v ) + { + ret = -ECONNREFUSED; + v4v_dprintk("!dst_d->v4v, ECONNREFUSED\n"); + break; + } + + read_lock (&dst_d->v4v->lock); + ring_info = + v4v_ring_find_info_by_addr (dst_d, dst_addr, src_addr->domain); + + if ( !ring_info ) + { + ret = -ECONNREFUSED; + v4v_dprintk("!ring_info\n"); + } + else + { + spin_lock (&ring_info->lock); + ret = + v4v_ringbuf_insert (dst_d, ring_info, &src_id, proto, buf, len); + if ( ret == -EAGAIN ) + { + v4v_dprintk("ret == EAGAIN\n"); + /* Schedule a wake up on the event channel when space is there */ + if (v4v_pending_requeue (ring_info, src_d->domain_id, len)) + { + v4v_dprintk("v4v_pending_requeue failed, ENOMEM\n"); + ret = -ENOMEM; + } + } + spin_unlock (&ring_info->lock); + + if (ret >= 0) + { + v4v_signal_domain (dst_d); + } + } + read_unlock (&dst_d->v4v->lock); + } + while ( 0 ); + + put_domain (dst_d); + read_unlock (&v4v_lock); + return ret; +} + +/*Hypercall to do the send*/ +static size_t +v4v_sendv (struct domain *src_d, v4v_addr_t * src_addr, + v4v_addr_t * dst_addr, uint32_t proto, + XEN_GUEST_HANDLE (v4v_iov_t) iovs, size_t niov) +{ + struct domain *dst_d; + struct v4v_ring_id src_id; + struct v4v_ring_info *ring_info; + int ret = 0; + + if ( !dst_addr ) + { + v4v_dprintk("!dst_addr, EINVAL\n"); + return -EINVAL; + } + + read_lock (&v4v_lock); + if (!src_d->v4v) + { + read_unlock (&v4v_lock); + v4v_dprintk("!src_d->v4v, EINVAL\n"); + return -EINVAL; + } + + src_id.addr.port = src_addr->port; + src_id.addr.domain = src_d->domain_id; + src_id.partner = dst_addr->domain; + + dst_d = get_domain_by_id (dst_addr->domain); + if (!dst_d) + { + read_unlock (&v4v_lock); + v4v_dprintk("!dst_d, ECONNREFUSED\n"); + return -ECONNREFUSED; + } + + do + { + if ( !dst_d->v4v ) + { + v4v_dprintk("dst_d->v4v, ECONNREFUSED\n"); + ret = -ECONNREFUSED; + break; + } + + read_lock (&dst_d->v4v->lock); + ring_info = + v4v_ring_find_info_by_addr (dst_d, dst_addr, src_addr->domain); + + if ( !ring_info ) + { + ret = -ECONNREFUSED; + v4v_dprintk(" !ring_info, ECONNREFUSED\n"); + } + else + { + uint32_t len = v4v_iov_count (iovs, niov); + + if ( len < 0 ) + { + ret = len; + break; + } + + spin_lock (&ring_info->lock); + ret = + v4v_ringbuf_insertv (dst_d, ring_info, &src_id, proto, iovs, + niov, len); + if ( ret == -EAGAIN ) + { + v4v_dprintk("v4v_ringbuf_insertv failed, EAGAIN\n"); + /* Schedule a wake up on the event channel when space is there */ + if (v4v_pending_requeue (ring_info, src_d->domain_id, len)) + { + v4v_dprintk("v4v_pending_requeue failed, ENOMEM\n"); + ret = -ENOMEM; + } + } + spin_unlock (&ring_info->lock); + + if ( ret >= 0 ) + { + v4v_signal_domain (dst_d); + } + + } + read_unlock (&dst_d->v4v->lock); + + } + while ( 0 ); + + put_domain (dst_d); + read_unlock (&v4v_lock); + return ret; +} + +/* + * hypercall glue + */ +long +do_v4v_op (int cmd, XEN_GUEST_HANDLE (void) arg1, + XEN_GUEST_HANDLE (void) arg2, + XEN_GUEST_HANDLE (void) arg3, uint32_t arg4, uint32_t arg5) +{ + struct domain *d = current->domain; + long rc = -EFAULT; + + v4v_dprintk("->do_v4v_op(%d,%p,%p,%p,%d,%d)\n", cmd, + (void *) arg1.p, (void *) arg2.p, (void *) arg3.p, + (int) arg4, (int) arg5); + + domain_lock (d); + switch (cmd) + { + case V4VOP_register_ring: + { + XEN_GUEST_HANDLE (v4v_ring_t) ring_hnd = + guest_handle_cast (arg1, v4v_ring_t); + XEN_GUEST_HANDLE (v4v_pfn_list_t) pfn_list_hnd = + guest_handle_cast (arg2, v4v_pfn_list_t); + if ( unlikely (!guest_handle_okay (ring_hnd, 1)) ) + goto out; + if ( unlikely (!guest_handle_okay (pfn_list_hnd, 1)) ) //FIXME + goto out; + rc = v4v_ring_add (d, ring_hnd, pfn_list_hnd); + break; + } + case V4VOP_unregister_ring: + { + XEN_GUEST_HANDLE (v4v_ring_t) ring_hnd = + guest_handle_cast (arg1, v4v_ring_t); + if ( unlikely (!guest_handle_okay (ring_hnd, 1)) ) + goto out; + rc = v4v_ring_remove (d, ring_hnd); + break; + } + case V4VOP_send: + { + v4v_addr_t src, dst; + uint32_t len = arg4; + uint32_t protocol = arg5; + XEN_GUEST_HANDLE (v4v_addr_t) src_hnd = + guest_handle_cast (arg1, v4v_addr_t); + XEN_GUEST_HANDLE (v4v_addr_t) dst_hnd = + guest_handle_cast (arg2, v4v_addr_t); + + if ( unlikely (!guest_handle_okay (src_hnd, 1)) ) + goto out; + if ( copy_from_guest (&src, src_hnd, 1) ) + goto out; + + if ( unlikely (!guest_handle_okay (dst_hnd, 1)) ) + goto out; + if ( copy_from_guest (&dst, dst_hnd, 1) ) + goto out; + + rc = v4v_send (d, &src, &dst, protocol, arg3, len); + break; + } + case V4VOP_sendv: + { + v4v_addr_t src, dst; + uint32_t niov = arg4; + uint32_t protocol = arg5; + XEN_GUEST_HANDLE (v4v_addr_t) src_hnd = + guest_handle_cast (arg1, v4v_addr_t); + XEN_GUEST_HANDLE (v4v_addr_t) dst_hnd = + guest_handle_cast (arg2, v4v_addr_t); + XEN_GUEST_HANDLE (v4v_iov_t) iovs = + guest_handle_cast (arg3, v4v_iov_t); + + if ( unlikely (!guest_handle_okay (src_hnd, 1)) ) + goto out; + if ( copy_from_guest (&src, src_hnd, 1) ) + goto out; + + if ( unlikely (!guest_handle_okay (dst_hnd, 1)) ) + goto out; + if ( copy_from_guest (&dst, dst_hnd, 1) ) + goto out; + + if ( unlikely (!guest_handle_okay (iovs, niov)) ) + goto out; + + rc = v4v_sendv (d, &src, &dst, protocol, iovs, niov); + break; + } + case V4VOP_notify: + { + XEN_GUEST_HANDLE (v4v_ring_data_t) ring_data_hnd = + guest_handle_cast (arg1, v4v_ring_data_t); + rc = v4v_notify (d, ring_data_hnd); + break; + } + default: + rc = -ENOSYS; + break; + } +out: + domain_unlock (d); + v4v_dprintk("<-do_v4v_op()=%d\n", (int) rc); + return rc; +} + +/* + * init + */ + +void +v4v_destroy (struct domain *d) +{ + int i; + + BUG_ON (!d->is_dying); + write_lock (&v4v_lock); + + v4v_dprintk("d->v=%p\n", d->v4v); + + if ( d->v4v ) + { + for ( i = 0; i < V4V_HTABLE_SIZE; ++i ) + { + struct hlist_node *node, *next; + struct v4v_ring_info *ring_info; + + hlist_for_each_entry_safe (ring_info, node, + next, &d->v4v->ring_hash[i], + node) + { + v4v_ring_remove_info (ring_info); + } + } + } + + d->v4v = NULL; + write_unlock (&v4v_lock); +} + +int +v4v_init (struct domain *d) +{ + struct v4v_domain *v4v; + int i; + + v4v = xmalloc (struct v4v_domain); + if ( !v4v ) + return -ENOMEM; + + rwlock_init (&v4v->lock); + + for ( i = 0; i < V4V_HTABLE_SIZE; ++i ) + { + INIT_HLIST_HEAD (&v4v->ring_hash[i]); + } + + write_lock (&v4v_lock); + d->v4v = v4v; + write_unlock (&v4v_lock); + + return 0; +} + + +/* + * debug + */ + +static void +dump_domain_ring (struct domain *d, struct v4v_ring_info *ring_info) +{ + uint32_t rx_ptr; + + printk (KERN_ERR " ring: domid=%d port=0x%08x partner=%d npage=%d\n", + (int) d->domain_id, (int) ring_info->id.addr.port, + (int) ring_info->id.partner, (int) ring_info->npage); + + if ( v4v_ringbuf_get_rx_ptr (d, ring_info, &rx_ptr) ) + { + printk (KERN_ERR " Failed to read rx_ptr\n"); + return; + } + + printk (KERN_ERR " tx_ptr=%d rx_ptr=%d len=%d\n", + (int) ring_info->tx_ptr, (int) rx_ptr, (int) ring_info->len); +} + +static void +dump_domain_rings (struct domain *d) +{ + int i; + + printk (KERN_ERR " domain %d:\n", (int) d->domain_id); + + read_lock (&d->v4v->lock); + + for ( i = 0; i < V4V_HTABLE_SIZE; ++i ) + { + struct hlist_node *node; + struct v4v_ring_info *ring_info; + + hlist_for_each_entry (ring_info, node, &d->v4v->ring_hash[i], node) + dump_domain_ring (d, ring_info); + } + read_unlock (&d->v4v->lock); + + printk (KERN_ERR "\n"); + v4v_signal_domain (d); +} + +static void +dump_rings (unsigned char key) +{ + struct domain *d; + + printk (KERN_ERR "\n\nV4V ring dump:\n"); + read_lock (&v4v_lock); + + rcu_read_lock (&domlist_read_lock); + + for_each_domain (d) dump_domain_rings (d); + + rcu_read_unlock (&domlist_read_lock); + + read_unlock (&v4v_lock); +} + +struct keyhandler v4v_info_keyhandler = { + .diagnostic = 1, + .u.fn = dump_rings, + .desc = "dump v4v ring states and intterupt" +}; + +static int __init +setup_dump_rings (void) +{ + register_keyhandler ('4', &v4v_info_keyhandler); + return 0; +} + +__initcall (setup_dump_rings); + + +/* + * Local variables: + * mode: C + * c-set-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/public/v4v.h b/xen/include/public/v4v.h new file mode 100644 index 0000000..8cb02a8 --- /dev/null +++ b/xen/include/public/v4v.h @@ -0,0 +1,544 @@ +/****************************************************************************** + * V4V + * + * Version 2 of v2v (Virtual-to-Virtual) + * + * Copyright (c) 2010, Citrix Systems + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __XEN_PUBLIC_V4V_H__ +#define __XEN_PUBLIC_V4V_H__ + +#include "xen.h" + +/* + * Compiler specific hacks + */ +#if !defined(__GNUC__) +# define V4V_PACKED +# define V4V_UNSUED +# define V4V_INLINE __inline +#else /* __GNUC__ */ +# define V4V_PACKED __attribute__ ((packed)) +# define V4V_UNUSED __attribute__ ((unused)) +# ifdef __XEN__ +# define V4V_INLINE +# define V4V_VOLATILE +# else +# define V4V_VOLATILE volatile +# ifndef __STRICT_ANSI__ +# define V4V_INLINE inline +# else +# define V4V_INLINE +# endif +# endif +#endif /* __GNUC__ */ + +#if !defined(__GNUC__) +# pragma pack(push, 1) +# pragma warning(push) +# pragma warning(disable: 4200) +#endif + +/* + * Structure definitions + */ + +#define V4V_PROTO_DGRAM 0x3c2c1db8 +#define V4V_PROTO_STREAM 0x70f6a8e5 + +#ifdef __i386__ +# define V4V_RING_MAGIC 0xdf6977f231abd910ULL +# define V4V_PFN_LIST_MAGIC 0x91dd6159045b302dULL +#else +# define V4V_RING_MAGIC 0xdf6977f231abd910 +# define V4V_PFN_LIST_MAGIC 0x91dd6159045b302d +#endif +#define V4V_DOMID_INVALID (0x7FFFU) +#define V4V_DOMID_NONE V4V_DOMID_INVALID +#define V4V_DOMID_ANY V4V_DOMID_INVALID +#define V4V_PORT_NONE 0 + +typedef struct v4v_iov +{ + uint64_t iov_base; + uint64_t iov_len; +} V4V_PACKED v4v_iov_t; +DEFINE_XEN_GUEST_HANDLE (v4v_iov_t); + +typedef struct v4v_addr +{ + uint32_t port; + domid_t domain; +} V4V_PACKED v4v_addr_t; + +DEFINE_XEN_GUEST_HANDLE (v4v_addr_t); + +typedef struct v4v_ring_id +{ + struct v4v_addr addr; + domid_t partner; +} V4V_PACKED v4v_ring_id_t; + + +typedef uint64_t v4v_pfn_t; +DEFINE_XEN_GUEST_HANDLE (v4v_pfn_t); + +typedef struct v4v_pfn_list_t +{ + uint64_t magic; + uint32_t npage; + uint32_t pad; + uint64_t reserved[3]; + v4v_pfn_t pages[0]; +} V4V_PACKED v4v_pfn_list_t; + +DEFINE_XEN_GUEST_HANDLE (v4v_pfn_list_t); + + +typedef struct v4v_ring +{ + /* v4v magic number */ + uint64_t magic; + /* + * id + * + * xen only looks at this during register/unregister + * and will fill in id.addr.domain + */ + struct v4v_ring_id id; + /* length of ring[], must be a multiple of 8 */ + uint32_t len; + /* rx pointer, modified by domain */ + V4V_VOLATILE uint32_t rx_ptr; + /* tx pointer, modified by xen */ + V4V_VOLATILE uint32_t tx_ptr; + /* padding */ + uint64_t reserved[4]; + /* ring */ + V4V_VOLATILE uint8_t ring[0]; +} V4V_PACKED v4v_ring_t; +DEFINE_XEN_GUEST_HANDLE (v4v_ring_t); + +#ifdef __i386__ +#define V4V_RING_DATA_MAGIC 0x4ce4d30fbc82e92aULL +#else +#define V4V_RING_DATA_MAGIC 0x4ce4d30fbc82e92a +#endif + +#define V4V_RING_DATA_F_EMPTY 1U << 0 /* Ring is empty */ +#define V4V_RING_DATA_F_EXISTS 1U << 1 /* Ring exists */ +#define V4V_RING_DATA_F_PENDING 1U << 2 /* Pending interrupt exists - do not + rely on this field - for + profiling only */ +#define V4V_RING_DATA_F_SUFFICIENT 1U << 3 /* Sufficient space to queue + space_required bytes exists */ + +typedef struct v4v_ring_data_ent +{ + struct v4v_addr ring; + uint16_t flags; + uint32_t space_required; + uint32_t max_message_size; +} V4V_PACKED v4v_ring_data_ent_t; +DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_ent_t); + +typedef struct v4v_ring_data +{ + uint64_t magic; + uint32_t nent; + uint32_t pad; + uint64_t reserved[4]; + struct v4v_ring_data_ent data[0]; +} V4V_PACKED v4v_ring_data_t; +DEFINE_XEN_GUEST_HANDLE (v4v_ring_data_t); + + +#define V4V_ROUNDUP(a) (((a) +0xf ) & ~0xf) +/* Messages on the ring are padded to 128 bits + * Len here refers to the exact length of the data not including the + * 128 bit header. The message uses + * ((len +0xf) & ~0xf) + sizeof(v4v_ring_message_header) bytes + */ + +struct v4v_stream_header +{ + uint32_t flags; + uint32_t conid; +} V4V_PACKED; + +struct v4v_ring_message_header +{ + uint32_t len; + struct v4v_addr source; + uint16_t pad; + uint32_t protocol; + uint8_t data[0]; +} V4V_PACKED; + + +/* + * HYPERCALLS + */ + +#define V4VOP_register_ring 1 +/* + * Registers a ring with Xen, if a ring with the same v4v_ring_id exists, + * this ring takes its place, registration will not change tx_ptr + * unless it is invalid + * + * do_v4v_op(V4VOP_unregister_ring, + * XEN_GUEST_HANDLE(v4v_ring_t), + * XEN_GUEST_HANDLE(v4v_pfn_list_t), + * NULL, 0, 0) + */ + + +#define V4VOP_unregister_ring 2 +/* + * Unregister a ring. + * + * v4v_hypercall(V4VOP_send, XEN_GUEST_HANDLE(v4v_ring_t), NULL, NULL, 0, 0) + */ + +#define V4VOP_send 3 +/* + * Sends len bytes of buf to dst, giving src as the source address (xen will + * ignore src->domain and put your domain in the actually message), xen + * first looks for a ring with id.addr==dst and id.partner==sending_domain + * if that fails it looks for id.addr==dst and id.partner==DOMID_ANY. + * protocol is the 32 bit protocol number used from the message + * most likely V4V_PROTO_DGRAM or STREAM. If insufficient space exists + * it will return -EAGAIN and xen will twing the V4V_INTERRUPT when + * sufficient space becomes available + * + * v4v_hypercall(V4VOP_send, + * XEN_GUEST_HANDLE(v4v_addr_t) src, + * XEN_GUEST_HANDLE(v4v_addr_t) dst, + * XEN_GUEST_HANDLE(void) buf, + * uint32_t len, + * uint32_t protocol) + */ + + +#define V4VOP_notify 4 +/* Asks xen for information about other rings in the system + * + * v4v_ring_data_t contains an array of v4v_ring_data_ent_t + * + * ent->ring is the v4v_addr_t of the ring you want information on + * the same matching rules are used as for V4VOP_send. + * + * ent->space_required if this field is not null xen will check + * that there is space in the destination ring for this many bytes + * of payload. If there is it will set the V4V_RING_DATA_F_SUFFICIENT + * and CANCEL any pending interrupt for that ent->ring, if insufficient + * space is available it will schedule an interrupt and the flag will + * not be set. + * + * The flags are set by xen when notify replies + * V4V_RING_DATA_F_EMPTY ring is empty + * V4V_RING_DATA_F_PENDING interrupt is pending - don't rely on this + * V4V_RING_DATA_F_SUFFICIENT sufficient space for space_required is there + * V4V_RING_DATA_F_EXISTS ring exists + * + * v4v_hypercall(V4VOP_notify, + * XEN_GUEST_HANDLE(v4v_ring_data_t) buf, + * NULL, NULL, 0, 0) + */ + + +#define V4VOP_sendv 5 +/* + * Identical to V4VOP_send except rather than buf and len it takes + * an array of v4v_iov_t and a length of the array. + * + * v4v_hypercall(V4VOP_sendv, + * XEN_GUEST_HANDLE(v4v_addr_t) src, + * XEN_GUEST_HANDLE(v4v_addr_t) dst, + * XEN_GUEST_HANDLE(v4v_iov_t), + * UINT32_t niov, + * uint32_t protocol) + */ + +#if !defined(__GNUC__) +# pragma warning(pop) +# pragma pack(pop) +#endif + +#if !defined(__GNUC__) +static __inline void +mb (void) +{ + _mm_mfence (); + _ReadWriteBarrier (); +} +#else +#ifndef mb +# define mb() __asm__ __volatile__ ("" ::: "memory"); +#endif +#endif + +#if !defined(V4V_EXCLUDE_UTILS) + +/*************** Utility functions **************/ + +static V4V_INLINE uint32_t +v4v_ring_bytes_to_read (volatile struct v4v_ring *r) +{ + int32_t ret; + ret = r->tx_ptr - r->rx_ptr; + if (ret >= 0) + return ret; + return (uint32_t) (r->len + ret); +} + + +/* + * Copy at most t bytes of the next message in the ring, into the buffer + * at _buf, setting from and protocol if they are not NULL, returns + * the actual length of the message, or -1 if there is nothing to read + */ +V4V_UNUSED static V4V_INLINE ssize_t +v4v_copy_out (struct v4v_ring *r, struct v4v_addr *from, uint32_t * protocol, + void *_buf, size_t t, int consume) +{ + volatile struct v4v_ring_message_header *mh; + /* unnecessary cast from void * required by MSVC compiler */ + uint8_t *buf = (uint8_t *) _buf; + uint32_t btr = v4v_ring_bytes_to_read (r); + uint32_t rxp = r->rx_ptr; + uint32_t bte; + uint32_t len; + ssize_t ret; + + + if (btr < sizeof (*mh)) + return -1; + + /* + * Becuase the message_header is 128 bits long and the ring is 128 bit + * aligned, we're gaurunteed never to wrap + */ + mh = (volatile struct v4v_ring_message_header *) &r->ring[r->rx_ptr]; + + len = mh->len; + if (btr < len) + return -1; + +#if defined(__GNUC__) + if (from) + *from = mh->source; +#else + /* MSVC can't do the above */ + if (from) + memcpy((void *) from, (void *) &(mh->source), sizeof(struct v4v_addr)); +#endif + + if (protocol) + *protocol = mh->protocol; + + rxp += sizeof (*mh); + if (rxp == r->len) + rxp = 0; + len -= sizeof (*mh); + ret = len; + + bte = r->len - rxp; + + if (bte < len) + { + if (t < bte) + { + if (buf) + { + memcpy (buf, (void *) &r->ring[rxp], t); + buf += t; + } + + rxp = 0; + len -= bte; + t = 0; + } + else + { + if (buf) + { + memcpy (buf, (void *) &r->ring[rxp], bte); + buf += bte; + } + rxp = 0; + len -= bte; + t -= bte; + } + } + + if (buf && t) + memcpy (buf, (void *) &r->ring[rxp], (t < len) ? t : len); + + + rxp += V4V_ROUNDUP (len); + if (rxp == r->len) + rxp = 0; + + mb (); + + if (consume) + r->rx_ptr = rxp; + + return ret; +} + +static V4V_INLINE void +v4v_memcpy_skip (void *_dst, const void *_src, size_t len, size_t *skip) +{ + const uint8_t *src = (const uint8_t *) _src; + uint8_t *dst = (uint8_t *) _dst; + + if (!*skip) + { + memcpy (dst, src, len); + return; + } + + if (*skip >= len) + { + *skip -= len; + return; + } + + src += *skip; + dst += *skip; + len -= *skip; + *skip = 0; + + memcpy (dst, src, len); +} + +/* + * Copy at most t bytes of the next message in the ring, into the buffer + * at _buf, skipping skip bytes, setting from and protocol if they are not + * NULL, returns the actual length of the message, or -1 if there is + * nothing to read + */ +static ssize_t +v4v_copy_out_offset (struct v4v_ring *r, struct v4v_addr *from, + uint32_t * protocol, void *_buf, size_t t, int consume, + size_t skip) V4V_UNUSED; + +V4V_INLINE static ssize_t +v4v_copy_out_offset (struct v4v_ring *r, struct v4v_addr *from, + uint32_t * protocol, void *_buf, size_t t, int consume, + size_t skip) +{ + volatile struct v4v_ring_message_header *mh; + /* unnecessary cast from void * required by MSVC compiler */ + uint8_t *buf = (uint8_t *) _buf; + uint32_t btr = v4v_ring_bytes_to_read (r); + uint32_t rxp = r->rx_ptr; + uint32_t bte; + uint32_t len; + ssize_t ret; + + buf -= skip; + + if (btr < sizeof (*mh)) + return -1; + + /* + * Becuase the message_header is 128 bits long and the ring is 128 bit + * aligned, we're gaurunteed never to wrap + */ + mh = (volatile struct v4v_ring_message_header *) &r->ring[r->rx_ptr]; + + len = mh->len; + if (btr < len) + return -1; + +#if defined(__GNUC__) + if (from) + *from = mh->source; +#else + /* MSVC can't do the above */ + if (from) + memcpy((void *) from, (void *) &(mh->source), sizeof(struct v4v_addr)); +#endif + + if (protocol) + *protocol = mh->protocol; + + rxp += sizeof (*mh); + if (rxp == r->len) + rxp = 0; + len -= sizeof (*mh); + ret = len; + + bte = r->len - rxp; + + if (bte < len) + { + if (t < bte) + { + if (buf) + { + v4v_memcpy_skip (buf, (void *) &r->ring[rxp], t, &skip); + buf += t; + } + + rxp = 0; + len -= bte; + t = 0; + } + else + { + if (buf) + { + v4v_memcpy_skip (buf, (void *) &r->ring[rxp], bte, + &skip); + buf += bte; + } + rxp = 0; + len -= bte; + t -= bte; + } + } + + if (buf && t) + v4v_memcpy_skip (buf, (void *) &r->ring[rxp], (t < len) ? t : len, + &skip); + + + rxp += V4V_ROUNDUP (len); + if (rxp == r->len) + rxp = 0; + + mb (); + + if (consume) + r->rx_ptr = rxp; + + return ret; +} + +#endif /* V4V_EXCLUDE_UTILS */ + +#endif /* __XEN_PUBLIC_V4V_H__ */ + +/* + * Local variables: + * mode: C + * c-set-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 033cbba..dce0338 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -99,7 +99,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); #define __HYPERVISOR_domctl 36 #define __HYPERVISOR_kexec_op 37 #define __HYPERVISOR_tmem_op 38 -#define __HYPERVISOR_xc_reserved_op 39 /* reserved for XenClient */ +#define __HYPERVISOR_v4v_op 39 /* Architecture-specific hypercall definitions. */ #define __HYPERVISOR_arch_0 48 diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 53804c8..457e3f2 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -23,6 +23,7 @@ #include <public/sysctl.h> #include <public/vcpu.h> #include <public/mem_event.h> +#include <xen/v4v.h> #ifdef CONFIG_COMPAT #include <compat/vcpu.h> @@ -350,6 +351,10 @@ struct domain nodemask_t node_affinity; unsigned int last_alloc_node; spinlock_t node_affinity_lock; + + /* v4v */ + rwlock_t v4v_lock; + struct v4v_domain *v4v; }; struct domain_setup_info diff --git a/xen/include/xen/v4v.h b/xen/include/xen/v4v.h new file mode 100644 index 0000000..4325a03 --- /dev/null +++ b/xen/include/xen/v4v.h @@ -0,0 +1,109 @@ +/****************************************************************************** + * V4V + * + * Version 2 of v2v (Virtual-to-Virtual) + * + * Copyright (c) 2010, Citrix Systems + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __V4V_PRIVATE_H__ +#define __V4V_PRIVATE_H__ + +#include <xen/config.h> +#include <xen/types.h> +#include <xen/spinlock.h> +#include <xen/smp.h> +#include <xen/shared.h> +#include <xen/list.h> +#include <public/v4v.h> + + +#define V4V_HTABLE_SIZE 32 + + +static inline uint16_t +v4v_hash_fn (struct v4v_ring_id *id) +{ + uint16_t ret; + ret = (uint16_t) (id->addr.port >> 16); + ret ^= (uint16_t) id->addr.port; + ret ^= id->addr.domain; + ret ^= id->partner; + + ret &= (V4V_HTABLE_SIZE-1); + + return ret; +} + +struct v4v_pending_ent +{ + struct hlist_node node; + domid_t id; + uint32_t len; +}; + + +struct v4v_ring_info +{ + /* next node in the hash, protected by L2 */ + struct hlist_node node; + /* this ring's id, protected by L2 */ + struct v4v_ring_id id; + /* L3 */ + spinlock_t lock; + /* cached length of the ring (from ring->len), protected by L3 */ + uint32_t len; + uint32_t npage; + /* cached tx pointer location, protected by L3 */ + uint32_t tx_ptr; + /* guest ring, protected by L3 */ + XEN_GUEST_HANDLE(v4v_ring_t) ring; + /* mapped ring pages protected by L3*/ + uint8_t **mfn_mapping; + /* list of mfns of guest ring */ + mfn_t *mfns; + /* list of struct v4v_pending_ent for this ring, L3 */ + struct hlist_head pending; +}; + +/* + * The value of the v4v element in a struct domain is + * protected by the global lock L1 + */ +struct v4v_domain +{ + /* L2 */ + rwlock_t lock; + /* protected by L2 */ + struct hlist_head ring_hash[V4V_HTABLE_SIZE]; +}; + +void v4v_destroy(struct domain *d); +int v4v_init(struct domain *d); +long do_v4v_op (int cmd, + XEN_GUEST_HANDLE (void) arg1, + XEN_GUEST_HANDLE (void) arg2, + XEN_GUEST_HANDLE (void) arg3, + uint32_t arg4, + uint32_t arg5); + +#endif /* __V4V_PRIVATE_H__ */ + +/* + * Local variables: + * mode: C + * c-set-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [PATCH 5/5] xen: Add V4V implementation 2012-05-31 15:07 ` [PATCH 5/5] xen: Add V4V implementation Jean Guyader @ 2012-05-31 15:59 ` Jan Beulich 0 siblings, 0 replies; 53+ messages in thread From: Jan Beulich @ 2012-05-31 15:59 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: > Setup of v4v domains a domain gets created and cleanup > when a domain die. Wire up the v4v hypercall. > > Include v4v internal and public headers. Iirc there was discussion about 6-argument hypercalls already, and it was suggested to convert the argument set to a structure instead. Am I mis-remembering, or is there any new reason why introducing these is necessary? Further, the public header doesn't seem to meet basic requirements. For example, who told you that each and every compiler other than gcc understands #pragma warning(...)? Nor is adding mb() again an option (see io/ring.h), not to speak of adding an MSVC specific implementation (this should be a requirement on the consumers of the header). Nor any other inline functions - they just don't belong here imo (if you absolutely need those, they should be in a conditional that by default prevents them from being seen by the compiler - with that, some of the other hackery you did in the earlier patches also becomes unnecessary). Structure packing should be achieved using explicit padding fields rather than using compiler- dependent constructs. Jan ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader ` (4 preceding siblings ...) 2012-05-31 15:07 ` [PATCH 5/5] xen: Add V4V implementation Jean Guyader @ 2012-06-01 12:41 ` Jan Beulich 2012-06-01 13:24 ` George Dunlap 2012-06-01 13:47 ` Ian Campbell 6 siblings, 1 reply; 53+ messages in thread From: Jan Beulich @ 2012-06-01 12:41 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel >>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: Is there a particular reason this series got resent with no apparent change (and also none mentioned in the description here or in the individual patches)? All of the comments sent yesterday still apply, and I continue to consider patches 1-4 as unnecessary/broken. Jan ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-01 12:41 ` [RFC][PATCH 0/5] Add V4V to Xen Jan Beulich @ 2012-06-01 13:24 ` George Dunlap 2012-06-14 14:01 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: George Dunlap @ 2012-06-01 13:24 UTC (permalink / raw) To: Jan Beulich; +Cc: Jean Guyader, xen-devel On Fri, Jun 1, 2012 at 1:41 PM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: > > Is there a particular reason this series got resent with no apparent > change (and also none mentioned in the description here or in the > individual patches)? All of the comments sent yesterday still apply, > and I continue to consider patches 1-4 as unnecessary/broken. The date on the second set of e-mails is 15 minutes before the original series. I presume this means that Jean sent the series but it got held up for some reason, so sent it again 15 minutes later; and the second series made it through but the first was held up until today. Never attribute to malice that which can be attributed to computer error. :-) -George ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-01 13:24 ` George Dunlap @ 2012-06-14 14:01 ` Jean Guyader 0 siblings, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-14 14:01 UTC (permalink / raw) To: George Dunlap; +Cc: Jan Beulich, xen-devel@lists.xen.org On 01/06 02:24, George Dunlap wrote: > On Fri, Jun 1, 2012 at 1:41 PM, Jan Beulich <JBeulich@suse.com> wrote: > >>>> On 31.05.12 at 17:07, Jean Guyader <jean.guyader@citrix.com> wrote: > > > > Is there a particular reason this series got resent with no apparent > > change (and also none mentioned in the description here or in the > > individual patches)? All of the comments sent yesterday still apply, > > and I continue to consider patches 1-4 as unnecessary/broken. > > The date on the second set of e-mails is 15 minutes before the > original series. I presume this means that Jean sent the series but > it got held up for some reason, so sent it again 15 minutes later; and > the second series made it through but the first was held up until > today. > > Never attribute to malice that which can be attributed to computer error. :-) > George is absolutely spot on, this is exactly what happen. Sorry about that :( Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader ` (5 preceding siblings ...) 2012-06-01 12:41 ` [RFC][PATCH 0/5] Add V4V to Xen Jan Beulich @ 2012-06-01 13:47 ` Ian Campbell 2012-06-07 8:47 ` Jean Guyader 2012-06-07 9:42 ` Jean Guyader 6 siblings, 2 replies; 53+ messages in thread From: Ian Campbell @ 2012-06-01 13:47 UTC (permalink / raw) To: Jean Guyader; +Cc: xen-devel@lists.xen.org On Thu, 2012-05-31 at 16:07 +0100, Jean Guyader wrote: > V4V is a copy based inter vm communication system. > > Please have a look at this thread for more detail > about V4V. > http://lists.xen.org/archives/html/xen-devel/2012-05/msg01866.html > > This patch series is work in progress but I wanted to > post it early enough so I can get feedback from people. The main thing which is missing here, which makes it rather hard to review, is any kind of design documentation. It would also be great to see the rationale for why things have to be done this way. For example it seems that there is a bunch of stuff being added to the hypervisor which could live in the context of some guest or service domain -- putting stuff like that in the hypervisor needs strong arguments and rationale why it has to be there rather than somewhere else. Likewise perhaps the v4v hypercall could effectively be implemented with a multicall containing some grant copy ops and evtchn manipulations? But in the absence of any descriptions of the whats, hows and whys of v4v its rather hard to have these sorts of conversations. The above are some concrete examples but I'm more interested in seeing the more general descriptions before we get into those. Ian. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-01 13:47 ` Ian Campbell @ 2012-06-07 8:47 ` Jean Guyader 2012-06-07 9:42 ` Jean Guyader 1 sibling, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-07 8:47 UTC (permalink / raw) To: Ian Campbell; +Cc: Jean Guyader, xen-devel@lists.xen.org On 01/06 02:47, Ian Campbell wrote: > On Thu, 2012-05-31 at 16:07 +0100, Jean Guyader wrote: > > V4V is a copy based inter vm communication system. > > > > Please have a look at this thread for more detail > > about V4V. > > http://lists.xen.org/archives/html/xen-devel/2012-05/msg01866.html > > > > This patch series is work in progress but I wanted to > > post it early enough so I can get feedback from people. > > The main thing which is missing here, which makes it rather hard to > review, is any kind of design documentation. It would also be great to > see the rationale for why things have to be done this way. > > For example it seems that there is a bunch of stuff being added to the > hypervisor which could live in the context of some guest or service > domain -- putting stuff like that in the hypervisor needs strong > arguments and rationale why it has to be there rather than somewhere > else. > > Likewise perhaps the v4v hypercall could effectively be implemented with > a multicall containing some grant copy ops and evtchn manipulations? > > But in the absence of any descriptions of the whats, hows and whys of > v4v its rather hard to have these sorts of conversations. The above are > some concrete examples but I'm more interested in seeing the more > general descriptions before we get into those. > Here is some documentation of the Xen V4V API and the ring structure. Let me know if you have other questions. * Overview ** Architecture v4v has a very simple architecture. The receiving domain registers a ring with xen. The sending domain requests that xen inserts data into the receiving domain's ring. Notification that there is new data to read is delivered by a VIRQ, and a domain can request an interrupt be generated when sufficient space is available in another domain's ring. The code that inserts the data into the ring is in xen, and xen writes a header describing the data and where it came from infront of the data. As both the ring manipulation and the header are written by the hypervisor, the receiving domain can trust their content and the format of the ring. ** Addressing v4v_addr_t identifies an endpoint address. typedef struct v4v_addr { uint32_t port; domid_t domain; } v4v_addr_t; struct v4v_ring_id uniquely identifies a ring on the platform. struct v4v_ring_id { struct v4v_addr addr; domid_t partner; }; When a send operation is performed, xen will attempt to find a ring registered by destination domain, with the required port, and with a partner of the sending domain. Failing that it will then search for a ring with the correct port and destination domain, but with partner set to V4V_DOMID_ANY. ** Setup A domain first generates a ring, and a structure describing the pages in the ring. Rings must be page aligned, and contiguous in virtual memory, but need not be contiguous in physical(pfn) or machine(mfn) memory. A ring is uniquely identified on the platform by its struct v4v_ring_id. A ring is contained in a v4v_ring_t typedef struct v4v_ring { uint64_t magic; /* * Identifies ring_id - xen only looks at this during register/unregister * and will fill in id.addr.domain */ struct v4v_ring_id id; /* length of ring[], must be a multiple of 16 */ uint32_t len; /* rx_ptr - modified by domain */ uint32_t rx_ptr; /* tx_ptr - modified by xen */ volatile uint32_t tx_ptr; uint64_t reserved[4]; volatile uint8_t ring[0]; } v4v_ring_t; To register the ring, xen also needs a list of pfn that the ring occupies. This is provided in a v4v_pfn_list_t typedef struct v4v_pfn_list_t { uint64_t magic; uint32_t npage; uint32_t pad; uint64_t reserved[3]; v4v_pfn_t pages[0]; } v4v_pfn_list_t; Registering the ring is done with a hypercall int hypercall(NR_V4V,V4VOP_register_ring,v4v_ring_t *ring,v4v_pfn_list_t *pfn_list); On registering, xen will check that the tx_ptr and rx_ptr values are valid (aligned and within the size of the ring) and modify them if they are not, it will also update id.addr.domain with the calling domain's domid and take an internal copy of all the relevant information. After registration the only field that xen will read is ring->rx_ptr, xen will write to ring->ring[] and ring->tx_ptr. Thus pfn_list can be freed after the registration call. Critically, registration is idempotent, and all a domain need do after hibernation or migration is re-register all its rings. (NB the id.addr.domain field may change). ** Sending To send data a guest merely calls the send or sendv hypercall int hypercall(NR_V4V,V4VOP_send,v4v_addr_t *src,v4v_addr_t *dst,void *buf, uint32_t len, uint32_t protocol); the caller provides a source address, a destination address, a buffer, a number of bytes to copy and a 32 bit protocol number. Xen ignores src->domain and instead uses the domain id of the caller. Xen will attempt to insert into a ring with id.addr equal to dst, and with id.partner equal to the caller's domid. Otherwise it will insert into a ring with id.addr equal to dst and id.partner equal to V4V_DOMID_ANY. If the insert is successfull then Xen will trigger the V4V VIRQ line in the receiving domain. ** Reception Xen pads all messages on the ring to 16 bytes (the macro V4V_ROUNDUP is provided for convience), and inserts a message header infront of all messages it copies onto the ring. struct v4v_ring_message_header { uint32_t len; struct v4v_addr source; uint16_t pad; uint32_t protocol; uint8_t data[0]; }; len is the number of bytes in the message including the struct v4v_ring_message_header source specifies the source address from which the message came, source.domain is set by xen, and source.port is taken from the arguments to the send or sendv call. protocol is the protocol value given to the send call. An inline function is provided to make reading from the ring easier: ssize_t v4v_copy_out (struct v4v_ring *r, struct v4v_addr *from, uint32_t * protocol, void *_buf, size_t t, int consume) v4v_copy_out will copy at most t bytes from the ring r and place them in buf, if it is non-null. If from and protocol are not NULL pointers then the source address and protocol number will by copied into them. If consume is non-zero then the ring's receive pointer will be advanced. The function returns the number of bytes of payload that the message has (ie. the number of bytes passed to the original send or sendv call). Thus, v4v_copy_out(r,NULL,NULL,NULL,0,0); will return the number of bytes in the next message, and v4v_copy_out(r,NULL,NULL,NULL,0,1); will return the number of bytes in the next message and delete it from the ring. ** Notification If the receiving ring is full in a send or sendv hypercall, the hypercall will return with the error -EAGAIN. When sufficient space becomes available in the ring xen will raise the V4V VIRQ line. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-01 13:47 ` Ian Campbell 2012-06-07 8:47 ` Jean Guyader @ 2012-06-07 9:42 ` Jean Guyader 2012-06-07 11:40 ` Tim Deegan 1 sibling, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-07 9:42 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel@lists.xen.org On 01/06 02:47, Ian Campbell wrote: > On Thu, 2012-05-31 at 16:07 +0100, Jean Guyader wrote: > > V4V is a copy based inter vm communication system. > > > > Please have a look at this thread for more detail > > about V4V. > > http://lists.xen.org/archives/html/xen-devel/2012-05/msg01866.html > > > > This patch series is work in progress but I wanted to > > post it early enough so I can get feedback from people. > > The main thing which is missing here, which makes it rather hard to > review, is any kind of design documentation. It would also be great to > see the rationale for why things have to be done this way. > > For example it seems that there is a bunch of stuff being added to the > hypervisor which could live in the context of some guest or service > domain -- putting stuff like that in the hypervisor needs strong > arguments and rationale why it has to be there rather than somewhere > else. > > Likewise perhaps the v4v hypercall could effectively be implemented with > a multicall containing some grant copy ops and evtchn manipulations? > > But in the absence of any descriptions of the whats, hows and whys of > v4v its rather hard to have these sorts of conversations. The above are > some concrete examples but I'm more interested in seeing the more > general descriptions before we get into those. > > Ian. > > (resent, sorry if you get this message twice) * Overview ** Architecture v4v has a very simple architecture. The receiving domain registers a ring with xen. The sending domain requests that xen inserts data into the receiving domain's ring. Notification that there is new data to read is delivered by a VIRQ, and a domain can request an interrupt be generated when sufficient space is available in another domain's ring. The code that inserts the data into the ring is in xen, and xen writes a header describing the data and where it came from infront of the data. As both the ring manipulation and the header are written by the hypervisor, the receiving domain can trust their content and the format of the ring. ** Addressing v4v_addr_t identifies an endpoint address. typedef struct v4v_addr { uint32_t port; domid_t domain; } v4v_addr_t; struct v4v_ring_id uniquely identifies a ring on the platform. struct v4v_ring_id { struct v4v_addr addr; domid_t partner; }; When a send operation is performed, xen will attempt to find a ring registered by destination domain, with the required port, and with a partner of the sending domain. Failing that it will then search for a ring with the correct port and destination domain, but with partner set to V4V_DOMID_ANY. ** Setup A domain first generates a ring, and a structure describing the pages in the ring. Rings must be page aligned, and contiguous in virtual memory, but need not be contiguous in physical(pfn) or machine(mfn) memory. A ring is uniquely identified on the platform by its struct v4v_ring_id. A ring is contained in a v4v_ring_t typedef struct v4v_ring { uint64_t magic; /* * Identifies ring_id - xen only looks at this during register/unregister * and will fill in id.addr.domain */ struct v4v_ring_id id; /* length of ring[], must be a multiple of 16 */ uint32_t len; /* rx_ptr - modified by domain */ uint32_t rx_ptr; /* tx_ptr - modified by xen */ volatile uint32_t tx_ptr; uint64_t reserved[4]; volatile uint8_t ring[0]; } v4v_ring_t; To register the ring, xen also needs a list of pfn that the ring occupies. This is provided in a v4v_pfn_list_t typedef struct v4v_pfn_list_t { uint64_t magic; uint32_t npage; uint32_t pad; uint64_t reserved[3]; v4v_pfn_t pages[0]; } v4v_pfn_list_t; Registering the ring is done with a hypercall int hypercall(NR_V4V,V4VOP_register_ring,v4v_ring_t *ring,v4v_pfn_list_t *pfn_list); On registering, xen will check that the tx_ptr and rx_ptr values are valid (aligned and within the size of the ring) and modify them if they are not, it will also update id.addr.domain with the calling domain's domid and take an internal copy of all the relevant information. After registration the only field that xen will read is ring->rx_ptr, xen will write to ring->ring[] and ring->tx_ptr. Thus pfn_list can be freed after the registration call. Critically, registration is idempotent, and all a domain need do after hibernation or migration is re-register all its rings. (NB the id.addr.domain field may change). ** Sending To send data a guest merely calls the send or sendv hypercall int hypercall(NR_V4V,V4VOP_send,v4v_addr_t *src,v4v_addr_t *dst,void *buf, uint32_t len, uint32_t protocol); the caller provides a source address, a destination address, a buffer, a number of bytes to copy and a 32 bit protocol number. Xen ignores src->domain and instead uses the domain id of the caller. Xen will attempt to insert into a ring with id.addr equal to dst, and with id.partner equal to the caller's domid. Otherwise it will insert into a ring with id.addr equal to dst and id.partner equal to V4V_DOMID_ANY. If the insert is successfull then Xen will trigger the V4V VIRQ line in the receiving domain. ** Reception Xen pads all messages on the ring to 16 bytes (the macro V4V_ROUNDUP is provided for convience), and inserts a message header infront of all messages it copies onto the ring. struct v4v_ring_message_header { uint32_t len; struct v4v_addr source; uint16_t pad; uint32_t protocol; uint8_t data[0]; }; len is the number of bytes in the message including the struct v4v_ring_message_header source specifies the source address from which the message came, source.domain is set by xen, and source.port is taken from the arguments to the send or sendv call. protocol is the protocol value given to the send call. An inline function is provided to make reading from the ring easier: ssize_t v4v_copy_out (struct v4v_ring *r, struct v4v_addr *from, uint32_t * protocol, void *_buf, size_t t, int consume) v4v_copy_out will copy at most t bytes from the ring r and place them in buf, if it is non-null. If from and protocol are not NULL pointers then the source address and protocol number will by copied into them. If consume is non-zero then the ring's receive pointer will be advanced. The function returns the number of bytes of payload that the message has (ie. the number of bytes passed to the original send or sendv call). Thus, v4v_copy_out(r,NULL,NULL,NULL,0,0); will return the number of bytes in the next message, and v4v_copy_out(r,NULL,NULL,NULL,0,1); will return the number of bytes in the next message and delete it from the ring. ** Notification If the receiving ring is full in a send or sendv hypercall, the hypercall will return with the error -EAGAIN. When sufficient space becomes available in the ring xen will raise the V4V VIRQ line. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-07 9:42 ` Jean Guyader @ 2012-06-07 11:40 ` Tim Deegan 2012-06-07 15:36 ` Tim Deegan 0 siblings, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-07 11:40 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, xen-devel@lists.xen.org Hi, Thanks for this. Overall, it looks like Xen is doing a few things here: - nameservice for registering services and finding endpoints; - ring manipulation arithmetic; - copying data; and - notifying endpoints. The shared-ring logic was able to do all of these, with a few drawbacks: - The xenstore handshake stuff is really grotty; - grant maps can cause zombie domains; and - it doesn't do many-to-one multicast. Is that a fair summary? Using copy semantics intead of shared rings fixes the zombie domain problem, and I think we should definitely take that. We talked last week about ways we could improve xenstore to provde the nameservice aspects of v4v; if that can be done I think it should be. The rest of is within spitting distance of a multicall of grant copies and event-channel pokes - except for the many-to-one multicast. That relies on Xen policing the ring headers to make sure the length fields are correct. What's the use case for that? Do you use it just to negotiate a dedicated ring for each sender or is there a need for multiple data streams to be multiplexed together? I have a few questions on the design, inline below. At 10:42 +0100 on 07 Jun (1339065745), Jean Guyader wrote: > v4v has a very simple architecture. The receiving domain registers a > ring with xen. The sending domain requests that xen inserts data into > the receiving domain's ring. Notification that there is new data to > read is delivered by a VIRQ Not an evtchn? > , and a domain can request an interrupt be generated when sufficient > space is available in another domain's ring. How is that arranged? It doesn't look like the receiver notifies Xen when it consumes a ring entry. > The code that inserts > the data into the ring is in xen, and xen writes a header describing > the data and where it came from infront of the data. As both the ring > manipulation and the header are written by the hypervisor, the > receiving domain can trust their content and the format of the ring. Does that save a copy on the receiving side? > ** Addressing > v4v_addr_t identifies an endpoint address. > typedef struct v4v_addr > { > uint32_t port; > domid_t domain; > } v4v_addr_t; > struct v4v_ring_id uniquely identifies a ring on the platform. > > struct v4v_ring_id > { > struct v4v_addr addr; > domid_t partner; > }; > > When a send operation is performed, xen will attempt to find a ring > registered by destination domain, with the required port, and with > a partner of the sending domain. Failing that it will then search > for a ring with the correct port and destination domain, but with > partner set to V4V_DOMID_ANY. > > ** Setup > A domain first generates a ring, and a structure describing the pages > in the ring. Rings must be page aligned, and contiguous in virtual > memory, but need not be contiguous in physical(pfn) or machine(mfn) > memory. A ring is uniquely identified on the platform by its struct > v4v_ring_id. > > A ring is contained in a v4v_ring_t > typedef struct v4v_ring > { > uint64_t magic; > /* > * Identifies ring_id - xen only looks at this during register/unregister > * and will fill in id.addr.domain > */ > struct v4v_ring_id id; > /* length of ring[], must be a multiple of 16 */ > uint32_t len; > /* rx_ptr - modified by domain */ > uint32_t rx_ptr; > /* tx_ptr - modified by xen */ > volatile uint32_t tx_ptr; > uint64_t reserved[4]; > volatile uint8_t ring[0]; > } v4v_ring_t; (Digressing into code review for a minute, I don't think we should use 'volatile' in the public headers, and possibly it will be more efficient to use barriers anyway if we're processing the received data in place). > To register the ring, xen also needs a list of pfn that the ring occupies. > This is provided in a v4v_pfn_list_t > typedef struct v4v_pfn_list_t > { > uint64_t magic; > uint32_t npage; > uint32_t pad; > uint64_t reserved[3]; > v4v_pfn_t pages[0]; > } v4v_pfn_list_t; > > Registering the ring is done with a hypercall > > int hypercall(NR_V4V,V4VOP_register_ring,v4v_ring_t *ring,v4v_pfn_list_t *pfn_list); > > On registering, xen will check that the tx_ptr and rx_ptr values are valid (aligned > and within the size of the ring) and modify them if they are not Modify them? Not return an error? >, it will also update > id.addr.domain with the calling domain's domid and take an internal copy of all the > relevant information. After registration the only field that xen will read is > ring->rx_ptr, xen will write to ring->ring[] and ring->tx_ptr. > Thus pfn_list can be freed after the registration call. Critically, registration is > idempotent, and all a domain need do after hibernation or migration is re-register all > its rings. (NB the id.addr.domain field may change). > > ** Sending > > To send data a guest merely calls the send or sendv hypercall > int hypercall(NR_V4V,V4VOP_send,v4v_addr_t *src,v4v_addr_t *dst,void *buf, uint32_t len, > uint32_t protocol); What does the source address do here? Do I need to register a ring in order to send to another ring? Why the separate 'protocol' argument when 'port' is bound into the address struct? A single ring receives messages for a given port but all protocols? > the caller provides a source address, a destination address, a buffer, a number of bytes to copy > and a 32 bit protocol number. Xen ignores src->domain and instead uses the domain id of the caller. > > Xen will attempt to insert into a ring with id.addr equal to dst, and with > id.partner equal to the caller's domid. Otherwise it will insert into a ring > with id.addr equal to dst and id.partner equal to V4V_DOMID_ANY. If the insert is successfull > then Xen will trigger the V4V VIRQ line in the receiving domain. > > ** Reception > > Xen pads all messages on the ring to 16 bytes (the macro V4V_ROUNDUP is provided > for convience), and inserts a message header infront of all messages it copies onto the ring. > > struct v4v_ring_message_header > { > uint32_t len; > struct v4v_addr source; > uint16_t pad; > uint32_t protocol; > uint8_t data[0]; > }; > > len is the number of bytes in the message including the struct v4v_ring_message_header > source specifies the source address from which the message came, source.domain is > set by xen, and source.port is taken from the arguments to the send or sendv call. > protocol is the protocol value given to the send call. > > An inline function is provided to make reading from the ring easier: > > ssize_t > v4v_copy_out (struct v4v_ring *r, struct v4v_addr *from, uint32_t * protocol, > void *_buf, size_t t, int consume) I guess that answers my question about avoiding a copy on the receiver. :( Cheers, Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-07 11:40 ` Tim Deegan @ 2012-06-07 15:36 ` Tim Deegan 2012-06-13 10:48 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-07 15:36 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, xen-devel@lists.xen.org At 12:40 +0100 on 07 Jun (1339072831), Tim Deegan wrote: > Hi, > > Thanks for this. > > Overall, it looks like Xen is doing a few things here: > - nameservice for registering services and finding endpoints; > - ring manipulation arithmetic; > - copying data; and > - notifying endpoints. > > The shared-ring logic was able to do all of these, with a few drawbacks: > - The xenstore handshake stuff is really grotty; > - grant maps can cause zombie domains; and > - it doesn't do many-to-one multicast. We've just had a discussion of this in person, and from my notes the two things that stand out are: - yes, you want to do many-to-one multiplexing, in particular to avoid the server needing a ring for every client; and - one reason not to use Xenstore is that there is only one Xenstore page per VM and it may not be possible to safely share it with other users (in particular between a BIOS/EFI user and the main OS). The Xenstore point is understandable, and probably something that ought to be fixed anyway -- we have seen people run into similar problems with BIOS drivers for blkfront and netfront. Using one ring for all clients raises the question of access control and admission control -- in particular how do you avoid DoS from badly-behaved clients? And, given your concerns about sharing an OS with an uncooperative Xenstore client, how do you handle sharing the OS with a badly behaved v4v client? If we _do_ need one ring with multiple writers, and therefore Xen needs to arbitrate writes, there's still room for the policy-based parts (controlling connection setup, for example) to live outside the hypervisor, openvswitch-style. Cheers, Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-07 15:36 ` Tim Deegan @ 2012-06-13 10:48 ` Jean Guyader 2012-06-13 11:44 ` Tim Deegan 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-13 10:48 UTC (permalink / raw) To: Tim Deegan; +Cc: Ian Campbell, xen-devel@lists.xen.org On 07/06 04:36, Tim Deegan wrote: > At 12:40 +0100 on 07 Jun (1339072831), Tim Deegan wrote: > > Hi, > > > > Thanks for this. > > > > Overall, it looks like Xen is doing a few things here: > > - nameservice for registering services and finding endpoints; > > - ring manipulation arithmetic; > > - copying data; and > > - notifying endpoints. > > > > The shared-ring logic was able to do all of these, with a few drawbacks: > > - The xenstore handshake stuff is really grotty; > > - grant maps can cause zombie domains; and > > - it doesn't do many-to-one multicast. > > We've just had a discussion of this in person, and from my notes the two > things that stand out are: > > - yes, you want to do many-to-one multiplexing, in particular to > avoid the server needing a ring for every client; and > - one reason not to use Xenstore is that there is only one Xenstore > page per VM and it may not be possible to safely share it with other > users (in particular between a BIOS/EFI user and the main OS). > > The Xenstore point is understandable, and probably something that ought > to be fixed anyway -- we have seen people run into similar problems with > BIOS drivers for blkfront and netfront. > > Using one ring for all clients raises the question of access control and > admission control -- in particular how do you avoid DoS from > badly-behaved clients? > > And, given your concerns about sharing an OS with an uncooperative > Xenstore client, how do you handle sharing the OS with a badly behaved > v4v client? > > If we _do_ need one ring with multiple writers, and therefore Xen needs > to arbitrate writes, there's still room for the policy-based parts > (controlling connection setup, for example) to live outside the > hypervisor, openvswitch-style. > Thanks for summary Tim. Today the acl check in V4V (not part of the current patch serie) is done for every copy by Xen. Moving the policy control outside of Xen would mean that you still need to have a copy of the acls in Xen and the worst thing that can happen is for the copy to get out of sync. What do you think would be the next step going forward? Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-13 10:48 ` Jean Guyader @ 2012-06-13 11:44 ` Tim Deegan 2012-06-14 10:55 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-13 11:44 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, xen-devel@lists.xen.org At 11:48 +0100 on 13 Jun (1339588138), Jean Guyader wrote: > On 07/06 04:36, Tim Deegan wrote: > > Using one ring for all clients raises the question of access control and > > admission control -- in particular how do you avoid DoS from > > badly-behaved clients? > > > > And, given your concerns about sharing an OS with an uncooperative > > Xenstore client, how do you handle sharing the OS with a badly behaved > > v4v client? > > > > If we _do_ need one ring with multiple writers, and therefore Xen needs > > to arbitrate writes, there's still room for the policy-based parts > > (controlling connection setup, for example) to live outside the > > hypervisor, openvswitch-style. > > Today the acl check in V4V (not part of the current patch serie) is > done for every copy by Xen. OK; can you describe roughly how it works? Is it a whitelist of good domains, or a blacklist of bad? Does it just do access control or is there rate limiting? Can it detect and block badly-behaved clients, or is that something you do in the server? Have you given any thought to my second question -- if you can't rely on the existing xenstore code, are there problems with sharing a VM with other v4v drivers? My intuition is that at least it would not be so bad, since non-malicious drivers ought to be able to coexist, but I'd like to know your opinion. > Moving the policy control outside of Xen > would mean that you still need to have a copy of the acls in Xen and > the worst thing that can happen is for the copy to get out of sync. What I meant by openvswitch-style is to have a single low-level ACL in the hypervisor (maybe as a whitelist of known good connections) and fault all unusual behaviour (new domain appears, &c) to the more complex policy engine in user-space. Really it depends on what kind of policy you want to be able to express. If a simple yes/no whitelist is good enough (and always will be) then it may as well live in the hypervisor and we can skip the 'defer to userspace' part. > What do you think would be the next step going forward? Hopefully, to get some feedback from other people (specifically Ian, Ian and Stefano but anyone else too!) and decide what, if anything, ought to be changed about the design. My feedback has mostly been about the hypervisor side of things -- I'm sure the tools maintainers have some ideas about how this should be merged with libvchan, to avoid having two separate comms interfaces. Cheers, Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-13 11:44 ` Tim Deegan @ 2012-06-14 10:55 ` Jean Guyader 2012-06-14 14:56 ` Tim Deegan 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-14 10:55 UTC (permalink / raw) To: Tim Deegan; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org On 13 June 2012 12:44, Tim Deegan <tim@xen.org> wrote: > At 11:48 +0100 on 13 Jun (1339588138), Jean Guyader wrote: >> On 07/06 04:36, Tim Deegan wrote: >> > Using one ring for all clients raises the question of access control and >> > admission control -- in particular how do you avoid DoS from >> > badly-behaved clients? >> > >> > And, given your concerns about sharing an OS with an uncooperative >> > Xenstore client, how do you handle sharing the OS with a badly behaved >> > v4v client? >> > >> > If we _do_ need one ring with multiple writers, and therefore Xen needs >> > to arbitrate writes, there's still room for the policy-based parts >> > (controlling connection setup, for example) to live outside the >> > hypervisor, openvswitch-style. >> >> Today the acl check in V4V (not part of the current patch serie) is >> done for every copy by Xen. > > OK; can you describe roughly how it works? Is it a whitelist of good > domains, or a blacklist of bad? Does it just do access control or is > there rate limiting? Can it detect and block badly-behaved clients, > or is that something you do in the server? > In xen we keep of list of simple acls. Here is the usage of the userspace tools that controls it. This is a straight forward implementation of the rule API. Usage: viptables command [rule] Commands : --append -A Append rule --insert -I <n> Insert rule before rule <n> --list -L List rules --delete -D[<n>] Delete rule <n> or the following rule --flush -F Flush rules --help -h Help Rule options: --dom-in -d <n> Client domid --dom-out -e <n> Server domid --port-in -p <n> Client port --port-out -q <n> Server port --jump -j {ACCEPT|REJECT} What to do > Have you given any thought to my second question -- if you can't rely on > the existing xenstore code, are there problems with sharing a VM with > other v4v drivers? My intuition is that at least it would not be so > bad, since non-malicious drivers ought to be able to coexist, but I'd > like to know your opinion. > Are you talking about having different version of V4V driver running in the same VM? I don't think that is a problem they both interact with Xen via hypercall directly so if they follow the v4v hypercall interface it's all fine. >> Moving the policy control outside of Xen >> would mean that you still need to have a copy of the acls in Xen and >> the worst thing that can happen is for the copy to get out of sync. > > What I meant by openvswitch-style is to have a single low-level ACL in > the hypervisor (maybe as a whitelist of known good connections) and > fault all unusual behaviour (new domain appears, &c) to the more complex > policy engine in user-space. > Yes sure. > Really it depends on what kind of policy you want to be able to express. > If a simple yes/no whitelist is good enough (and always will be) then it > may as well live in the hypervisor and we can skip the 'defer to > userspace' part. > >> What do you think would be the next step going forward? > > Hopefully, to get some feedback from other people (specifically Ian, Ian > and Stefano but anyone else too!) and decide what, if anything, ought to > be changed about the design. > > My feedback has mostly been about the hypervisor side of things -- I'm > sure the tools maintainers have some ideas about how this should be > merged with libvchan, to avoid having two separate comms interfaces. > Ok. Thanks for your feedback. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-14 10:55 ` Jean Guyader @ 2012-06-14 14:56 ` Tim Deegan 2012-06-14 15:10 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-14 14:56 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > Are you talking about having different version of V4V driver running > in the same VM? Yes. > I don't think that is a problem they both interact with Xen via > hypercall directly so if they follow the v4v hypercall interface it's > all fine. AFAICS if they both try to register the same port then one of them will silently get its ring discarded. And if they both try to communicate with the same remote port their entries on the pending lists will get merged (which is probably not too bad). I think the possibility for confusion depends on how you use the service. Still, it seems better than the xenstore case, anyway. :) Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-14 14:56 ` Tim Deegan @ 2012-06-14 15:10 ` Jean Guyader 2012-06-14 15:35 ` Tim Deegan 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-14 15:10 UTC (permalink / raw) To: Tim Deegan; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org On 14/06 03:56, Tim Deegan wrote: > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > Are you talking about having different version of V4V driver running > > in the same VM? > > Yes. > > > I don't think that is a problem they both interact with Xen via > > hypercall directly so if they follow the v4v hypercall interface it's > > all fine. > > AFAICS if they both try to register the same port then one of them will > silently get its ring discarded. And if they both try to communicate > with the same remote port their entries on the pending lists will get > merged (which is probably not too bad). I think the possibility for > confusion depends on how you use the service. Still, it seems better > than the xenstore case, anyway. :) > Not silently, register_ring will return an error. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-14 15:10 ` Jean Guyader @ 2012-06-14 15:35 ` Tim Deegan 2012-06-14 21:14 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-14 15:35 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > On 14/06 03:56, Tim Deegan wrote: > > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > Are you talking about having different version of V4V driver running > > > in the same VM? > > > > Yes. > > > > > I don't think that is a problem they both interact with Xen via > > > hypercall directly so if they follow the v4v hypercall interface it's > > > all fine. > > > > AFAICS if they both try to register the same port then one of them will > > silently get its ring discarded. And if they both try to communicate > > with the same remote port their entries on the pending lists will get > > merged (which is probably not too bad). I think the possibility for > > confusion depends on how you use the service. Still, it seems better > > than the xenstore case, anyway. :) > > > > Not silently, register_ring will return an error. Will it? It looks to me like v4v_ring_add just clobbers the old MFN list. Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-14 15:35 ` Tim Deegan @ 2012-06-14 21:14 ` Jean Guyader 2012-06-25 9:05 ` Tim Deegan 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-14 21:14 UTC (permalink / raw) To: Tim Deegan; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: >> On 14/06 03:56, Tim Deegan wrote: >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: >> > > Are you talking about having different version of V4V driver running >> > > in the same VM? >> > >> > Yes. >> > >> > > I don't think that is a problem they both interact with Xen via >> > > hypercall directly so if they follow the v4v hypercall interface it's >> > > all fine. >> > >> > AFAICS if they both try to register the same port then one of them will >> > silently get its ring discarded. And if they both try to communicate >> > with the same remote port their entries on the pending lists will get >> > merged (which is probably not too bad). I think the possibility for >> > confusion depends on how you use the service. Still, it seems better >> > than the xenstore case, anyway. :) >> > >> >> Not silently, register_ring will return an error. > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > list. > Ha yes. It does that now but I think it should return an error informing up the stack that a ring has already been registered. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-14 21:14 ` Jean Guyader @ 2012-06-25 9:05 ` Tim Deegan 2012-06-26 14:38 ` Ian Campbell 2012-06-28 10:19 ` Jean Guyader 0 siblings, 2 replies; 53+ messages in thread From: Tim Deegan @ 2012-06-25 9:05 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > >> On 14/06 03:56, Tim Deegan wrote: > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > >> > > Are you talking about having different version of V4V driver running > >> > > in the same VM? > >> > > >> > Yes. > >> > > >> > > I don't think that is a problem they both interact with Xen via > >> > > hypercall directly so if they follow the v4v hypercall interface it's > >> > > all fine. > >> > > >> > AFAICS if they both try to register the same port then one of them will > >> > silently get its ring discarded. And if they both try to communicate > >> > with the same remote port their entries on the pending lists will get > >> > merged (which is probably not too bad). I think the possibility for > >> > confusion depends on how you use the service. Still, it seems better > >> > than the xenstore case, anyway. :) > >> > > >> > >> Not silently, register_ring will return an error. > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > list. > > > > Ha yes. It does that now but I think it should return an error > informing up the stack that a ring has already been registered. Actually, I think it's deliberate, to allow a guest to re-register all its rings after a suspend/resume or migration, without having to worry about whether it was actually migrated into a new domain or not. That raises the question of how a v4v client ought to handle migration; doesn't it have to rely on other OS components to notify it that one has happened? Cheers, Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-25 9:05 ` Tim Deegan @ 2012-06-26 14:38 ` Ian Campbell 2012-06-28 10:38 ` Jean Guyader 2012-06-28 10:19 ` Jean Guyader 1 sibling, 1 reply; 53+ messages in thread From: Ian Campbell @ 2012-06-26 14:38 UTC (permalink / raw) To: Tim Deegan; +Cc: Jean Guyader (3P), Jean Guyader, xen-devel@lists.xen.org Hi, Sorry it's taken me so long to get round to responding to this. On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > >> On 14/06 03:56, Tim Deegan wrote: > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > >> > > Are you talking about having different version of V4V driver running > > >> > > in the same VM? > > >> > > > >> > Yes. > > >> > > > >> > > I don't think that is a problem they both interact with Xen via > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > >> > > all fine. > > >> > > > >> > AFAICS if they both try to register the same port then one of them will > > >> > silently get its ring discarded. And if they both try to communicate > > >> > with the same remote port their entries on the pending lists will get > > >> > merged (which is probably not too bad). I think the possibility for > > >> > confusion depends on how you use the service. Still, it seems better > > >> > than the xenstore case, anyway. :) > > >> > > > >> > > >> Not silently, register_ring will return an error. > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > list. > > > > > > > Ha yes. It does that now but I think it should return an error > > informing up the stack that a ring has already been registered. > > Actually, I think it's deliberate, to allow a guest to re-register all > its rings after a suspend/resume or migration, without having to worry > about whether it was actually migrated into a new domain or not. Which takes us back to the original issue Tim asked about with cohabitation of multiple (perhaps just plain buggy or even malicious) v4v clients in a single domain, doesn't it? If one of the reasons for not using xenstore is the inability for multiple clients to co-exist then there is no point moving to a different scheme with the same (even if lesser) issue. Up until now v4v has only been used by XenClient so it has avoided this issue but if we take it upstream then presumably the potential for this sort of problem to creep in comes back. Some other things from my notes... Can you remind me of the reasoning behind using VIRQ_V4V instead of regular event channels? I can't see any reason why these shouldn't/couldn't be regular event channels demuxed in the usual way. Are you going to submit any client code? I think the most important of these would be code to make libvchan able to use v4v if it is available (and both ends agree), but any other client code (like the sockets interface overlay I've heard about) would be interesting to see too. Have you had any contact from any one else who is interested in building stuff with these interfaces? (This is just curiosity about potential wider usage) I think you and Tim have been discussing access control -- can we get a summary of what this is going to look like going forward. I suppose there will be tools for manipulating this stuff -- can you post them? The patches need plenty of documentation adding to them, both in the interface docs in xen/include/public (marked up such that it comes out nicely in docs/html aka http://xenbits.xen.org/docs/unstable/hypercall/index.html) as well as design docs and any other useful material you have under docs itself. Are we generally happy that we could run e.g. block or net traffic over this channel? As it stands for example the single VIRQ would seem to provide a scalability limit to how many disks and nics a domain could have. Are there any other considerations we need to take into account here? Lastly -- have you given any thoughts to making the copy operation asynchronous? This might allow us to take advantage of copy offload hardware in the future? Ian. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-26 14:38 ` Ian Campbell @ 2012-06-28 10:38 ` Jean Guyader 2012-06-28 10:50 ` Tim Deegan 2012-06-28 11:34 ` Ian Campbell 0 siblings, 2 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-28 10:38 UTC (permalink / raw) To: Ian Campbell; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On 26/06 03:38, Ian Campbell wrote: > Hi, > > Sorry it's taken me so long to get round to responding to this. > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > >> On 14/06 03:56, Tim Deegan wrote: > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > >> > > Are you talking about having different version of V4V driver running > > > >> > > in the same VM? > > > >> > > > > >> > Yes. > > > >> > > > > >> > > I don't think that is a problem they both interact with Xen via > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > >> > > all fine. > > > >> > > > > >> > AFAICS if they both try to register the same port then one of them will > > > >> > silently get its ring discarded. And if they both try to communicate > > > >> > with the same remote port their entries on the pending lists will get > > > >> > merged (which is probably not too bad). I think the possibility for > > > >> > confusion depends on how you use the service. Still, it seems better > > > >> > than the xenstore case, anyway. :) > > > >> > > > > >> > > > >> Not silently, register_ring will return an error. > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > list. > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > informing up the stack that a ring has already been registered. > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > its rings after a suspend/resume or migration, without having to worry > > about whether it was actually migrated into a new domain or not. > > Which takes us back to the original issue Tim asked about with > cohabitation of multiple (perhaps just plain buggy or even malicious) > v4v clients in a single domain, doesn't it? > There is nothing wrong the two v4v driver running in the same guest. The probably that Tim reported was about trying to create two connections on the same port. Today with the code that I've submited in the RFC one will overwrite the other silently which isn't a good thing, that can easily be changed to notify which one got registered up the stack. > If one of the reasons for not using xenstore is the inability for > multiple clients to co-exist then there is no point moving to a > different scheme with the same (even if lesser) issue. Up until now v4v > has only been used by XenClient so it has avoided this issue but if we > take it upstream then presumably the potential for this sort of problem > to creep in comes back. > > Some other things from my notes... > > Can you remind me of the reasoning behind using VIRQ_V4V instead of > regular event channels? I can't see any reason why these > shouldn't/couldn't be regular event channels demuxed in the usual way. > No good reason, the virq can be changed to a event channel. We thought that event channels required other machinery to function. If we can deal with the event channel in the v4v driver directly I don't have any problem changing it to a event channel. What I have in mind is if one would want to use v4v in a rombios for instance where you can't rely on any of the xen pv driver code to be present. > Are you going to submit any client code? I think the most important of > these would be code to make libvchan able to use v4v if it is available > (and both ends agree), but any other client code (like the sockets > interface overlay I've heard about) would be interesting to see too. > Yes. I still have to submit the kernel driver code and the v4v library that talks to it. But now that you've pointed out that we might be able to use an event channel instead of a virq, I think we might event be able to have a driver in userspace only. > Have you had any contact from any one else who is interested in building > stuff with these interfaces? (This is just curiosity about potential > wider usage) > Today in XenClient we use this interface for: - USB transport between dom0 and guest - RPC bus beween dom0 and domU - Cross domain Citrix ICA connections. The good thing that this interface can offer is a transport for all the existing networking programs. I haven't had any contact from people interested in building stuff with these interfaces but I think V4V has quite a lot to offer already in term of thing at the top that can use it. > I think you and Tim have been discussing access control -- can we get a > summary of what this is going to look like going forward. I suppose > there will be tools for manipulating this stuff -- can you post them? > I'll post a new series this week that will include some of a feedback and the access control. > The patches need plenty of documentation adding to them, both in the > interface docs in xen/include/public (marked up such that it comes out > nicely in docs/html aka > http://xenbits.xen.org/docs/unstable/hypercall/index.html) as well as > design docs and any other useful material you have under docs itself. > I agree such change need a lot of document. Once we agreed on the interfaces I will start working on a complete documentation for them. > Are we generally happy that we could run e.g. block or net traffic over > this channel? As it stands for example the single VIRQ would seem to > provide a scalability limit to how many disks and nics a domain could > have. Are there any other considerations we need to take into account > here? > > Lastly -- have you given any thoughts to making the copy operation > asynchronous? This might allow us to take advantage of copy offload > hardware in the future? > A CPU copy already has to happen once in the guest to put it in the ring so the second copy that is done in Xen will be really cheap because it's very linkely going to be in the cache. I don't doing async copy in Xen will have a huge impact on the performance. Thanks for taking the time to review. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 10:38 ` Jean Guyader @ 2012-06-28 10:50 ` Tim Deegan 2012-06-28 11:24 ` Jean Guyader 2012-06-28 11:34 ` Ian Campbell 1 sibling, 1 reply; 53+ messages in thread From: Tim Deegan @ 2012-06-28 10:50 UTC (permalink / raw) To: Jean Guyader; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org At 11:38 +0100 on 28 Jun (1340883538), Jean Guyader wrote: > On 26/06 03:38, Ian Campbell wrote: > > Lastly -- have you given any thoughts to making the copy operation > > asynchronous? This might allow us to take advantage of copy offload > > hardware in the future? > > A CPU copy already has to happen once in the guest to put it in the > ring I don't understand -- isn't the vector-send operation designed to have Xen do a single copy from scattered sources into the destination ring? So the sender could be zero-copy? > so the second copy that is done in Xen will be really cheap because > it's very linkely going to be in the cache. I don't doing async copy > in Xen will have a huge impact on the performance. Using a DMA engine to do the copy could free up CPU cycles that Xen could use for other vcpus, so even if there isn't a throughput increase on the v4v channel, there could be a benefit to the system as a whole. We could do that with synchronous copies, pausing the vcpu while the copy happens, but I think to get full throughput we'd want to pipeline a few writes ahead. Anyway, that's all very forward-looking: AFAICS there's no need to address it right now except to be careful not to bake things into the interface that would stop us doing this later on. Cheers, Tim. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 10:50 ` Tim Deegan @ 2012-06-28 11:24 ` Jean Guyader 0 siblings, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-28 11:24 UTC (permalink / raw) To: Tim Deegan; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org On 28/06 11:50, Tim Deegan wrote: > At 11:38 +0100 on 28 Jun (1340883538), Jean Guyader wrote: > > On 26/06 03:38, Ian Campbell wrote: > > > Lastly -- have you given any thoughts to making the copy operation > > > asynchronous? This might allow us to take advantage of copy offload > > > hardware in the future? > > > > A CPU copy already has to happen once in the guest to put it in the > > ring > > I don't understand -- isn't the vector-send operation designed to have > Xen do a single copy from scattered sources into the destination ring? > So the sender could be zero-copy? > Yes sorry that is right. On send a get a pointer from the guest and then Xen memcpy the data on the receiver's ring. So the second copy that will probably happened between the ring and some memory in the receiver will be mostly free. > > so the second copy that is done in Xen will be really cheap because > > it's very linkely going to be in the cache. I don't doing async copy > > in Xen will have a huge impact on the performance. > > Using a DMA engine to do the copy could free up CPU cycles that Xen > could use for other vcpus, so even if there isn't a throughput increase > on the v4v channel, there could be a benefit to the system as a whole. > We could do that with synchronous copies, pausing the vcpu while the > copy happens, but I think to get full throughput we'd want to pipeline a > few writes ahead. > Maybe if you ring is big enough and if the guest sends very big buffer to be copied that will makes sence to use async copy like i/oat or DMA. > Anyway, that's all very forward-looking: AFAICS there's no need to > address it right now except to be careful not to bake things into the > interface that would stop us doing this later on. > Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 10:38 ` Jean Guyader 2012-06-28 10:50 ` Tim Deegan @ 2012-06-28 11:34 ` Ian Campbell 2012-06-28 11:43 ` Jean Guyader 1 sibling, 1 reply; 53+ messages in thread From: Ian Campbell @ 2012-06-28 11:34 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > On 26/06 03:38, Ian Campbell wrote: > > Hi, > > > > Sorry it's taken me so long to get round to responding to this. > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > >> > > Are you talking about having different version of V4V driver running > > > > >> > > in the same VM? > > > > >> > > > > > >> > Yes. > > > > >> > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > >> > > all fine. > > > > >> > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > >> > with the same remote port their entries on the pending lists will get > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > >> > than the xenstore case, anyway. :) > > > > >> > > > > > >> > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > list. > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > informing up the stack that a ring has already been registered. > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > its rings after a suspend/resume or migration, without having to worry > > > about whether it was actually migrated into a new domain or not. > > > > Which takes us back to the original issue Tim asked about with > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > v4v clients in a single domain, doesn't it? > > > > There is nothing wrong the two v4v driver running in the same guest. > The probably that Tim reported was about trying to create two connections > on the same port. Today with the code that I've submited in the RFC > one will overwrite the other silently which isn't a good thing, that can > easily be changed to notify which one got registered up the stack. So they'd somehow need to randomise (and retry) their use of source ports in order to co-exist? Speaking of ports, is there a registry somewhere of the well known port numbers and/or any scheme for administering these? (a text file in the repo would be find by me). > > If one of the reasons for not using xenstore is the inability for > > multiple clients to co-exist then there is no point moving to a > > different scheme with the same (even if lesser) issue. Up until now v4v > > has only been used by XenClient so it has avoided this issue but if we > > take it upstream then presumably the potential for this sort of problem > > to creep in comes back. > > > > Some other things from my notes... > > > > Can you remind me of the reasoning behind using VIRQ_V4V instead of > > regular event channels? I can't see any reason why these > > shouldn't/couldn't be regular event channels demuxed in the usual way. > > > > No good reason, the virq can be changed to a event channel. We thought > that event channels required other machinery to function. If we can > deal with the event channel in the v4v driver directly I don't have any > problem changing it to a event channel. What I have in mind is if one > would want to use v4v in a rombios for instance where you can't rely > on any of the xen pv driver code to be present. I think that will be ok -- under the hood a VIRQ is just an evtchn too so if you can make the VIRQ work you can a regular evtchn work too. > > Are you going to submit any client code? I think the most important of > > these would be code to make libvchan able to use v4v if it is available > > (and both ends agree), but any other client code (like the sockets > > interface overlay I've heard about) would be interesting to see too. > > > > Yes. I still have to submit the kernel driver code and the v4v library that > talks to it. But now that you've pointed out that we might be able to use > an event channel instead of a virq, I think we might event be able to have > a driver in userspace only. That would be a nice property to have! > > The patches need plenty of documentation adding to them, both in the > > interface docs in xen/include/public (marked up such that it comes out > > nicely in docs/html aka > > http://xenbits.xen.org/docs/unstable/hypercall/index.html) as well as > > design docs and any other useful material you have under docs itself. > > > > I agree such change need a lot of document. Once we agreed on the interfaces > I will start working on a complete documentation for them. Thanks! Ian. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 11:34 ` Ian Campbell @ 2012-06-28 11:43 ` Jean Guyader 2012-06-28 11:58 ` Ian Campbell 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-28 11:43 UTC (permalink / raw) To: Ian Campbell; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On 28/06 12:34, Ian Campbell wrote: > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > > On 26/06 03:38, Ian Campbell wrote: > > > Hi, > > > > > > Sorry it's taken me so long to get round to responding to this. > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > > >> > > Are you talking about having different version of V4V driver running > > > > > >> > > in the same VM? > > > > > >> > > > > > > >> > Yes. > > > > > >> > > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > > >> > > all fine. > > > > > >> > > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > > >> > with the same remote port their entries on the pending lists will get > > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > > >> > than the xenstore case, anyway. :) > > > > > >> > > > > > > >> > > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > > list. > > > > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > > informing up the stack that a ring has already been registered. > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > > its rings after a suspend/resume or migration, without having to worry > > > > about whether it was actually migrated into a new domain or not. > > > > > > Which takes us back to the original issue Tim asked about with > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > > v4v clients in a single domain, doesn't it? > > > > > > > There is nothing wrong the two v4v driver running in the same guest. > > The probably that Tim reported was about trying to create two connections > > on the same port. Today with the code that I've submited in the RFC > > one will overwrite the other silently which isn't a good thing, that can > > easily be changed to notify which one got registered up the stack. > > So they'd somehow need to randomise (and retry) their use of source > ports in order to co-exist? > That can be assimilated to two userspace programs trying to bind to the same TCP port. I think it's not v4v's responsability to solve this problem. > Speaking of ports, is there a registry somewhere of the well known port > numbers and/or any scheme for administering these? (a text file in the > repo would be find by me). > Port numbers are 32 bits, by convention the first 65535 will match the TCP onces, then for the rest we can have a file in the repo to reference them. > > > If one of the reasons for not using xenstore is the inability for > > > multiple clients to co-exist then there is no point moving to a > > > different scheme with the same (even if lesser) issue. Up until now v4v > > > has only been used by XenClient so it has avoided this issue but if we > > > take it upstream then presumably the potential for this sort of problem > > > to creep in comes back. > > > > > > Some other things from my notes... > > > > > > Can you remind me of the reasoning behind using VIRQ_V4V instead of > > > regular event channels? I can't see any reason why these > > > shouldn't/couldn't be regular event channels demuxed in the usual way. > > > > > > > No good reason, the virq can be changed to a event channel. We thought > > that event channels required other machinery to function. If we can > > deal with the event channel in the v4v driver directly I don't have any > > problem changing it to a event channel. What I have in mind is if one > > would want to use v4v in a rombios for instance where you can't rely > > on any of the xen pv driver code to be present. > > I think that will be ok -- under the hood a VIRQ is just an evtchn too > so if you can make the VIRQ work you can a regular evtchn work too. > > > > Are you going to submit any client code? I think the most important of > > > these would be code to make libvchan able to use v4v if it is available > > > (and both ends agree), but any other client code (like the sockets > > > interface overlay I've heard about) would be interesting to see too. > > > > > > > Yes. I still have to submit the kernel driver code and the v4v library that > > talks to it. But now that you've pointed out that we might be able to use > > an event channel instead of a virq, I think we might event be able to have > > a driver in userspace only. > > That would be a nice property to have! > > > > The patches need plenty of documentation adding to them, both in the > > > interface docs in xen/include/public (marked up such that it comes out > > > nicely in docs/html aka > > > http://xenbits.xen.org/docs/unstable/hypercall/index.html) as well as > > > design docs and any other useful material you have under docs itself. > > > > > > > I agree such change need a lot of document. Once we agreed on the interfaces > > I will start working on a complete documentation for them. > > Thanks! > Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 11:43 ` Jean Guyader @ 2012-06-28 11:58 ` Ian Campbell 2012-06-28 12:10 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Ian Campbell @ 2012-06-28 11:58 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: > On 28/06 12:34, Ian Campbell wrote: > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > > > On 26/06 03:38, Ian Campbell wrote: > > > > Hi, > > > > > > > > Sorry it's taken me so long to get round to responding to this. > > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > > > >> > > Are you talking about having different version of V4V driver running > > > > > > >> > > in the same VM? > > > > > > >> > > > > > > > >> > Yes. > > > > > > >> > > > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > > > >> > > all fine. > > > > > > >> > > > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > > > >> > with the same remote port their entries on the pending lists will get > > > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > > > >> > than the xenstore case, anyway. :) > > > > > > >> > > > > > > > >> > > > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > > > list. > > > > > > > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > > > informing up the stack that a ring has already been registered. > > > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > > > its rings after a suspend/resume or migration, without having to worry > > > > > about whether it was actually migrated into a new domain or not. > > > > > > > > Which takes us back to the original issue Tim asked about with > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > > > v4v clients in a single domain, doesn't it? > > > > > > > > > > There is nothing wrong the two v4v driver running in the same guest. > > > The probably that Tim reported was about trying to create two connections > > > on the same port. Today with the code that I've submited in the RFC > > > one will overwrite the other silently which isn't a good thing, that can > > > easily be changed to notify which one got registered up the stack. > > > > So they'd somehow need to randomise (and retry) their use of source > > ports in order to co-exist? > > > > That can be assimilated to two userspace programs trying to bind to the > same TCP port. I think it's not v4v's responsability to solve this problem. An application using TCP doesn't need to worry about choosing its own source port though. Or does this only effect destination / listening ports? > > > Speaking of ports, is there a registry somewhere of the well known port > > numbers and/or any scheme for administering these? (a text file in the > > repo would be find by me). > > > > Port numbers are 32 bits, by convention the first 65535 will match the TCP onces, > then for the rest we can have a file in the repo to reference them. OK. Ian. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 11:58 ` Ian Campbell @ 2012-06-28 12:10 ` Jean Guyader 2012-06-28 12:36 ` Ian Campbell 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-28 12:10 UTC (permalink / raw) To: Ian Campbell; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On 28/06 12:58, Ian Campbell wrote: > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: > > On 28/06 12:34, Ian Campbell wrote: > > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > > > > On 26/06 03:38, Ian Campbell wrote: > > > > > Hi, > > > > > > > > > > Sorry it's taken me so long to get round to responding to this. > > > > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > > > > >> > > Are you talking about having different version of V4V driver running > > > > > > > >> > > in the same VM? > > > > > > > >> > > > > > > > > >> > Yes. > > > > > > > >> > > > > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > > > > >> > > all fine. > > > > > > > >> > > > > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > > > > >> > with the same remote port their entries on the pending lists will get > > > > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > > > > >> > than the xenstore case, anyway. :) > > > > > > > >> > > > > > > > > >> > > > > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > > > > list. > > > > > > > > > > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > > > > informing up the stack that a ring has already been registered. > > > > > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > > > > its rings after a suspend/resume or migration, without having to worry > > > > > > about whether it was actually migrated into a new domain or not. > > > > > > > > > > Which takes us back to the original issue Tim asked about with > > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > > > > v4v clients in a single domain, doesn't it? > > > > > > > > > > > > > There is nothing wrong the two v4v driver running in the same guest. > > > > The probably that Tim reported was about trying to create two connections > > > > on the same port. Today with the code that I've submited in the RFC > > > > one will overwrite the other silently which isn't a good thing, that can > > > > easily be changed to notify which one got registered up the stack. > > > > > > So they'd somehow need to randomise (and retry) their use of source > > > ports in order to co-exist? > > > > > > > That can be assimilated to two userspace programs trying to bind to the > > same TCP port. I think it's not v4v's responsability to solve this problem. > > An application using TCP doesn't need to worry about choosing its own > source port though. > > Or does this only effect destination / listening ports? > The guest v4v driver knows which port are in used so if you put port 0 we will pick a random unused number for the source port. Jean > > > > > Speaking of ports, is there a registry somewhere of the well known port > > > numbers and/or any scheme for administering these? (a text file in the > > > repo would be find by me). > > > > > > > Port numbers are 32 bits, by convention the first 65535 will match the TCP onces, > > then for the rest we can have a file in the repo to reference them. > > OK. > > Ian. > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 12:10 ` Jean Guyader @ 2012-06-28 12:36 ` Ian Campbell 2012-06-28 13:43 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Ian Campbell @ 2012-06-28 12:36 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On Thu, 2012-06-28 at 13:10 +0100, Jean Guyader wrote: > On 28/06 12:58, Ian Campbell wrote: > > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: > > > On 28/06 12:34, Ian Campbell wrote: > > > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > > > > > On 26/06 03:38, Ian Campbell wrote: > > > > > > Hi, > > > > > > > > > > > > Sorry it's taken me so long to get round to responding to this. > > > > > > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > > > > > >> > > Are you talking about having different version of V4V driver running > > > > > > > > >> > > in the same VM? > > > > > > > > >> > > > > > > > > > >> > Yes. > > > > > > > > >> > > > > > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > > > > > >> > > all fine. > > > > > > > > >> > > > > > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > > > > > >> > with the same remote port their entries on the pending lists will get > > > > > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > > > > > >> > than the xenstore case, anyway. :) > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > > > > > list. > > > > > > > > > > > > > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > > > > > informing up the stack that a ring has already been registered. > > > > > > > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > > > > > its rings after a suspend/resume or migration, without having to worry > > > > > > > about whether it was actually migrated into a new domain or not. > > > > > > > > > > > > Which takes us back to the original issue Tim asked about with > > > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > > > > > v4v clients in a single domain, doesn't it? > > > > > > > > > > > > > > > > There is nothing wrong the two v4v driver running in the same guest. > > > > > The probably that Tim reported was about trying to create two connections > > > > > on the same port. Today with the code that I've submited in the RFC > > > > > one will overwrite the other silently which isn't a good thing, that can > > > > > easily be changed to notify which one got registered up the stack. > > > > > > > > So they'd somehow need to randomise (and retry) their use of source > > > > ports in order to co-exist? > > > > > > > > > > That can be assimilated to two userspace programs trying to bind to the > > > same TCP port. I think it's not v4v's responsability to solve this problem. > > > > An application using TCP doesn't need to worry about choosing its own > > source port though. > > > > Or does this only effect destination / listening ports? > > > > The guest v4v driver knows which port are in used so if you put port 0 > we will pick a random unused number for the source port. Except when there are two such drivers each doesn't know which the other one is using. Ian. > > Jean > > > > > > > > Speaking of ports, is there a registry somewhere of the well known port > > > > numbers and/or any scheme for administering these? (a text file in the > > > > repo would be find by me). > > > > > > > > > > Port numbers are 32 bits, by convention the first 65535 will match the TCP onces, > > > then for the rest we can have a file in the repo to reference them. > > > > OK. > > > > Ian. > > > > ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 12:36 ` Ian Campbell @ 2012-06-28 13:43 ` Jean Guyader 2012-06-28 13:47 ` Ian Campbell 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-28 13:43 UTC (permalink / raw) To: Ian Campbell; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On 28/06 01:36, Ian Campbell wrote: > On Thu, 2012-06-28 at 13:10 +0100, Jean Guyader wrote: > > On 28/06 12:58, Ian Campbell wrote: > > > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: > > > > On 28/06 12:34, Ian Campbell wrote: > > > > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > > > > > > On 26/06 03:38, Ian Campbell wrote: > > > > > > > Hi, > > > > > > > > > > > > > > Sorry it's taken me so long to get round to responding to this. > > > > > > > > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > > > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > > > > > > >> > > Are you talking about having different version of V4V driver running > > > > > > > > > >> > > in the same VM? > > > > > > > > > >> > > > > > > > > > > >> > Yes. > > > > > > > > > >> > > > > > > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > > > > > > >> > > all fine. > > > > > > > > > >> > > > > > > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > > > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > > > > > > >> > with the same remote port their entries on the pending lists will get > > > > > > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > > > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > > > > > > >> > than the xenstore case, anyway. :) > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > > > > > > list. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > > > > > > informing up the stack that a ring has already been registered. > > > > > > > > > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > > > > > > its rings after a suspend/resume or migration, without having to worry > > > > > > > > about whether it was actually migrated into a new domain or not. > > > > > > > > > > > > > > Which takes us back to the original issue Tim asked about with > > > > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > > > > > > v4v clients in a single domain, doesn't it? > > > > > > > > > > > > > > > > > > > There is nothing wrong the two v4v driver running in the same guest. > > > > > > The probably that Tim reported was about trying to create two connections > > > > > > on the same port. Today with the code that I've submited in the RFC > > > > > > one will overwrite the other silently which isn't a good thing, that can > > > > > > easily be changed to notify which one got registered up the stack. > > > > > > > > > > So they'd somehow need to randomise (and retry) their use of source > > > > > ports in order to co-exist? > > > > > > > > > > > > > That can be assimilated to two userspace programs trying to bind to the > > > > same TCP port. I think it's not v4v's responsability to solve this problem. > > > > > > An application using TCP doesn't need to worry about choosing its own > > > source port though. > > > > > > Or does this only effect destination / listening ports? > > > > > > > The guest v4v driver knows which port are in used so if you put port 0 > > we will pick a random unused number for the source port. > > Except when there are two such drivers each doesn't know which the other > one is using. > Then the kernel will try to register the ring and the hypercall will fail because it's already registered. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 13:43 ` Jean Guyader @ 2012-06-28 13:47 ` Ian Campbell 2012-06-28 16:35 ` Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Ian Campbell @ 2012-06-28 13:47 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On Thu, 2012-06-28 at 14:43 +0100, Jean Guyader wrote: > On 28/06 01:36, Ian Campbell wrote: > > On Thu, 2012-06-28 at 13:10 +0100, Jean Guyader wrote: > > > On 28/06 12:58, Ian Campbell wrote: > > > > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: > > > > > On 28/06 12:34, Ian Campbell wrote: > > > > > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > > > > > > > On 26/06 03:38, Ian Campbell wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > Sorry it's taken me so long to get round to responding to this. > > > > > > > > > > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > > > > > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > > > > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > > > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > > > > > > > > > >> On 14/06 03:56, Tim Deegan wrote: > > > > > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > > > > > > > > > >> > > Are you talking about having different version of V4V driver running > > > > > > > > > > >> > > in the same VM? > > > > > > > > > > >> > > > > > > > > > > > >> > Yes. > > > > > > > > > > >> > > > > > > > > > > > >> > > I don't think that is a problem they both interact with Xen via > > > > > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > > > > > > > > > >> > > all fine. > > > > > > > > > > >> > > > > > > > > > > > >> > AFAICS if they both try to register the same port then one of them will > > > > > > > > > > >> > silently get its ring discarded. And if they both try to communicate > > > > > > > > > > >> > with the same remote port their entries on the pending lists will get > > > > > > > > > > >> > merged (which is probably not too bad). I think the possibility for > > > > > > > > > > >> > confusion depends on how you use the service. Still, it seems better > > > > > > > > > > >> > than the xenstore case, anyway. :) > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> Not silently, register_ring will return an error. > > > > > > > > > > > > > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > > > > > > > > > list. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ha yes. It does that now but I think it should return an error > > > > > > > > > > informing up the stack that a ring has already been registered. > > > > > > > > > > > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > > > > > > > > > its rings after a suspend/resume or migration, without having to worry > > > > > > > > > about whether it was actually migrated into a new domain or not. > > > > > > > > > > > > > > > > Which takes us back to the original issue Tim asked about with > > > > > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > > > > > > > > v4v clients in a single domain, doesn't it? > > > > > > > > > > > > > > > > > > > > > > There is nothing wrong the two v4v driver running in the same guest. > > > > > > > The probably that Tim reported was about trying to create two connections > > > > > > > on the same port. Today with the code that I've submited in the RFC > > > > > > > one will overwrite the other silently which isn't a good thing, that can > > > > > > > easily be changed to notify which one got registered up the stack. > > > > > > > > > > > > So they'd somehow need to randomise (and retry) their use of source > > > > > > ports in order to co-exist? > > > > > > > > > > > > > > > > That can be assimilated to two userspace programs trying to bind to the > > > > > same TCP port. I think it's not v4v's responsability to solve this problem. > > > > > > > > An application using TCP doesn't need to worry about choosing its own > > > > source port though. > > > > > > > > Or does this only effect destination / listening ports? > > > > > > > > > > The guest v4v driver knows which port are in used so if you put port 0 > > > we will pick a random unused number for the source port. > > > > Except when there are two such drivers each doesn't know which the other > > one is using. > > > > Then the kernel will try to register the ring and the hypercall will fail > because it's already registered. At which point what happens? How do two unrelated V4V drivers co-exist given this? Ian. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 13:47 ` Ian Campbell @ 2012-06-28 16:35 ` Jean Guyader 2012-07-02 14:14 ` Ian Campbell 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-06-28 16:35 UTC (permalink / raw) To: Ian Campbell; +Cc: Tim (Xen.org), Jean Guyader, xen-devel@lists.xen.org On 28 June 2012 14:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2012-06-28 at 14:43 +0100, Jean Guyader wrote: >> On 28/06 01:36, Ian Campbell wrote: >> > On Thu, 2012-06-28 at 13:10 +0100, Jean Guyader wrote: >> > > On 28/06 12:58, Ian Campbell wrote: >> > > > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: >> > > > > On 28/06 12:34, Ian Campbell wrote: >> > > > > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: >> > > > > > > On 26/06 03:38, Ian Campbell wrote: >> > > > > > > > Hi, >> > > > > > > > >> > > > > > > > Sorry it's taken me so long to get round to responding to this. >> > > > > > > > >> > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: >> > > > > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: >> > > > > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: >> > > > > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: >> > > > > > > > > > >> On 14/06 03:56, Tim Deegan wrote: >> > > > > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: >> > > > > > > > > > >> > > Are you talking about having different version of V4V driver running >> > > > > > > > > > >> > > in the same VM? >> > > > > > > > > > >> > >> > > > > > > > > > >> > Yes. >> > > > > > > > > > >> > >> > > > > > > > > > >> > > I don't think that is a problem they both interact with Xen via >> > > > > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's >> > > > > > > > > > >> > > all fine. >> > > > > > > > > > >> > >> > > > > > > > > > >> > AFAICS if they both try to register the same port then one of them will >> > > > > > > > > > >> > silently get its ring discarded. And if they both try to communicate >> > > > > > > > > > >> > with the same remote port their entries on the pending lists will get >> > > > > > > > > > >> > merged (which is probably not too bad). I think the possibility for >> > > > > > > > > > >> > confusion depends on how you use the service. Still, it seems better >> > > > > > > > > > >> > than the xenstore case, anyway. :) >> > > > > > > > > > >> > >> > > > > > > > > > >> >> > > > > > > > > > >> Not silently, register_ring will return an error. >> > > > > > > > > > > >> > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN >> > > > > > > > > > > list. >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > Ha yes. It does that now but I think it should return an error >> > > > > > > > > > informing up the stack that a ring has already been registered. >> > > > > > > > > >> > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all >> > > > > > > > > its rings after a suspend/resume or migration, without having to worry >> > > > > > > > > about whether it was actually migrated into a new domain or not. >> > > > > > > > >> > > > > > > > Which takes us back to the original issue Tim asked about with >> > > > > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) >> > > > > > > > v4v clients in a single domain, doesn't it? >> > > > > > > > >> > > > > > > >> > > > > > > There is nothing wrong the two v4v driver running in the same guest. >> > > > > > > The probably that Tim reported was about trying to create two connections >> > > > > > > on the same port. Today with the code that I've submited in the RFC >> > > > > > > one will overwrite the other silently which isn't a good thing, that can >> > > > > > > easily be changed to notify which one got registered up the stack. >> > > > > > >> > > > > > So they'd somehow need to randomise (and retry) their use of source >> > > > > > ports in order to co-exist? >> > > > > > >> > > > > >> > > > > That can be assimilated to two userspace programs trying to bind to the >> > > > > same TCP port. I think it's not v4v's responsability to solve this problem. >> > > > >> > > > An application using TCP doesn't need to worry about choosing its own >> > > > source port though. >> > > > >> > > > Or does this only effect destination / listening ports? >> > > > >> > > >> > > The guest v4v driver knows which port are in used so if you put port 0 >> > > we will pick a random unused number for the source port. >> > >> > Except when there are two such drivers each doesn't know which the other >> > one is using. >> > >> >> Then the kernel will try to register the ring and the hypercall will fail >> because it's already registered. > > At which point what happens? How do two unrelated V4V drivers co-exist > given this? > It happens when both driver will start registering their rings and if they are using the same port. One will get a success out of the hypercall the other one a failure. If we are in the case that user space used 0 for port we will retry with another random port number until it succeed. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-28 16:35 ` Jean Guyader @ 2012-07-02 14:14 ` Ian Campbell 0 siblings, 0 replies; 53+ messages in thread From: Ian Campbell @ 2012-07-02 14:14 UTC (permalink / raw) To: Jean Guyader; +Cc: Tim (Xen.org), Jean Guyader (3P), xen-devel@lists.xen.org On Thu, 2012-06-28 at 17:35 +0100, Jean Guyader wrote: > On 28 June 2012 14:47, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Thu, 2012-06-28 at 14:43 +0100, Jean Guyader wrote: > >> On 28/06 01:36, Ian Campbell wrote: > >> > On Thu, 2012-06-28 at 13:10 +0100, Jean Guyader wrote: > >> > > On 28/06 12:58, Ian Campbell wrote: > >> > > > On Thu, 2012-06-28 at 12:43 +0100, Jean Guyader wrote: > >> > > > > On 28/06 12:34, Ian Campbell wrote: > >> > > > > > On Thu, 2012-06-28 at 11:38 +0100, Jean Guyader wrote: > >> > > > > > > On 26/06 03:38, Ian Campbell wrote: > >> > > > > > > > Hi, > >> > > > > > > > > >> > > > > > > > Sorry it's taken me so long to get round to responding to this. > >> > > > > > > > > >> > > > > > > > On Mon, 2012-06-25 at 10:05 +0100, Tim Deegan wrote: > >> > > > > > > > > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > >> > > > > > > > > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > >> > > > > > > > > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > >> > > > > > > > > > >> On 14/06 03:56, Tim Deegan wrote: > >> > > > > > > > > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > >> > > > > > > > > > >> > > Are you talking about having different version of V4V driver running > >> > > > > > > > > > >> > > in the same VM? > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > Yes. > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > I don't think that is a problem they both interact with Xen via > >> > > > > > > > > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > >> > > > > > > > > > >> > > all fine. > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > AFAICS if they both try to register the same port then one of them will > >> > > > > > > > > > >> > silently get its ring discarded. And if they both try to communicate > >> > > > > > > > > > >> > with the same remote port their entries on the pending lists will get > >> > > > > > > > > > >> > merged (which is probably not too bad). I think the possibility for > >> > > > > > > > > > >> > confusion depends on how you use the service. Still, it seems better > >> > > > > > > > > > >> > than the xenstore case, anyway. :) > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > >> > > > > > > > > > >> Not silently, register_ring will return an error. > >> > > > > > > > > > > > >> > > > > > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > >> > > > > > > > > > > list. > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > Ha yes. It does that now but I think it should return an error > >> > > > > > > > > > informing up the stack that a ring has already been registered. > >> > > > > > > > > > >> > > > > > > > > Actually, I think it's deliberate, to allow a guest to re-register all > >> > > > > > > > > its rings after a suspend/resume or migration, without having to worry > >> > > > > > > > > about whether it was actually migrated into a new domain or not. > >> > > > > > > > > >> > > > > > > > Which takes us back to the original issue Tim asked about with > >> > > > > > > > cohabitation of multiple (perhaps just plain buggy or even malicious) > >> > > > > > > > v4v clients in a single domain, doesn't it? > >> > > > > > > > > >> > > > > > > > >> > > > > > > There is nothing wrong the two v4v driver running in the same guest. > >> > > > > > > The probably that Tim reported was about trying to create two connections > >> > > > > > > on the same port. Today with the code that I've submited in the RFC > >> > > > > > > one will overwrite the other silently which isn't a good thing, that can > >> > > > > > > easily be changed to notify which one got registered up the stack. > >> > > > > > > >> > > > > > So they'd somehow need to randomise (and retry) their use of source > >> > > > > > ports in order to co-exist? > >> > > > > > > >> > > > > > >> > > > > That can be assimilated to two userspace programs trying to bind to the > >> > > > > same TCP port. I think it's not v4v's responsability to solve this problem. > >> > > > > >> > > > An application using TCP doesn't need to worry about choosing its own > >> > > > source port though. > >> > > > > >> > > > Or does this only effect destination / listening ports? > >> > > > > >> > > > >> > > The guest v4v driver knows which port are in used so if you put port 0 > >> > > we will pick a random unused number for the source port. > >> > > >> > Except when there are two such drivers each doesn't know which the other > >> > one is using. > >> > > >> > >> Then the kernel will try to register the ring and the hypercall will fail > >> because it's already registered. > > > > At which point what happens? How do two unrelated V4V drivers co-exist > > given this? > > > > It happens when both driver will start registering their rings and if they are > using the same port. One will get a success out of the hypercall the other one > a failure. If we are in the case that user space used 0 for port we > will retry with > another random port number until it succeed. So the upshot is that for two v4v clients to co-exist then they must both be prepared to do this retry loop as well as to handle unexpected failures to bind to particular ports which they might have thought were free. If a particular client is buggy in this regard then the only impact is it to itself and not other, correct, clients (which is scant consolation if it is the one built into your OS and driving your network). So, not exactly ideal but I suppose it is somewhat better than XenStore (which currently has only one client which must provide a suitable API to any other XS users). Ian. ^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [RFC][PATCH 0/5] Add V4V to Xen 2012-06-25 9:05 ` Tim Deegan 2012-06-26 14:38 ` Ian Campbell @ 2012-06-28 10:19 ` Jean Guyader 1 sibling, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-06-28 10:19 UTC (permalink / raw) To: Tim Deegan; +Cc: Ian Campbell, Jean Guyader, xen-devel@lists.xen.org On 25/06 10:05, Tim Deegan wrote: > At 22:14 +0100 on 14 Jun (1339712061), Jean Guyader wrote: > > On 14 June 2012 16:35, Tim Deegan <tim@xen.org> wrote: > > > At 16:10 +0100 on 14 Jun (1339690244), Jean Guyader wrote: > > >> On 14/06 03:56, Tim Deegan wrote: > > >> > At 11:55 +0100 on 14 Jun (1339674908), Jean Guyader wrote: > > >> > > Are you talking about having different version of V4V driver running > > >> > > in the same VM? > > >> > > > >> > Yes. > > >> > > > >> > > I don't think that is a problem they both interact with Xen via > > >> > > hypercall directly so if they follow the v4v hypercall interface it's > > >> > > all fine. > > >> > > > >> > AFAICS if they both try to register the same port then one of them will > > >> > silently get its ring discarded. And if they both try to communicate > > >> > with the same remote port their entries on the pending lists will get > > >> > merged (which is probably not too bad). I think the possibility for > > >> > confusion depends on how you use the service. Still, it seems better > > >> > than the xenstore case, anyway. :) > > >> > > > >> > > >> Not silently, register_ring will return an error. > > > > > > Will it? It looks to me like v4v_ring_add just clobbers the old MFN > > > list. > > > > > > > Ha yes. It does that now but I think it should return an error > > informing up the stack that a ring has already been registered. > > Actually, I think it's deliberate, to allow a guest to re-register all > its rings after a suspend/resume or migration, without having to worry > about whether it was actually migrated into a new domain or not. > Yes your are right. The driver will still try to register but a correct error code could tell it weather or not it has been done. > That raises the question of how a v4v client ought to handle migration; > doesn't it have to rely on other OS components to notify it that one has > happened? > Migration will cause the connection to close and the event will propagated up the stack. Jean ^ permalink raw reply [flat|nested] 53+ messages in thread
* [RFC][PATCH 0/5] Add V4V to Xen @ 2012-05-31 14:52 Jean Guyader 2012-05-31 14:52 ` [PATCH 3/5] v4v: Introduce VIRQ_V4V Jean Guyader 0 siblings, 1 reply; 53+ messages in thread From: Jean Guyader @ 2012-05-31 14:52 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 1401 bytes --] V4V is a copy based inter vm communication system. Please have a look at this thread for more detail about V4V. http://lists.xen.org/archives/html/xen-devel/2012-05/msg01866.html This patch series is work in progress but I wanted to post it early enough so I can get feedback from people. Jean Guyader (5): xen: add ssize_t to types.h xen: Add headers to include/Makefile v4v: Introduce VIRQ_V4V xen: Enforce casting for guest_handle_cast xen: Add V4V implementation xen/arch/x86/hvm/hvm.c | 9 +- xen/arch/x86/x86_32/entry.S | 2 + xen/arch/x86/x86_64/compat/entry.S | 2 + xen/arch/x86/x86_64/entry.S | 2 + xen/common/Makefile | 1 + xen/common/domain.c | 11 +- xen/common/event_channel.c | 1 + xen/common/v4v.c | 1779 ++++++++++++++++++++++++++++++++++++ xen/include/Makefile | 3 +- xen/include/asm-x86/guest_access.h | 2 +- xen/include/public/v4v.h | 544 +++++++++++ xen/include/public/xen.h | 4 +- xen/include/xen/sched.h | 5 + xen/include/xen/types.h | 1 + xen/include/xen/v4v.h | 109 +++ 15 files changed, 2467 insertions(+), 8 deletions(-) create mode 100644 xen/common/v4v.c create mode 100644 xen/include/public/v4v.h create mode 100644 xen/include/xen/v4v.h -- 1.7.2.5 [-- Attachment #2: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 53+ messages in thread
* [PATCH 3/5] v4v: Introduce VIRQ_V4V 2012-05-31 14:52 Jean Guyader @ 2012-05-31 14:52 ` Jean Guyader 0 siblings, 0 replies; 53+ messages in thread From: Jean Guyader @ 2012-05-31 14:52 UTC (permalink / raw) To: xen-devel; +Cc: Jean Guyader [-- Attachment #1: Type: text/plain, Size: 221 bytes --] Introduce the global virq VIRQ_V4V Signed-off-by: Jean Guyader <jean.guyader@citrix.com> --- xen/common/event_channel.c | 1 + xen/include/public/xen.h | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0003-v4v-Introduce-VIRQ_V4V.patch --] [-- Type: text/x-patch; name="0003-v4v-Introduce-VIRQ_V4V.patch", Size: 1123 bytes --] diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c index 53777f8..e138600 100644 --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -107,6 +107,7 @@ static int virq_is_global(uint32_t virq) case VIRQ_TIMER: case VIRQ_DEBUG: case VIRQ_XENOPROF: + case VIRQ_V4V: rc = 0; break; case VIRQ_ARCH_0 ... VIRQ_ARCH_7: diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index b2f6c50..033cbba 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -157,7 +157,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); #define VIRQ_CON_RING 8 /* G. (DOM0) Bytes received on console */ #define VIRQ_PCPU_STATE 9 /* G. (DOM0) PCPU state changed */ #define VIRQ_MEM_EVENT 10 /* G. (DOM0) A memory event has occured */ -#define VIRQ_XC_RESERVED 11 /* G. Reserved for XenClient */ +#define VIRQ_V4V 11 /* G. V4V event has occurred */ #define VIRQ_ENOMEM 12 /* G. (DOM0) Low on heap memory */ /* Architecture-specific VIRQ definitions. */ [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 53+ messages in thread
end of thread, other threads:[~2012-07-02 14:14 UTC | newest] Thread overview: 53+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-31 15:07 [RFC][PATCH 0/5] Add V4V to Xen Jean Guyader 2012-05-31 15:07 ` [PATCH 1/5] xen: add ssize_t to types.h Jean Guyader 2012-05-31 15:29 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 2/5] xen: Add headers to include/Makefile Jean Guyader 2012-05-31 15:37 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 3/5] v4v: Introduce VIRQ_V4V Jean Guyader 2012-05-31 15:44 ` Jan Beulich 2012-05-31 15:07 ` [PATCH 4/5] xen: Enforce casting for guest_handle_cast Jean Guyader 2012-05-31 15:47 ` Jan Beulich 2012-06-14 14:08 ` Jean Guyader 2012-06-14 14:23 ` Jan Beulich 2012-06-14 14:26 ` Tim Deegan 2012-06-14 14:27 ` Tim Deegan 2012-06-14 14:40 ` Jean Guyader 2012-06-14 15:39 ` Jean Guyader 2012-06-14 15:50 ` Tim Deegan 2012-06-14 16:00 ` Jan Beulich 2012-06-14 21:19 ` Jean Guyader 2012-06-18 11:36 ` Jan Beulich 2012-06-18 12:50 ` Jean Guyader 2012-05-31 15:07 ` [PATCH 5/5] xen: Add V4V implementation Jean Guyader 2012-05-31 15:59 ` Jan Beulich 2012-06-01 12:41 ` [RFC][PATCH 0/5] Add V4V to Xen Jan Beulich 2012-06-01 13:24 ` George Dunlap 2012-06-14 14:01 ` Jean Guyader 2012-06-01 13:47 ` Ian Campbell 2012-06-07 8:47 ` Jean Guyader 2012-06-07 9:42 ` Jean Guyader 2012-06-07 11:40 ` Tim Deegan 2012-06-07 15:36 ` Tim Deegan 2012-06-13 10:48 ` Jean Guyader 2012-06-13 11:44 ` Tim Deegan 2012-06-14 10:55 ` Jean Guyader 2012-06-14 14:56 ` Tim Deegan 2012-06-14 15:10 ` Jean Guyader 2012-06-14 15:35 ` Tim Deegan 2012-06-14 21:14 ` Jean Guyader 2012-06-25 9:05 ` Tim Deegan 2012-06-26 14:38 ` Ian Campbell 2012-06-28 10:38 ` Jean Guyader 2012-06-28 10:50 ` Tim Deegan 2012-06-28 11:24 ` Jean Guyader 2012-06-28 11:34 ` Ian Campbell 2012-06-28 11:43 ` Jean Guyader 2012-06-28 11:58 ` Ian Campbell 2012-06-28 12:10 ` Jean Guyader 2012-06-28 12:36 ` Ian Campbell 2012-06-28 13:43 ` Jean Guyader 2012-06-28 13:47 ` Ian Campbell 2012-06-28 16:35 ` Jean Guyader 2012-07-02 14:14 ` Ian Campbell 2012-06-28 10:19 ` Jean Guyader -- strict thread matches above, loose matches on Subject: below -- 2012-05-31 14:52 Jean Guyader 2012-05-31 14:52 ` [PATCH 3/5] v4v: Introduce VIRQ_V4V Jean Guyader
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).