[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
Hello Jan, > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: Friday, June 29, 2018 6:05 PM > To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Xin Li > <talons.lee@xxxxxxxxx> > Cc: Ming Lu <ming.lu@xxxxxxxxxx>; Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>; > Wei Liu <wei.liu2@xxxxxxxxxx>; Xin Li (Talons) <xin.li@xxxxxxxxxx>; George > Dunlap <George.Dunlap@xxxxxxxxxx>; Stefano Stabellini > <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxx; Konrad Rzeszutek Wilk > <konrad.wilk@xxxxxxxxxx>; Daniel de Graaf <dgdegra@xxxxxxxxxxxxx>; Tim > (Xen.org) <tim@xxxxxxx> > Subject: Re: [PATCH 1/2] xen/xsm: Introduce new boot parameter xsm > > >>> On 29.06.18 at 11:47, <andrew.cooper3@xxxxxxxxxx> wrote: > > On 29/06/18 10:28, Xin Li wrote: > >> --- a/docs/misc/xen-command-line.markdown > >> +++ b/docs/misc/xen-command-line.markdown > >> @@ -865,6 +865,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 | silo | flask` > > > > This should be just "dummy | flask" in this patch, and extended with > > silo in the next path. Also, options in this file should be sorted > > alphabetically, so ### xsm should be near the end, rather than beside 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. No special restriction will be > >> applied. > >> + it's also used when XSM is compiled out. > >> +enum xsm_bootparam __read_mostly xsm_bootparam = > >> +XSM_BOOTPARAM_DUMMY; > > So why "dummy" instead of "none" (or one of the boolean false strings)? It seems dummy is not fully stub. (some check by XSM_* classification) So we want to keep this "dummy" check, and override it. > > > This should be __initdata rather than __read_mostly. It is safe to be > > discarded after boot. > > And static. OK. Done. > > >> +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 > >> + xsm_bootparam = XSM_BOOTPARAM_INVALID; > >> + > >> + return 0; > > > > else > > rc = -EINVAL; > > > > return rc; > > > > As a result, the core command line infrastructure will inform the user > > if they passed an unrecognised option. > > > > ~Andrew > > > >> +} > >> + > >> +custom_param("xsm", parse_xsm_param); > > Please avoid the blank line above here - in the majority of similar cases, we > have the custom_param() immediately follow the parsing function. OK. Done. > > >> @@ -57,7 +81,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: > >> + /* empty */ > > I'm not sure of the value of this comment. I just want avoid an empty switch case. > > >> + break; > >> + > >> + case XSM_BOOTPARAM_FLASK: > >> + flask_init(policy_buffer, policy_size); > >> + break; > >> + > >> + default: > >> + printk("XSM: Invalid value for xsm= boot parameter.\n"); > >> + } > > Please don't omit the "break;" here. Sure, done. > > Jan > Best regards Xin(Talons) Li _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |