[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen/xsm: Introduce new boot parameter xsm
Thanks Jan. > >>> On 29.09.18 at 11:22, <talons.lee@xxxxxxxxx> wrote: > > --- a/docs/misc/xen-command-line.markdown > > +++ b/docs/misc/xen-command-line.markdown > > @@ -899,6 +899,19 @@ hardware domain is architecture dependent. > > Note that specifying zero as domU value means zero, while for dom0 it > > means to use the default. > > > > +### xsm > > +> `= dummy | flask` > > + > > +> Default: `dummy` > > + > > +Specify which XSM module should be enabled. This option is only > > +available if the hypervisor was compiled with XSM support. > > + > > +* `dummy`: this is the default choice. Basic restriction for common > > +deployment > > + (the dummy module) will be applied. it's also used when XSM is compiled > out. > > "It's" (upper case I). > OK. Done. > > +* `flask`: this is the policy based access control. To choose this, > > +the > > + separated option in kconfig must also be enabled. > > Perhaps better "separate" (but I'm not a native speaker)? > I prefer separated, as I checked here: https://english.stackexchange.com/questions/13144/separated-versus-separate But generally both are OK I think. > > @@ -154,6 +154,17 @@ config XSM_FLASK_POLICY > > > > If unsure, say Y. > > > > +choice > > + prompt "Default XSM implementation" > > + depends on XSM > > + default XSM_FLASK_DEFAULT if XSM_FLASK > > + default XSM_DUMMY_DEFAULT > > + config XSM_DUMMY_DEFAULT > > + bool "Match non-XSM behavior" > > + config XSM_FLASK_DEFAULT > > + bool "FLux Advanced Security Kernel" if XSM_FLASK endchoice > > Personally I'd prefer XSM_DEFAULT_*; not sure what others think. > I would like to keep this style, because I see " default "credit" if SCHED_CREDIT_DEFAULT default "credit2" if SCHED_CREDIT2_DEFAULT " in config file. > > --- a/xen/xsm/xsm_core.c > > +++ b/xen/xsm/xsm_core.c > > @@ -31,6 +31,37 @@ > > > > struct xsm_operations *xsm_ops; > > > > +enum xsm_bootparam { > > + XSM_BOOTPARAM_DUMMY, > > + XSM_BOOTPARAM_FLASK, > > Considering the #ifdef-s further down, should this perhaps also be put in > "#ifdef > CONFIG_XSM_FLASK", to avoid it mistakenly getting used when the code was > compiled out? > I would like to avoid #ifdef here, because I prefer the same enum value in different config. > > +}; > > + > > +static enum xsm_bootparam __initdata xsm_bootparam = #ifdef > > +CONFIG_XSM_FLASK_DEFAULT > > + XSM_BOOTPARAM_FLASK; > > +#else > > + XSM_BOOTPARAM_DUMMY; > > +#endif > > + > > +static int __init parse_xsm_param(const char *s) { > > + int rc = 0; > > + > > + if ( !strcmp(s, "dummy") ) > > + xsm_bootparam = XSM_BOOTPARAM_DUMMY; #ifdef > CONFIG_XSM_FLASK > > + else if ( !strcmp(s, "flask") ) > > + xsm_bootparam = XSM_BOOTPARAM_FLASK; #endif > > + else { > > Style (brace goes on its own line). OK. Done. > > > + printk("XSM: can't parse boot parameter xsm=%s\n", s); > > Again, not being a native speaker, "can't parse" sounds odd. Why not just > "unknown" or "unknown or unsupported"? OK. Done. > > > @@ -57,7 +88,20 @@ static int __init xsm_core_init(const void > *policy_buffer, size_t policy_size) > > } > > > > xsm_ops = &dummy_xsm_ops; > > - flask_init(policy_buffer, policy_size); > > + > > + switch ( xsm_bootparam ) > > + { > > + case XSM_BOOTPARAM_DUMMY: > > + break; > > + > > + case XSM_BOOTPARAM_FLASK: > > + flask_init(policy_buffer, policy_size); > > + break; > > + > > + default: > > + printk("XSM: Invalid value for xsm= boot parameter\n"); > > + break; > > What situation is this covering, when all enumerators already have their case > label? Simply ASSERT_UNREACHABLE()? > OK. Done. > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |