Re: [Xen-staging] [xen-unstable] xsm: Consolidate xsm processing within domain control hypercall.

Re: [Xen-staging] [xen-unstable] xsm: Consolidate xsm processing within domain control hypercall.

[Alex Williamson]

  Does this work right across a PV domain save/restore on x86?  On ia64
I end up with "Domain-Unnamed" after I save a PV domain and another
after I restore it, then do a shutdown.  Reverting this patch restores
correct behavior.  Thanks,

	Alex

On Tue, 2007-12-04 at 10:26 +0000, Xen staging patchbot-unstable wrote:
> # HG changeset patch
> # User Keir Fraser
> # Date 1196763935 0
> # Node ID d2bef6551c1263e457aef75ce403ba53652a803f
> # Parent  190c2592247d3258d6b2c60939d27928c70ac5ca
> xsm: Consolidate xsm processing within domain control hypercall.
> 
> Consolidate all the 15 xsm calls from within do_domctl a single
> routine that is only called in one place, xsm_domctl:
> 
> int xsm_domctl (struct xen_domctl *domctl);
> 
> The parameter to domctl is a pointer to the xen_domctl structure that
> contains a union of all sub operational parameters.
> 
> The benefits of this patch include:
> 
> (1) Easier to maintain because there is one place in the entire
> hypercall to check with the xsm, instead of 15 or more.
> 
> (2) New sub-operations don't also need to add a corresponding xsm
>     function.
> 
> (3) Removes 178 lines of code.
> 
> (4) Enhanced security because of 1-4.
> 

-- 
Alex Williamson                             HP Open Source & Linux Org.

Re: [Xen-staging] [xen-unstable] xsm: Consolidate xsm processing within domain control hypercall.

[Alex Williamson]

On Tue, 2007-12-04 at 13:06 -0700, Alex Williamson wrote:
> Does this work right across a PV domain save/restore on x86?  On ia64
> I end up with "Domain-Unnamed" after I save a PV domain and another
> after I restore it, then do a shutdown.  Reverting this patch restores
> correct behavior.  Thanks,

   This also doesn't look like it builds with XSM_ENABLE=y

	Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.

Re: Re: [Xen-staging] [xen-unstable] xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

A couple of things:

- For these modifications to work, updates also have to be made to the dummy
module for XSM_ENABLE=y to compile

- I do not think these modifications are a win.  I would like to see this
changeset reverted for the following reasons:

1) While it may reduce the number of lines of code in the domctl hypercall,
it won't really reduce the overall number of lines of code in the hypervisor
if a module chooses to implement security operations on all of the donctl
operations. 

2) This will also impose on the security modules the responsibility to
acquire and hold locks on hypervisor resources.  It would seem dangerous to
give modules this responsibility.

3) Performance will be impacted because of the additional multiplexing in 1)
and additional resource management in 2).

George

On 12/4/07 3:44 PM, "Alex Williamson" <alex.williamson@hp.com> wrote:

> 
> On Tue, 2007-12-04 at 13:06 -0700, Alex Williamson wrote:
>> Does this work right across a PV domain save/restore on x86?  On ia64
>> I end up with "Domain-Unnamed" after I save a PV domain and another
>> after I restore it, then do a shutdown.  Reverting this patch restores
>> correct behavior.  Thanks,
> 
>    This also doesn't look like it builds with XSM_ENABLE=y
> 
> Alex

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Mike D. Day]

On 04/12/07 13:44 -0700, Alex Williamson wrote:
> 
> On Tue, 2007-12-04 at 13:06 -0700, Alex Williamson wrote:
> > Does this work right across a PV domain save/restore on x86?  On ia64
> > I end up with "Domain-Unnamed" after I save a PV domain and another
> > after I restore it, then do a shutdown.  Reverting this patch restores
> > correct behavior.  Thanks,
> 
>    This also doesn't look like it builds with XSM_ENABLE=y
>  Alex -- Alex Williamson HP Open Source & Linux Org.

It builds, if you like I'll send you a build log.

Mike


-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Mike D. Day]

On 04/12/07 16:20 -0500, George S. Coker, II wrote:
> A couple of things:
> 
> - For these modifications to work, updates also have to be made to the dummy
> module for XSM_ENABLE=y to compile
> 
> - I do not think these modifications are a win.  I would like to see this
> changeset reverted for the following reasons:
> 
> 1) While it may reduce the number of lines of code in the domctl hypercall,
> it won't really reduce the overall number of lines of code in the hypervisor
> if a module chooses to implement security operations on all of the donctl
> operations. 

True, but it does concentrate the code in the security module. Also,
it only requires one entry point to the security module from within
the domctrl hypercall. I think that makes the code more maintainable
and less likely that new domctl operations will bypass xsm security. 


> 2) This will also impose on the security modules the responsibility to
> acquire and hold locks on hypervisor resources.  It would seem dangerous to
> give modules this responsibility.

I don't see it, the locking logic is still the same. Can you show me
where the module needs to acquire locks differently than without the
patch?

> 3) Performance will be impacted because of the additional multiplexing in 1)
> and additional resource management in 2).

I thought about this. I concluded it probably isn't measurable and
even if so, it really doesn't matter because domctl hypercalls are
infrequent and never performance-critical.

Mike

-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Mike D. Day]

On 04/12/07 13:06 -0700, Alex Williamson wrote:
> 
>   Does this work right across a PV domain save/restore on x86?  On ia64
> I end up with "Domain-Unnamed" after I save a PV domain and another
> after I restore it, then do a shutdown.  Reverting this patch restores
> correct behavior.  Thanks,

I'll investigate this regression. Do you have any ideas as to why this
is happening?

Mike
 

-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Alex Williamson]

On Tue, 2007-12-04 at 16:36 -0500, Mike D. Day wrote:
> On 04/12/07 13:44 -0700, Alex Williamson wrote:
> > 
> > On Tue, 2007-12-04 at 13:06 -0700, Alex Williamson wrote:
> > > Does this work right across a PV domain save/restore on x86?  On ia64
> > > I end up with "Domain-Unnamed" after I save a PV domain and another
> > > after I restore it, then do a shutdown.  Reverting this patch restores
> > > correct behavior.  Thanks,
> > 
> >    This also doesn't look like it builds with XSM_ENABLE=y
> >  Alex -- Alex Williamson HP Open Source & Linux Org.
> 
> It builds, if you like I'll send you a build log.

   Hmm, I'm not sure how.  Fails for me as shown below (x86_64).  Does
x86 have the save/restore issue I mentioned?  Thanks,

	Alex

cset 16519:62451388f630
$ make XSM_ENABLE=y FLASK_ENABLE=y xen
...
gcc -O2 -fomit-frame-pointer -m64 -DNDEBUG -fno-strict-aliasing
-std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-value
-Wdeclaration-after-statement -DVMXASSIST -nostdinc -fno-builtin
-fno-common -iwithprefix include -Werror -Wno-pointer-arith -pipe
-I/tmp/test/xen/include -I/tmp/test/xen/include/asm-x86/mach-generic
-I/tmp/test/xen/include/asm-x86/mach-default -msoft-float
-fno-stack-protector -mno-red-zone -fpic -fno-reorder-blocks
-fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE -g
-D__XEN__ -DXSM_ENABLE -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
-DFLASK_DEVELOP -DFLASK_BOOTPARAM -DFLASK_AVC_STATS -c domctl.c -o
domctl.o
In file included from domctl.c:27:
/tmp/test/xen/include/xsm/xsm.h: In function ‘xsm_getdomaininfo’:
/tmp/test/xen/include/xsm/xsm.h:148: error: ‘struct xsm_operations’ has
no member named ‘domain_getdomaininfo’
make[4]: *** [domctl.o] Error 1
make[4]: Leaving directory `/tmp/test/xen/common'
make[3]: *** [/tmp/test/xen/common/built_in.o] Error 2
make[3]: Leaving directory `/tmp/test/xen/arch/x86'
make[2]: *** [/tmp/test/xen/xen] Error 2
make[2]: Leaving directory `/tmp/test/xen'
make[1]: *** [install] Error 2
make[1]: Leaving directory `/tmp/test/xen'
make: *** [install-xen] Error 2

-- 
Alex Williamson                             HP Open Source & Linux Org.

Re: xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

On 12/4/07 4:46 PM, "Mike D. Day" <ncmike@us.ibm.com> wrote:

> On 04/12/07 16:20 -0500, George S. Coker, II wrote:
>> A couple of things:
>> 
>> - For these modifications to work, updates also have to be made to the dummy
>> module for XSM_ENABLE=y to compile
>> 
>> - I do not think these modifications are a win.  I would like to see this
>> changeset reverted for the following reasons:
>> 
>> 1) While it may reduce the number of lines of code in the domctl hypercall,
>> it won't really reduce the overall number of lines of code in the hypervisor
>> if a module chooses to implement security operations on all of the donctl
>> operations. 
> 
> True, but it does concentrate the code in the security module. Also,
> it only requires one entry point to the security module from within
> the domctrl hypercall. I think that makes the code more maintainable
> and less likely that new domctl operations will bypass xsm security.
> 
True, but it makes the security interface incredibly broad.

> 
>> 2) This will also impose on the security modules the responsibility to
>> acquire and hold locks on hypervisor resources.  It would seem dangerous to
>> give modules this responsibility.
> 
> I don't see it, the locking logic is still the same. Can you show me
> where the module needs to acquire locks differently than without the
> patch?
> 
It's not that the locking logic is different.  A security module may be
sloppy about its locking and cause Xen to crash without specifically
indicating a flaw in the security module.

Getting locks right is tricky business, it would seem the Xen would want the
responsibility for the locking of resources to avoid the ills of race
conditions, etc.

>> 3) Performance will be impacted because of the additional multiplexing in 1)
>> and additional resource management in 2).
> 
> I thought about this. I concluded it probably isn't measurable and
> even if so, it really doesn't matter because domctl hypercalls are
> infrequent and never performance-critical.
> 
True, this isn't the substantive argument.  I'm concerned about points 1) &
2).

> Mike

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: Re: xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

On 12/4/07 4:52 PM, "Alex Williamson" <alex.williamson@hp.com> wrote:

> 
> On Tue, 2007-12-04 at 16:36 -0500, Mike D. Day wrote:
>> On 04/12/07 13:44 -0700, Alex Williamson wrote:
>>> 
>>> On Tue, 2007-12-04 at 13:06 -0700, Alex Williamson wrote:
>>>> Does this work right across a PV domain save/restore on x86?  On ia64
>>>> I end up with "Domain-Unnamed" after I save a PV domain and another
>>>> after I restore it, then do a shutdown.  Reverting this patch restores
>>>> correct behavior.  Thanks,
>>> 
>>>    This also doesn't look like it builds with XSM_ENABLE=y
>>>  Alex -- Alex Williamson HP Open Source & Linux Org.
>> 
>> It builds, if you like I'll send you a build log.
> 
>    Hmm, I'm not sure how.  Fails for me as shown below (x86_64).  Does
> x86 have the save/restore issue I mentioned?  Thanks,
> 

This is because the Flask module has not been updated for this change.
While I believe it will build with XSM_ENABLE=y with no security module
selected, the resulting binary will segfault on boot when the xsm_init
routines register the dummy module and reach a NULL dereference for the
unimplemented domctl hook.

> Alex
> 
> cset 16519:62451388f630
> $ make XSM_ENABLE=y FLASK_ENABLE=y xen
> ...
> gcc -O2 -fomit-frame-pointer -m64 -DNDEBUG -fno-strict-aliasing
> -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-value
> -Wdeclaration-after-statement -DVMXASSIST -nostdinc -fno-builtin
> -fno-common -iwithprefix include -Werror -Wno-pointer-arith -pipe
> -I/tmp/test/xen/include -I/tmp/test/xen/include/asm-x86/mach-generic
> -I/tmp/test/xen/include/asm-x86/mach-default -msoft-float
> -fno-stack-protector -mno-red-zone -fpic -fno-reorder-blocks
> -fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE -g
> -D__XEN__ -DXSM_ENABLE -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
> -DFLASK_DEVELOP -DFLASK_BOOTPARAM -DFLASK_AVC_STATS -c domctl.c -o
> domctl.o
> In file included from domctl.c:27:
> /tmp/test/xen/include/xsm/xsm.h: In function Œxsm_getdomaininfo¹:
> /tmp/test/xen/include/xsm/xsm.h:148: error: Œstruct xsm_operations¹ has
> no member named Œdomain_getdomaininfo¹
> make[4]: *** [domctl.o] Error 1
> make[4]: Leaving directory `/tmp/test/xen/common'
> make[3]: *** [/tmp/test/xen/common/built_in.o] Error 2
> make[3]: Leaving directory `/tmp/test/xen/arch/x86'
> make[2]: *** [/tmp/test/xen/xen] Error 2
> make[2]: Leaving directory `/tmp/test/xen'
> make[1]: *** [install] Error 2
> make[1]: Leaving directory `/tmp/test/xen'
> make: *** [install-xen] Error 2

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Mike D. Day]

On 04/12/07 16:54 -0500, George S. Coker, II wrote:
> 
> > 
> >> 2) This will also impose on the security modules the responsibility to
> >> acquire and hold locks on hypervisor resources.  It would seem dangerous to
> >> give modules this responsibility.
> > 
> > I don't see it, the locking logic is still the same. Can you show me
> > where the module needs to acquire locks differently than without the
> > patch?
> > 
> It's not that the locking logic is different.  A security module may be
> sloppy about its locking and cause Xen to crash without specifically
> indicating a flaw in the security module.
> 
> Getting locks right is tricky business, it would seem the Xen would want the
> responsibility for the locking of resources to avoid the ills of race
> conditions, etc.

I agree with your comments, but I don't think the patch changes
locking at all. If I'm wrong I agree that's a problem. 

Mike

-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Alex Williamson]

On Tue, 2007-12-04 at 16:49 -0500, Mike D. Day wrote:
> On 04/12/07 13:06 -0700, Alex Williamson wrote:
> > 
> >   Does this work right across a PV domain save/restore on x86?  On ia64
> > I end up with "Domain-Unnamed" after I save a PV domain and another
> > after I restore it, then do a shutdown.  Reverting this patch restores
> > correct behavior.  Thanks,
> 
> I'll investigate this regression. Do you have any ideas as to why this
> is happening?

   Nope, that's why I'm hoping x86 does something similar ;^)  Since I'm
not using XSM_ENABLE, xsm_call should just be (0), which means the
changed code should all be noops... but apparently something changes.
BTW, xsm_call(domctl(domctl)) seems unnecessarily obfuscated.  Thanks,

	Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.

Re: Re: xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

On 12/4/07 5:05 PM, "Alex Williamson" <alex.williamson@hp.com> wrote:

> 
> On Tue, 2007-12-04 at 16:49 -0500, Mike D. Day wrote:
>> On 04/12/07 13:06 -0700, Alex Williamson wrote:
>>> 
>>>   Does this work right across a PV domain save/restore on x86?  On ia64
>>> I end up with "Domain-Unnamed" after I save a PV domain and another
>>> after I restore it, then do a shutdown.  Reverting this patch restores
>>> correct behavior.  Thanks,
>> 
>> I'll investigate this regression. Do you have any ideas as to why this
>> is happening?
> 
>    Nope, that's why I'm hoping x86 does something similar ;^)  Since I'm
> not using XSM_ENABLE, xsm_call should just be (0), which means the
> changed code should all be noops... but apparently something changes.
> BTW, xsm_call(domctl(domctl)) seems unnecessarily obfuscated.  Thanks,
> 

It does build on x86, but I'm confident it will rollover on boot.

> Alex

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Mike D. Day]

On 04/12/07 16:58 -0500, George S. Coker, II wrote:
> On 12/4/07 4:52 PM, "Alex Williamson" <alex.williamson@hp.com> wrote:
> > 
> >    Hmm, I'm not sure how.  Fails for me as shown below (x86_64).  Does
> > x86 have the save/restore issue I mentioned?  Thanks,
> > 
> 
> This is because the Flask module has not been updated for this change.
> While I believe it will build with XSM_ENABLE=y with no security module
> selected, the resulting binary will segfault on boot when the xsm_init
> routines register the dummy module and reach a NULL dereference for the
> unimplemented domctl hook.

Got it. I'll fix it. 

Mike

-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Mike D. Day]

On 04/12/07 13:06 -0700, Alex Williamson wrote:
> 
>   Does this work right across a PV domain save/restore on x86?  On ia64
> I end up with "Domain-Unnamed" after I save a PV domain and another
> after I restore it, then do a shutdown.  Reverting this patch restores
> correct behavior.  Thanks,

Keir, please revert this patch. I'll implement the flask module and
investigate the save/restore issue, then resubmit.

thanks,

Mike

-- 
Mike D. Day
IBM LTC
Cell: 919 412-3900
Sametime: ncmike@us.ibm.com AIM: ncmikeday  Yahoo: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

Re: xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

On 12/4/07 4:46 PM, "Mike D. Day" <ncmike@us.ibm.com> wrote:

> On 04/12/07 16:20 -0500, George S. Coker, II wrote:
>> A couple of things:
>> 
>> - For these modifications to work, updates also have to be made to the dummy
>> module for XSM_ENABLE=y to compile
>> 
>> - I do not think these modifications are a win.  I would like to see this
>> changeset reverted for the following reasons:
>> 
>> 1) While it may reduce the number of lines of code in the domctl hypercall,
>> it won't really reduce the overall number of lines of code in the hypervisor
>> if a module chooses to implement security operations on all of the donctl
>> operations. 
> 
> True, but it does concentrate the code in the security module. Also,
> it only requires one entry point to the security module from within
> the domctrl hypercall. I think that makes the code more maintainable
> and less likely that new domctl operations will bypass xsm security.
> 
I would argue that it gives the false sense of coverage because the actual
coverage is obfuscated from developers and users.  The danger is that one
could be too dismissive here because we're assuming coverage that may or may
not be implemented.

> 
>> 2) This will also impose on the security modules the responsibility to
>> acquire and hold locks on hypervisor resources.  It would seem dangerous to
>> give modules this responsibility.
> 
> I don't see it, the locking logic is still the same. Can you show me
> where the module needs to acquire locks differently than without the
> patch?
> 
>> 3) Performance will be impacted because of the additional multiplexing in 1)
>> and additional resource management in 2).
> 
> I thought about this. I concluded it probably isn't measurable and
> even if so, it really doesn't matter because domctl hypercalls are
> infrequent and never performance-critical.
> 
> Mike

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

On 12/4/07 4:59 PM, "Mike D. Day" <ncmike@us.ibm.com> wrote:

> On 04/12/07 16:54 -0500, George S. Coker, II wrote:
>> 
>>> 
>>>> 2) This will also impose on the security modules the responsibility to
>>>> acquire and hold locks on hypervisor resources.  It would seem dangerous to
>>>> give modules this responsibility.
>>> 
>>> I don't see it, the locking logic is still the same. Can you show me
>>> where the module needs to acquire locks differently than without the
>>> patch?
>>> 
>> It's not that the locking logic is different.  A security module may be
>> sloppy about its locking and cause Xen to crash without specifically
>> indicating a flaw in the security module.
>> 
>> Getting locks right is tricky business, it would seem the Xen would want the
>> responsibility for the locking of resources to avoid the ills of race
>> conditions, etc.
> 
> I agree with your comments, but I don't think the patch changes
> locking at all. If I'm wrong I agree that's a problem.

I guess I'm not quite following here.  True the locking mechanisms won't be
any different.  The same resources will be locked, hopefully, and released,
hopefully.  The issue is one of interfaces, and which parts of Xen have
responsibility for resource management and which parts are responsible for
security.  Having a clean delegation of responsibility is good for core xen
developers and xen security developers.

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: Re: xsm: Consolidate xsm processing within domain control hypercall.

[George S. Coker, II]

I've been getting together the necessary updates to XSM and Flask to deal
with the updates to domctl and a few other hypercall operations.  I've also
been working on a default module that captures the existing dom0/domU
capability model.  This default module will allow one to exercise XSM as the
extensible security framework for Xen.  However, the xsm_domctl throws a
wrench into these activities.

I'd really like to get a better understanding of the argument that motivated
the change to the creation of the xsm_domctl rollup.

> On 04/12/07 13:06 -0700, Alex Williamson wrote:
>> 
>>   Does this work right across a PV domain save/restore on x86?  On ia64
>> I end up with "Domain-Unnamed" after I save a PV domain and another
>> after I restore it, then do a shutdown.  Reverting this patch restores
>> correct behavior.  Thanks,
> 
> Keir, please revert this patch. I'll implement the flask module and
> investigate the save/restore issue, then resubmit.
> 
> thanks,
> 
> Mike

-- 
George S. Coker, II <gscoker@alpha.ncsc.mil>

Re: xsm: Consolidate xsm processing within domain control hypercall.

[Alex Williamson]

On Tue, 2007-12-04 at 15:05 -0700, Alex Williamson wrote:
> On Tue, 2007-12-04 at 16:49 -0500, Mike D. Day wrote:
> > On 04/12/07 13:06 -0700, Alex Williamson wrote:
> > > 
> > >   Does this work right across a PV domain save/restore on x86?  On ia64
> > > I end up with "Domain-Unnamed" after I save a PV domain and another
> > > after I restore it, then do a shutdown.  Reverting this patch restores
> > > correct behavior.  Thanks,
> > 
> > I'll investigate this regression. Do you have any ideas as to why this
> > is happening?
> 
>    Nope, that's why I'm hoping x86 does something similar ;^)  Since I'm
> not using XSM_ENABLE, xsm_call should just be (0), which means the
> changed code should all be noops... but apparently something changes.
> BTW, xsm_call(domctl(domctl)) seems unnecessarily obfuscated.  Thanks,

Looks like the problem is that XEN_DOMCTL_destroydomain now always
returns -ESRCH.  The patch below seems to fix it although it may be just
as correct to hard code ret to 0.  I also took the liberty of returning
the error value xsm_domctl() returns, which seems to match previous
behavior.  Thanks,

	Alex

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
---

diff -r 62451388f630 xen/common/domctl.c
--- a/xen/common/domctl.c	Tue Dec 04 11:52:10 2007 +0000
+++ b/xen/common/domctl.c	Tue Dec 04 17:11:07 2007 -0700
@@ -193,7 +193,8 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
 
     spin_lock(&domctl_lock);
 
-    if ( xsm_domctl(op) )
+    ret = xsm_domctl(op);
+    if ( ret != 0 )
         goto domctl_out;
 
     switch ( op->cmd )
@@ -400,7 +401,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( d != NULL )
         {
-            domain_kill(d);
+            ret = domain_kill(d);
             rcu_unlock_domain(d);
         }
     }