Xen devel - Sep 2006 - [RFC][PATCH] Isolating ACM''s architecture-dependent parts

This is a modification of Tristan''s previously posted patch. I have
tried to isolate architecture-dependent parts into functions that are
kept in architecture-specific include files. For example, the code that
deals with starting up ACM on the x86 architecture has been moved to
include/asm-x86/acm.h. There the policy is tried to be picked from the
multiboot structure. 
If other architectures can recycle some of the x86-specific code, such
as for example the multiboot-related code, then I guess this could be
moved to shared code areas. 
The easiest way to support ACM is to call acm_init(NULL,0); from within
the setup function if for some reason the machine cannot be started with
a policy.


Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
(patch based on previously posted patch by Tristan on 9/5/6)




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

On Tue, 2006-09-12 at 21:41 -0400, Stefan Berger wrote:
> 
> Index: root/xen-unstable.hg/xen/acm/Makefile
> ==================================================================> ---
root.orig/xen-unstable.hg/xen/acm/Makefile
> +++ root/xen-unstable.hg/xen/acm/Makefile
> @@ -3,3 +3,5 @@ obj-y += acm_policy.o
>  obj-y += acm_simple_type_enforcement_hooks.o
>  obj-y += acm_chinesewall_hooks.o
>  obj-y += acm_null_hooks.o
> +obj-$(x86_32) += acm_multiboot.o
> +obj-$(x86_64) += acm_multiboot.o 
config/x86_* defines CONFIG_X86, so that would be better to use here.
> Index: root/xen-unstable.hg/xen/include/asm-x86/acm.h
> ==================================================================> ---
/dev/null
> +++ root/xen-unstable.hg/xen/include/asm-x86/acm.h
> @@ -0,0 +1,91 @@
> +#ifndef _XEN_ASM_ACM_H
> +#define _XEN_ASM_ACM_H
> +
> +#include <xen/multiboot.h>
> +#include <acm/acm_hooks.h>
> +
> +#ifdef ACM_SECURITY
> +
> +/* Fetch acm policy module from multiboot modules.  */
> +static inline void
> +extract_acm_policy(multiboot_info_t *mbi,
> +                   unsigned int *initrdidx,
> +                   unsigned long initial_images_start,
> +                   char **_policy_start, unsigned long *_policy_len)
> +{
> +    int i;
> +    module_t *mod = (module_t *)__va(mbi->mods_addr);
> +
> +    if ( mbi->mods_count > 1 )
> +        *initrdidx = 1;
> +
> +    /*
> +     * Try all modules and see whichever could be the binary policy.
> +     * Adjust the initrdidx if module[1] is the binary policy.
> +     */
> +    for ( i = mbi->mods_count-1; i >= 1; i-- )
> +    {
> +        unsigned long start;
> +        char *policy_start;
> +        unsigned long policy_len;
> +
> +        start = initial_images_start +
> (mod[i].mod_start-mod[0].mod_start);
> +#if defined(__i386__)
> +        policy_start = (char *)start;
> +#elif defined(__x86_64__)
> +        policy_start = __va(start);
> +#endif
> +        policy_len   = mod[i].mod_end - mod[i].mod_start;
> +        if ( acm_is_policy(policy_start, policy_len) )
> +        {
> +            printf("Policy len  0x%lx, start at %p - module
%d.\n",
> +                   policy_len, policy_start, i);
> +            *_policy_start = policy_start;
> +            *_policy_len = policy_len;
> +            if ( i == 1 )
> +            {
> +                if (mbi->mods_count > 2)
> +                    *initrdidx = 2;
> +                else
> +                    *initrdidx = 0;
> +            }
> +            else
> +                *initrdidx = 1;
> +            break;
> +        }
> +    }
> +}
> +
> +static inline
> +int acm_x86_init(multiboot_info_t *mbi,
> +                 unsigned int *initrdidx,
> +                 unsigned long initial_images_start)
> +{
> +    char *_policy_start = NULL;
> +    unsigned long _policy_len = 0;
> +    /* Extract policy from multiboot.  */
> +    extract_acm_policy(mbi,
> +                       initrdidx,
> +                       initial_images_start,
> +                       &_policy_start, &_policy_len);
> +
> +    /*
> +     * Initialize access control security module no matter whether
> +     * a policy has been found or not.
> +     */
> +    return acm_init(_policy_start, _policy_len);
> +}
> +
> +#else
> +
> +static inline
> +int acm_x86_init(multiboot_info_t *mbi,
> +                 unsigned int *initrdidx,
> +                 unsigned long initial_images_start)
> +{
> +    return 0;
> +}
> +
> +#endif
> +
> +#endif
These are way too big to be static inlines. Make them regular functions
please.

Other than that, this patch seems great.

-- 
Hollis Blanchard
IBM Linux Technology Center


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

hollisb@us.ltcfwd.linux.ibm.com wrote on 09/13/2006 10:51:29 AM:
> > +
> > +static inline
> > +int acm_x86_init(multiboot_info_t *mbi,
> > +                 unsigned int *initrdidx,
> > +                 unsigned long initial_images_start)
> > +{
> > +    return 0;
> > +}
> > +
> > +#endif
> > +
> > +#endif
> 
> These are way too big to be static inlines. Make them regular functions
> please.
> 
> Other than that, this patch seems great.
Don''t want to have that checked in so far, though.

What about the multiboot code. Do you think PPC will be able to also use 
this part? Not that I would move it, it''s more out of curiosity.

   Stefan


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

On Wed, 2006-09-13 at 10:53 -0400, Stefan Berger wrote:
> 
> hollisb@us.ltcfwd.linux.ibm.com wrote on 09/13/2006 10:51:29 AM:
> > 
> > Other than that, this patch seems great. 
> 
> Don''t want to have that checked in so far, though. 
Why not?

BTW, I missed this first time:
> --- /dev/null
> +++ root/xen-unstable.hg/xen/acm/acm_multiboot.c
> @@ -0,0 +1,2 @@
> +#include <acm/acm_hooks.h>
That is where the (non-inline) ACM/multiboot functions should live; not
in a header file.
> What about the multiboot code. Do you think PPC will be able to also
> use this part? Not that I would move it, it''s more out of
curiosity.
Well, that ifdef will need changing. Why must it exist at all, is it
some weirdness of Xen/x86-64?

-- 
Hollis Blanchard
IBM Linux Technology Center


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

xen-devel-bounces@lists.xensource.com wrote on 09/13/2006 11:00:16 AM:
> 
> That is where the (non-inline) ACM/multiboot functions should live; not
> in a header file.
I could move them there but that would include the architecture-dependent 
#ifdef''s.
> 
> > What about the multiboot code. Do you think PPC will be able to also
> > use this part? Not that I would move it, it''s more out of
curiosity.
> 
> Well, that ifdef will need changing. Why must it exist at all, is it
> some weirdness of Xen/x86-64?
Yes, on x86-64 we need that. It would be possible to define MACROs for 
x86-64 and i386 so the code could look the same. It will be necessary to 
do either that for ia64 and ppc as well, or we just leave the #ifdef''s
in
the ACM code. Either way is fine by me. From what I could find, there''s
at
least grub available for ia64, so chances that ia64 can also use the 
multiboot code are high.

   Stefan


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

Le Mercredi 13 Septembre 2006 18:42, Stefan Berger a écrit
:
> xen-devel-bounces@lists.xensource.com wrote on 09/13/2006 11:00:16 AM:
> > That is where the (non-inline) ACM/multiboot functions should live;
not
> > in a header file.
>
> I could move them there but that would include the architecture-dependent
> #ifdef''s.
>
> > > What about the multiboot code. Do you think PPC will be able to
also
> > > use this part? Not that I would move it, it''s more out
of curiosity.
> >
> > Well, that ifdef will need changing. Why must it exist at all, is it
> > some weirdness of Xen/x86-64?
>
> Yes, on x86-64 we need that. It would be possible to define MACROs for
> x86-64 and i386 so the code could look the same. It will be necessary to
> do either that for ia64 and ppc as well, or we just leave the
#ifdef''s in
> the ACM code.
Hi,

sorry for the late reply, I am just back from holidays.

It seems you patch has not yet been merged.  Is there any reason ?
I''d like to see it in the repository, it will help me to enable ACM on
ia64.
> Either way is fine by me. From what I could find, there''s at
> least grub available for ia64, so chances that ia64 can also use the
> multiboot code are high.
Yes I am porting grub to ia64.  I am not sure it could use multiboot as is 
because multiboot is not 64 bits ready.

Tristan.

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

Tristan Gingold <Tristan.Gingold@bull.net> wrote on 09/26/2006 07:54:27 
AM:
> Le Mercredi 13 Septembre 2006 18:42, Stefan Berger a écrit :
> > xen-devel-bounces@lists.xensource.com wrote on 09/13/2006 11:00:16 AM:
> > > That is where the (non-inline) ACM/multiboot functions should
live;
not
> > > in a header file.
> >
> > I could move them there but that would include the 
architecture-dependent
> > #ifdef''s.
> >
> > > > What about the multiboot code. Do you think PPC will be able
to
also
> > > > use this part? Not that I would move it, it''s more
out of
curiosity.
> > >
> > > Well, that ifdef will need changing. Why must it exist at all, is
it
> > > some weirdness of Xen/x86-64?
> >
> > Yes, on x86-64 we need that. It would be possible to define MACROs for
> > x86-64 and i386 so the code could look the same. It will be necessary 
to
> > do either that for ia64 and ppc as well, or we just leave the
#ifdef''s
in
> > the ACM code.
> Hi,
> 
> sorry for the late reply, I am just back from holidays.
> 
> It seems you patch has not yet been merged.  Is there any reason ?
> I''d like to see it in the repository, it will help me to enable
ACM on
ia64.
> 
We wanted to wait for the 3.0.3 close and submit them soon after that.
> > Either way is fine by me. From what I could find, there''s at
> > least grub available for ia64, so chances that ia64 can also use the
> > multiboot code are high.
> Yes I am porting grub to ia64.  I am not sure it could use multiboot as 
is 

:-)

  Stefan
> because multiboot is not 64 bits ready.




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

Le Mardi 26 Septembre 2006 13:56, Stefan Berger a écrit
:
> Tristan Gingold <Tristan.Gingold@bull.net> wrote on 09/26/2006
07:54:27
[...]
> > Hi,
> >
> > sorry for the late reply, I am just back from holidays.
> >
> > It seems you patch has not yet been merged.  Is there any reason ?
> > I''d like to see it in the repository, it will help me to
enable ACM on
> ia64.
>
> We wanted to wait for the 3.0.3 close and submit them soon after that.
Sound reasonnable.
Thanks.

Tristan.

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