| Message ID | 20210805140644.357-7-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xsm: refactoring xsm hooks | expand |
On 05.08.2021 16:06, Daniel P. Smith wrote: > The internal define flag is not used by any XSM module, removing the #ifdef > leaving the generic event channel labeling as always present. With this description ... > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -120,15 +120,12 @@ struct evtchn > unsigned short notify_vcpu_id; /* VCPU for local delivery notification */ > uint32_t fifo_lastq; /* Data for identifying last queue. */ > > -#ifdef CONFIG_XSM > union { > -#ifdef XSM_NEED_GENERIC_EVTCHN_SSID > /* > * If an XSM module needs more space for its event channel context, > * this pointer stores the necessary data for the security server. > */ > void *generic; > -#endif > #ifdef CONFIG_XSM_FLASK > /* > * Inlining the contents of the structure for FLASK avoids unneeded > @@ -138,7 +135,6 @@ struct evtchn > uint32_t flask_sid; > #endif > } ssid; > -#endif > } __attribute__((aligned(64))); ... I can see the inner #ifdef go away, but not the outer one. While the (imo bogus) attribute means you don't alter the size of the struct, I'm afraid that's not obvious at all without counting bits and bytes, and hence this may also want saying explicitly in the description. Jan
On 8/25/21 11:44 AM, Jan Beulich wrote: > On 05.08.2021 16:06, Daniel P. Smith wrote: >> The internal define flag is not used by any XSM module, removing the #ifdef >> leaving the generic event channel labeling as always present. > > With this description ... > >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -120,15 +120,12 @@ struct evtchn >> unsigned short notify_vcpu_id; /* VCPU for local delivery notification */ >> uint32_t fifo_lastq; /* Data for identifying last queue. */ >> >> -#ifdef CONFIG_XSM >> union { >> -#ifdef XSM_NEED_GENERIC_EVTCHN_SSID >> /* >> * If an XSM module needs more space for its event channel context, >> * this pointer stores the necessary data for the security server. >> */ >> void *generic; >> -#endif >> #ifdef CONFIG_XSM_FLASK >> /* >> * Inlining the contents of the structure for FLASK avoids unneeded >> @@ -138,7 +135,6 @@ struct evtchn >> uint32_t flask_sid; >> #endif >> } ssid; >> -#endif >> } __attribute__((aligned(64))); > > ... I can see the inner #ifdef go away, but not the outer one. While > the (imo bogus) attribute means you don't alter the size of the > struct, I'm afraid that's not obvious at all without counting bits > and bytes, and hence this may also want saying explicitly in the > description. I can put the #ifdef CONFIG_XSM back and in the subsequent patch change it to CONFIG_XSM_CONFIGURABLE, making this the only difference between being able to select the XSM policy in effect or not. v/r, dps
On 27.08.2021 16:16, Daniel P. Smith wrote: > On 8/25/21 11:44 AM, Jan Beulich wrote: >> On 05.08.2021 16:06, Daniel P. Smith wrote: >>> The internal define flag is not used by any XSM module, removing the #ifdef >>> leaving the generic event channel labeling as always present. >> >> With this description ... >> >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -120,15 +120,12 @@ struct evtchn >>> unsigned short notify_vcpu_id; /* VCPU for local delivery notification */ >>> uint32_t fifo_lastq; /* Data for identifying last queue. */ >>> >>> -#ifdef CONFIG_XSM >>> union { >>> -#ifdef XSM_NEED_GENERIC_EVTCHN_SSID >>> /* >>> * If an XSM module needs more space for its event channel context, >>> * this pointer stores the necessary data for the security server. >>> */ >>> void *generic; >>> -#endif >>> #ifdef CONFIG_XSM_FLASK >>> /* >>> * Inlining the contents of the structure for FLASK avoids unneeded >>> @@ -138,7 +135,6 @@ struct evtchn >>> uint32_t flask_sid; >>> #endif >>> } ssid; >>> -#endif >>> } __attribute__((aligned(64))); >> >> ... I can see the inner #ifdef go away, but not the outer one. While >> the (imo bogus) attribute means you don't alter the size of the >> struct, I'm afraid that's not obvious at all without counting bits >> and bytes, and hence this may also want saying explicitly in the >> description. > > I can put the #ifdef CONFIG_XSM back and in the subsequent patch change > it to CONFIG_XSM_CONFIGURABLE, making this the only difference between > being able to select the XSM policy in effect or not. Well, yes, that's one way of addressing my comment. Another would be to actually justify the removal. (Personally I'm in favor of the former.) Jan
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 28146ee404..b089324510 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -120,15 +120,12 @@ struct evtchn unsigned short notify_vcpu_id; /* VCPU for local delivery notification */ uint32_t fifo_lastq; /* Data for identifying last queue. */ -#ifdef CONFIG_XSM union { -#ifdef XSM_NEED_GENERIC_EVTCHN_SSID /* * If an XSM module needs more space for its event channel context, * this pointer stores the necessary data for the security server. */ void *generic; -#endif #ifdef CONFIG_XSM_FLASK /* * Inlining the contents of the structure for FLASK avoids unneeded @@ -138,7 +135,6 @@ struct evtchn uint32_t flask_sid; #endif } ssid; -#endif } __attribute__((aligned(64))); int evtchn_init(struct domain *d, unsigned int max_port);
The internal define flag is not used by any XSM module, removing the #ifdef leaving the generic event channel labeling as always present. Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/include/xen/sched.h | 4 ---- 1 file changed, 4 deletions(-)