[XenPPC] Re: [Xen-devel] [PATCH] xencomm, xenmem and hypercall continuation

2006-11-10 Thread Keir Fraser
On 10/11/06 5:49 am, Isaku Yamahata [EMAIL PROTECTED] wrote:

 fix xenmem hypercall for non-trivial xencomm arch(i.e. ia64, and powerpc)
 On ia64 and powerpc, guest_handle_add_offset() effect persists over
 hypercall continuation because its consumed offset is recorced in
 guest domains memory space.
 On the other hand, x86 guest_handle_add_offset() effect is volatile
 over hypercall continuation.
 So xenmem hypercall(more specifically increase_reservation,
 decrease_reservaton, populate_memory and exchange) is broken on
 ia64 and powerpc.
 #ifdef/ifndef CONFIG_X86 is used to solve this issue without breaking
 the existing ABI. #ifdef is ugly, but it would be difficult to solve
 this issue without #ifdef and to preserve the existing ABI simaltaneously.

There must be a cleaner solution. We're not going to ifdef all over the
place.

Does xencomm have to persist the offset increments in guest memory (does the
guest depend on this)? Could it undo these effects across continuations so
that guest_handle_add_offset works properly?

 -- Keir



___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


[XenPPC] Re: [Xen-devel] [PATCH] xencomm, xenmem and hypercall continuation

2006-11-10 Thread Isaku Yamahata
On Fri, Nov 10, 2006 at 08:03:51AM +, Keir Fraser wrote:
 On 10/11/06 5:49 am, Isaku Yamahata [EMAIL PROTECTED] wrote:
 
  fix xenmem hypercall for non-trivial xencomm arch(i.e. ia64, and powerpc)
  On ia64 and powerpc, guest_handle_add_offset() effect persists over
  hypercall continuation because its consumed offset is recorced in
  guest domains memory space.
  On the other hand, x86 guest_handle_add_offset() effect is volatile
  over hypercall continuation.
  So xenmem hypercall(more specifically increase_reservation,
  decrease_reservaton, populate_memory and exchange) is broken on
  ia64 and powerpc.
  #ifdef/ifndef CONFIG_X86 is used to solve this issue without breaking
  the existing ABI. #ifdef is ugly, but it would be difficult to solve
  this issue without #ifdef and to preserve the existing ABI simaltaneously.
 
 There must be a cleaner solution. We're not going to ifdef all over the
 place.

 Does xencomm have to persist the offset increments in guest memory (does the
 guest depend on this)? 

The guest domain doesn't depend on this.
(at least it doesn't on ia64.
Probably on powerpc neither. please correct me if I'm wrong)
The other callers of guest_handle_add_offset() depend on this.
do_multicall() @ xen/common/multicall.c and
guest_console_write() @ xen/drivers/char/console.c.


 Could it undo these effects across continuations so
 that guest_handle_add_offset works properly?

- introduce guest_handle_putback_offset_for_continuation()
  (or something like that) and
  call it right before hypercall_create_continuation() in memory.c

  on x86 define it as nop.
  on ia64 and powerpc, define it as something to prepare 
  hypercall continuation.
  leave do_multicall() and guest_console_write() as is.

- introduce guest_handle_add_offset_after_continuatiton() and
  replace guest_handle_add_offset() in memory.c with it.
  leave do_multicall() and guest_console_write() as is.

- any other better idea?

-- 
yamahata

___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


[XenPPC] Re: [Xen-devel] [PATCH] xencomm, xenmem and hypercall continuation

2006-11-10 Thread Keir Fraser
On 10/11/06 08:45, Isaku Yamahata [EMAIL PROTECTED] wrote:

 - introduce guest_handle_add_offset_after_continuatiton() and
   replace guest_handle_add_offset() in memory.c with it.
   leave do_multicall() and guest_console_write() as is.

This is the best option I think. But I'm loathe to make it part of the
guest_handle API. We should avoid getting into this mess in the first place
for future hypercalls, so this will be a memory-specific function.

We should stick it at the top of memory.c, with a comment and make it a
no-op dependent on something like ARCH_HAS_XENCOMM (or just __ia64__ ||
__powerpc__ would be fine, actually, since it's just one place).

 -- Keir


___
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel


[XenPPC] Re: [Xen-devel] [PATCH] xencomm, xenmem and hypercall continuation

2006-11-10 Thread Isaku Yamahata
On Fri, Nov 10, 2006 at 09:57:44AM +, Keir Fraser wrote:
 On 10/11/06 08:45, Isaku Yamahata [EMAIL PROTECTED] wrote:
 
  - introduce guest_handle_add_offset_after_continuatiton() and
replace guest_handle_add_offset() in memory.c with it.
leave do_multicall() and guest_console_write() as is.
 
 This is the best option I think. But I'm loathe to make it part of the
 guest_handle API. We should avoid getting into this mess in the first place
 for future hypercalls, so this will be a memory-specific function.
 
 We should stick it at the top of memory.c, with a comment and make it a
 no-op dependent on something like ARCH_HAS_XENCOMM (or just __ia64__ ||
 __powerpc__ would be fine, actually, since it's just one place).

Here is the patch. Powerpc guys may have their own idea, though.
Unfortunately 4 #ifndef's are needed.

-- 
yamahata
# HG changeset patch
# User [EMAIL PROTECTED]
# Date 1163155082 -32400
# Node ID 9c2edde14184d7b5600b09f7571e6752ed097ae9
# Parent  b4e7365d451de6ffa84f24cfc29d59fea9aa50be
fix xenmem hypercall for non-trivial xencomm arch(i.e. ia64, and powerpc)
On ia64 and powerpc, guest_handle_add_offset() effect persists over
hypercall continuation because its consumed offset is recorced in
guest domains memory space.
On the other hand, x86 guest_handle_add_offset() effect is volatile
over hypercall continuation.
So xenmem hypercall(more specifically increase_reservation,
decrease_reservaton, populate_memory and exchange) is broken on
ia64 and powerpc.
On the other hand, do_multicall() @ xen/common/multicall.c and
guest_console_write() @ xen/drivers/char/console.c depends on
guest_handle_add_offset() behaviour.

#ifndef ARCH_HAS_XENCOMM is used to solve this issue without breaking
the existing ABI. #ifndef is ugly and ARCH_HAS_XENCOMM should be
used in only XENMEM hypercall.
PATCHNAME: xencomm_and_xenmem_hypercall

Signed-off-by: Isaku Yamahata [EMAIL PROTECTED]

diff -r b4e7365d451d -r 9c2edde14184 xen/common/memory.c
--- a/xen/common/memory.c   Fri Nov 10 07:48:25 2006 +
+++ b/xen/common/memory.c   Fri Nov 10 19:38:02 2006 +0900
@@ -341,23 +341,35 @@ memory_exchange(XEN_GUEST_HANDLE(xen_mem
 memflags = MEMF_dma;
 }
 
+#ifndef ARCH_HAS_XENCOMM
+/* on x86 its effect is volatile over hypercall continuation,
+ * so this adjustment is necessary.
+ * OTOH on ia64 and powerpc, guest_handle_add_offset() effects 
+ * persists over hypercall continuation so that this adjustment
+ * isn't necessary.
+ */
 guest_handle_add_offset(exch.in.extent_start, exch.nr_exchanged);
+#endif
 exch.in.nr_extents -= exch.nr_exchanged;
 
 if ( exch.in.extent_order = exch.out.extent_order )
 {
 in_chunk_order  = exch.out.extent_order - exch.in.extent_order;
 out_chunk_order = 0;
+#ifndef ARCH_HAS_XENCOMM
 guest_handle_add_offset(
 exch.out.extent_start, exch.nr_exchanged  in_chunk_order);
+#endif
 exch.out.nr_extents -= exch.nr_exchanged  in_chunk_order;
 }
 else
 {
 in_chunk_order  = 0;
 out_chunk_order = exch.in.extent_order - exch.out.extent_order;
+#ifndef ARCH_HAS_XENCOMM
 guest_handle_add_offset(
 exch.out.extent_start, exch.nr_exchanged  out_chunk_order);
+#endif
 exch.out.nr_extents -= exch.nr_exchanged  out_chunk_order;
 }
 
@@ -379,6 +391,17 @@ memory_exchange(XEN_GUEST_HANDLE(xen_mem
 {
 if ( hypercall_preempt_check() )
 {
+/* This is for ia64 and powerpc.
+ * on x86 this results in nop because its effect is volatile
+ * over hypercall continuation.
+ */
+guest_handle_add_offset(exch.in.extent_start, i);
+if ( exch.in.extent_order = exch.out.extent_order )
+guest_handle_add_offset(
+exch.out.extent_start, i  in_chunk_order);
+else
+guest_handle_add_offset(
+exch.out.extent_start, i  out_chunk_order);
 exch.nr_exchanged += i  in_chunk_order;
 if ( copy_field_to_guest(arg, exch, nr_exchanged) )
 return -EFAULT;
@@ -543,8 +566,16 @@ long do_memory_op(unsigned long cmd, XEN
 if ( unlikely(start_extent  reservation.nr_extents) )
 return start_extent;
 
+#ifndef ARCH_HAS_XENCOMM
+/* on x86 its effect is volatile over hypercall continuation,
+ * so this adjustment is necessary.
+ * OTOH on ia64 and powerpc, guest_handle_add_offset() effects 
+ * persists over hypercall continuation so that this adjustment
+ * isn't necessary.
+ */
 if ( !guest_handle_is_null(reservation.extent_start) )
 guest_handle_add_offset(reservation.extent_start, start_extent);
+#endif
 reservation.nr_extents -= start_extent;
 
 if ( (reservation.address_bits != 0) 
@@ -596,6 +627,12 @@ long do_memory_op(unsigned long cmd, XEN
 if (