* XSM SILO boot time spew @ 2018-10-31 12:35 Andrew Cooper 2018-11-01 3:19 ` Xin Li (Talons) 0 siblings, 1 reply; 6+ messages in thread From: Andrew Cooper @ 2018-10-31 12:35 UTC (permalink / raw) To: Xen-devel List; +Cc: Xin Li, Daniel De Graaf Hello, I've noticed that the SILO code causes the following debug spew: (XEN) XSM Framework v1.0.0 initialized (XEN) Initialising XSM SILO mode (XEN) dummy.c:31: Had to override the security_domaininfo security operation with the dummy one. (XEN) dummy.c:32: Had to override the domain_create security operation with the dummy one. ... (XEN) dummy.c:158: Had to override the xen_version security operation with the dummy one. (XEN) dummy.c:159: Had to override the domain_resource_map security operation with the dummy one. (XEN) microcode: CPU0 updated from revision 0x1a to 0x25, date = 2018-04-02 (XEN) xstate: size: 0x340 and states: 0x7 which is a side effect of silo_xsm_ops only implementing a few of the hooks, falling back to dummy for the rest. Presumably we don't want to special case SILO when overriding the hooks. Would having silo_init() explicitly copy from dummy be ok? ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: XSM SILO boot time spew 2018-10-31 12:35 XSM SILO boot time spew Andrew Cooper @ 2018-11-01 3:19 ` Xin Li (Talons) 2018-11-07 17:52 ` Daniel De Graaf 0 siblings, 1 reply; 6+ messages in thread From: Xin Li (Talons) @ 2018-11-01 3:19 UTC (permalink / raw) To: Andrew Cooper, Daniel De Graaf; +Cc: Xen-devel List In patchset v4, we call register_xsm() to setup silo module. This debug log is to check if some ops not overrided by the module. I thought this is OK, since the log level is debug. I think calling register_xsm() is good, if we do want to suppress this debug log explicitly, we can check if ops eqauls silo_xsm_ops in macro set_to_dummy_if_null(). The follow diff shows what I am suggesting, is it OK? diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 3b192b5c31..b94fc43961 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -734,6 +734,7 @@ extern const unsigned int xsm_flask_init_policy_size; #endif #ifdef CONFIG_XSM_SILO +extern struct xsm_operations silo_xsm_ops; extern void silo_init(void); #else static inline void silo_init(void) {} diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 06a674fad0..5af990514f 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -15,12 +15,20 @@ struct xsm_operations dummy_xsm_ops; +#ifdef CONFIG_XSM_SILO +#define check_xsm_module(ops) \ + if (ops != &dummy_xsm_ops && ops != &silo_xsm_ops) +#else +#define check_xsm_module(ops) \ + if (ops != &dummy_xsm_ops) +#endif + #define set_to_dummy_if_null(ops, function) \ do { \ if ( !ops->function ) \ { \ ops->function = xsm_##function; \ - if (ops != &dummy_xsm_ops) \ + check_xsm_module(ops) \ dprintk(XENLOG_DEBUG, "Had to override the " #function \ " security operation with the dummy one.\n"); \ } \ diff --git a/xen/xsm/silo.c b/xen/xsm/silo.c index 4850756a3d..d2e6724e26 100644 --- a/xen/xsm/silo.c +++ b/xen/xsm/silo.c @@ -81,7 +81,7 @@ static int silo_grant_copy(struct domain *d1, struct domain *d2) return -EPERM; } -static struct xsm_operations silo_xsm_ops = { +struct xsm_operations silo_xsm_ops = { .evtchn_unbound = silo_evtchn_unbound, .evtchn_interdomain = silo_evtchn_interdomain, .grant_mapref = silo_grant_mapref, ________________________________________ From: Andrew Cooper Sent: Wednesday, October 31, 2018 8:35 PM To: Xen-devel List Cc: Daniel De Graaf; Xin Li (Talons) Subject: XSM SILO boot time spew Hello, I've noticed that the SILO code causes the following debug spew: (XEN) XSM Framework v1.0.0 initialized (XEN) Initialising XSM SILO mode (XEN) dummy.c:31: Had to override the security_domaininfo security operation with the dummy one. (XEN) dummy.c:32: Had to override the domain_create security operation with the dummy one. ... (XEN) dummy.c:158: Had to override the xen_version security operation with the dummy one. (XEN) dummy.c:159: Had to override the domain_resource_map security operation with the dummy one. (XEN) microcode: CPU0 updated from revision 0x1a to 0x25, date = 2018-04-02 (XEN) xstate: size: 0x340 and states: 0x7 which is a side effect of silo_xsm_ops only implementing a few of the hooks, falling back to dummy for the rest. Presumably we don't want to special case SILO when overriding the hooks. Would having silo_init() explicitly copy from dummy be ok? ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: XSM SILO boot time spew 2018-11-01 3:19 ` Xin Li (Talons) @ 2018-11-07 17:52 ` Daniel De Graaf 2018-11-08 3:03 ` Xin Li (Talons) 2018-11-08 10:05 ` Jan Beulich 0 siblings, 2 replies; 6+ messages in thread From: Daniel De Graaf @ 2018-11-07 17:52 UTC (permalink / raw) To: Xin Li (Talons), Andrew Cooper; +Cc: Xen-devel List On 10/31/2018 11:19 PM, Xin Li (Talons) wrote: > In patchset v4, we call register_xsm() to setup silo module. > This debug log is to check if some ops not overrided by the module. > I thought this is OK, since the log level is debug. > > I think calling register_xsm() is good, > if we do want to suppress this debug log explicitly, > we can check if ops eqauls silo_xsm_ops in macro set_to_dummy_if_null(). > > The follow diff shows what I am suggesting, is it OK? If SILO is a good example of what a potential third XSM module would look like, it's probably better to just remove the printing and allow the dummy module's hooks to fill in any null values in the xsm_operations structure. The printing part was written with FLASK and ACM in mind, which both intended to hook everything and might add new hooks without changing the other. Another possible solution would be to add a bool parameter to register_xsm that disables the warnings instead of checking the pointer value, but that feels like overkill to me; we still only have two XSM modules. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: XSM SILO boot time spew 2018-11-07 17:52 ` Daniel De Graaf @ 2018-11-08 3:03 ` Xin Li (Talons) 2018-11-08 10:05 ` Jan Beulich 1 sibling, 0 replies; 6+ messages in thread From: Xin Li (Talons) @ 2018-11-08 3:03 UTC (permalink / raw) To: Daniel De Graaf, Andrew Cooper; +Cc: Xen-devel List > -----Original Message----- > From: Daniel De Graaf [mailto:dgdegra@tycho.nsa.gov] > Sent: Thursday, November 8, 2018 1:53 AM > To: Xin Li (Talons) <xin.li@citrix.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com> > Cc: Xen-devel List <xen-devel@lists.xen.org> > Subject: Re: XSM SILO boot time spew > If SILO is a good example of what a potential third XSM module would look like, > it's probably better to just remove the printing and allow the dummy module's > hooks to fill in any null values in the xsm_operations structure. The printing > part was written with FLASK and ACM in mind, which both intended to hook > everything and might add new hooks without changing the other. > > Another possible solution would be to add a bool parameter to register_xsm > that disables the warnings instead of checking the pointer value, but that feels > like overkill to me; we still only have two XSM modules. OK. I will just remove the printing. Best regards Xin(Talons) Li _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: XSM SILO boot time spew 2018-11-07 17:52 ` Daniel De Graaf 2018-11-08 3:03 ` Xin Li (Talons) @ 2018-11-08 10:05 ` Jan Beulich 2018-11-08 10:13 ` Andrew Cooper 1 sibling, 1 reply; 6+ messages in thread From: Jan Beulich @ 2018-11-08 10:05 UTC (permalink / raw) To: Daniel de Graaf; +Cc: Andrew Cooper, Xen-devel List, Xin Li >>> On 07.11.18 at 18:52, <dgdegra@tycho.nsa.gov> wrote: > On 10/31/2018 11:19 PM, Xin Li (Talons) wrote: >> In patchset v4, we call register_xsm() to setup silo module. >> This debug log is to check if some ops not overrided by the module. >> I thought this is OK, since the log level is debug. >> >> I think calling register_xsm() is good, >> if we do want to suppress this debug log explicitly, >> we can check if ops eqauls silo_xsm_ops in macro set_to_dummy_if_null(). >> >> The follow diff shows what I am suggesting, is it OK? > > If SILO is a good example of what a potential third XSM module would look > like, it's probably better to just remove the printing and allow the > dummy module's hooks to fill in any null values in the xsm_operations > structure. The printing part was written with FLASK and ACM in mind, > which both intended to hook everything and might add new hooks without > changing the other. > > Another possible solution would be to add a bool parameter to register_xsm > that disables the warnings instead of checking the pointer value, but that > feels like overkill to me; we still only have two XSM modules. Why? Retaining the log message for the FLASK case seems quite desirable, and comparing pointers to special ops structures seems quite obviously worse to me. Of course a patch to drop the logging altogether was already posted, so you're free to ack that one and the discussion would be ended. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: XSM SILO boot time spew 2018-11-08 10:05 ` Jan Beulich @ 2018-11-08 10:13 ` Andrew Cooper 0 siblings, 0 replies; 6+ messages in thread From: Andrew Cooper @ 2018-11-08 10:13 UTC (permalink / raw) To: Jan Beulich, Daniel de Graaf; +Cc: Xin Li, Xen-devel List On 08/11/2018 10:05, Jan Beulich wrote: >>>> On 07.11.18 at 18:52, <dgdegra@tycho.nsa.gov> wrote: >> On 10/31/2018 11:19 PM, Xin Li (Talons) wrote: >>> In patchset v4, we call register_xsm() to setup silo module. >>> This debug log is to check if some ops not overrided by the module. >>> I thought this is OK, since the log level is debug. >>> >>> I think calling register_xsm() is good, >>> if we do want to suppress this debug log explicitly, >>> we can check if ops eqauls silo_xsm_ops in macro set_to_dummy_if_null(). >>> >>> The follow diff shows what I am suggesting, is it OK? >> If SILO is a good example of what a potential third XSM module would look >> like, it's probably better to just remove the printing and allow the >> dummy module's hooks to fill in any null values in the xsm_operations >> structure. The printing part was written with FLASK and ACM in mind, >> which both intended to hook everything and might add new hooks without >> changing the other. >> >> Another possible solution would be to add a bool parameter to register_xsm >> that disables the warnings instead of checking the pointer value, but that >> feels like overkill to me; we still only have two XSM modules. > Why? Retaining the log message for the FLASK case seems quite > desirable, and comparing pointers to special ops structures seems > quite obviously worse to me. Of course a patch to drop the logging > altogether was already posted, so you're free to ack that one and > the discussion would be ended. So I was thinking about this. I think I can arrange for the compiler to check the full-ness of the dummy and flask ops, and remove all of this runtime logic. This would be by having an interestingly typed sentinel at either end of the structure, then using a straight comma separated list of functions in the initialiser. Any change to the ops structure without a matching change in the flask and dummy ops will result in a function pointer type failure. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-08 10:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-10-31 12:35 XSM SILO boot time spew Andrew Cooper 2018-11-01 3:19 ` Xin Li (Talons) 2018-11-07 17:52 ` Daniel De Graaf 2018-11-08 3:03 ` Xin Li (Talons) 2018-11-08 10:05 ` Jan Beulich 2018-11-08 10:13 ` Andrew Cooper
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).