Re: [Xen-devel] [PATCH RFC 5/5] xen/xsm: include default hook action in name

[Tim Deegan <tim@xxxxxxx>]

At 11:42 -0500 on 23 Nov (1353670952), Daniel De Graaf wrote:
> This would not work for all the hooks, although it would work for most:
> domctl and mmu_update are both too complex for this type of hack, and
> console_io would need its #define moved back up to the source. For the
> other hooks, you would have to code in the domain arguments manually
> (sometimes current->domain is available, sometimes it's not) so it's not
> automatically an improvement in readability since you still have to look
> inside dummy.h to see what domain arguments are being checked.

Hrmn.  Yes, I see.

> I think a solution similar to yours could work, although it's not as 
> nice looking - use the constants in the source files but have them 
> compiled out of the XSM hooks, and have the dummy ops include a
> compile-time assertion in the XSM-disabled case to verify that the
> constant passed in matches their implementation; something like:
> 
>    #define BUILD_ASSERT(x) if (!(x)) cause_linker_error_if_not_inlined_out()
>    #ifdef XSM_ENABLE
>    /* XSM_INLINE == empty; compiled in to dummy.c */
>    #define XSM_DEFAULT_VOID void
>    #define XSM_DEFAULT_ARG
>    #define XSM_ASSERT_ACTION(action) xsm_default_t default = action
>    #else
>    /* XSM_INLINE == inline; compiled as a header */
>    #define XSM_DEFAULT_VOID xsm_default_t default
>    #define XSM_DEFAULT_ARG xsm_default_t default,
>    #define XSM_ASSERT_ACTION(action) BUILD_ASSERT(default == action)
>    #endif
> 
>    static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
>    {
>       XSM_ASSERT_ACTION(XSM_PRIV);
>       return xsm_default_action(default, current->domain, NULL);
>    }

So on an xsm-disabled build this checks that the action is the right one.
And if it's changed the author has to touch this file so will be more
likely to see that there is some other code to be changed.  Nice.

The CPP gore is a bit unfortunate, but I think it's worth it (as long as
it gets a great big comment explaining how it should be used, of course!).

>    static XSM_INLINE int xsm_unbind_pt_irq(XSM_DEFAULT_ARG struct domain *d, 
> struct xen_domctl_bind_pt_irq *bind)
>    {
>       XSM_ASSERT_ACTION(XSM_HOOK);
>       return xsm_default_action(default, current->domain, d);
>    }
> 
> Does this seem reasonable? It would force some level of compiler
> optimization to be used at all times, but I think that's already
> required due to GCC otherwise wasting lots of stack space.

Yes, even debug builds get -O1.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel