Xen devel - Nov 2006 - [PATCH] xencomm, xenmem and hypercall continuation

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.

I checked other users of both guest_handle_add_offset() and hypercall
continuation. Fortunately other users records restart points
by hypercall argument so that this isn''t an issue.


-- 
yamahata


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

On 10/11/06 5:49 am, "Isaku Yamahata" <yamahata@valinux.co.jp>
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-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Nov 10, 2006 at 08:03:51AM +0000, Keir Fraser
wrote:
> On 10/11/06 5:49 am, "Isaku Yamahata"
<yamahata@valinux.co.jp> 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-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 10/11/06 08:45, "Isaku Yamahata" <yamahata@valinux.co.jp>
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

On Fri, Nov 10, 2006 at 09:57:44AM +0000, Keir Fraser
wrote:
> On 10/11/06 08:45, "Isaku Yamahata"
<yamahata@valinux.co.jp> 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


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

On 10/11/06 10:42, "Isaku Yamahata" <yamahata@valinux.co.jp>
wrote:
>> 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.
That''s still pretty bad. I''ll rewrite the hypercall to avoid
using
guest_handle_add_offset().

 -- Keir


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