[PATCH v2 1/2] xen/xsm: Reduce compiler command line clutter

[PATCH v2 1/2] xen/xsm: Reduce compiler command line clutter

[Daniel De Graaf]

Move the preprocessor definitions for all FLASK parameters other than
the enable flag off the compiler command line and into config.h, which
is the preferred location for such definitions.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>

---
Changes from v1: Leave FLASK_ENABLE on the command line to retain the
ability to turn off FLASK while leaving XSM enabled.

 xen/Rules.mk             |  3 +--
 xen/include/xen/config.h | 10 ++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3a6cec5..42c713f 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -47,8 +47,7 @@ CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
 CFLAGS += -nostdinc
 
 CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
-CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
-CFLAGS-$(FLASK_ENABLE)  += -DFLASK_DEVELOP -DFLASK_BOOTPARAM -DFLASK_AVC_STATS
+CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE
 CFLAGS-$(verbose)       += -DVERBOSE
 CFLAGS-$(crash_debug)   += -DCRASH_DEBUG
 CFLAGS-$(perfc)         += -DPERF_COUNTERS
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 8bae6e6..7bef8a6 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -98,4 +98,14 @@
 #define __cpuinitdata
 #define __cpuinit
 
+#ifdef FLASK_ENABLE
+#define XSM_MAGIC 0xf97cff8c
+/* Enable permissive mode (xl setenforce or flask_enforcing parameter) */
+#define FLASK_DEVELOP 1
+/* Allow runtime disabling of FLASK via the flask_enable parameter */
+#define FLASK_BOOTPARAM 1
+/* Maintain statistics on the access vector cache */
+#define FLASK_AVC_STATS 1
+#endif
+
 #endif /* __XEN_CONFIG_H__ */
-- 
1.8.5.3

[PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Daniel De Graaf]

When FLASK is the only enabled implementation of the XSM hooks in Xen,
some of the abstractions required to handle multiple XSM providers are
redundant and only produce unneeded overhead.  This patch reduces the
memory overhead of enabling XSM on event channels by replacing the
untyped ssid pointer from struct evtchn with a union containing the
contents of the structure.  This avoids an additional heap allocation
for every event channel, and on 64-bit systems, reduces the size of
struct evtchn by 4 bytes.  If an out-of-tree XSM module needs the full
flexibility of the generic evtcnn ssid pointer, defining the symbol
XSM_NEED_GENERIC_EVTCHN_SSID will include a suitable pointer field.

This also cleans up the unused selinux_checkreqprot declaration left
from the Linux port.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
---
Changes from v1:
 - Enclose the security server fields in a union to make supporting
   additional XSM modules that use this field less intrusive.

 xen/include/xen/sched.h        | 16 +++++++++++++++-
 xen/xsm/flask/hooks.c          | 37 ++++++-------------------------------
 xen/xsm/flask/include/objsec.h |  6 ------
 3 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 00f0eba..d087e43 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -100,8 +100,22 @@ struct evtchn
     u8 pending:1;
     u16 last_vcpu_id;
     u8 last_priority;
+#ifdef XSM_ENABLE
+    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 FLASK_ENABLE
-    void *ssid;
+        /* Inlining the contents of the structure for FLASK avoids unneeded
+         * allocations, and on 64-bit platforms with only FLASK enabled,
+         * reduces the size of struct evtchn.
+         */
+        u32 flask_sid;
+#endif
+    } ssid;
 #endif
 };
 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 96276ac..1868922 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -52,8 +52,7 @@ static u32 domain_target_sid(struct domain *src, struct domain *dst)
 
 static u32 evtchn_sid(const struct evtchn *chn)
 {
-    struct evtchn_security_struct *esec = chn->ssid;
-    return esec->sid;
+    return chn->ssid.flask_sid;
 }
 
 static int domain_has_perm(struct domain *dom1, struct domain *dom2, 
@@ -174,7 +173,6 @@ static int flask_evtchn_unbound(struct domain *d1, struct evtchn *chn,
     u32 sid1, sid2, newsid;
     int rc;
     struct domain *d2;
-    struct evtchn_security_struct *esec;
 
     d2 = rcu_lock_domain_by_any_id(id2);
     if ( d2 == NULL )
@@ -182,7 +180,6 @@ static int flask_evtchn_unbound(struct domain *d1, struct evtchn *chn,
 
     sid1 = domain_sid(d1);
     sid2 = domain_target_sid(d1, d2);
-    esec = chn->ssid;
 
     rc = security_transition_sid(sid1, sid2, SECCLASS_EVENT, &newsid);
     if ( rc )
@@ -196,7 +193,7 @@ static int flask_evtchn_unbound(struct domain *d1, struct evtchn *chn,
     if ( rc )
         goto out;
 
-    esec->sid = newsid;
+    chn->ssid.flask_sid = newsid;
 
  out:
     rcu_unlock_domain(d2);
@@ -208,7 +205,6 @@ static int flask_evtchn_interdomain(struct domain *d1, struct evtchn *chn1,
 {
     u32 sid1, sid2, newsid, reverse_sid;
     int rc;
-    struct evtchn_security_struct *esec1;
     struct avc_audit_data ad;
     AVC_AUDIT_DATA_INIT(&ad, NONE);
     ad.sdom = d1;
@@ -217,8 +213,6 @@ static int flask_evtchn_interdomain(struct domain *d1, struct evtchn *chn1,
     sid1 = domain_sid(d1);
     sid2 = domain_target_sid(d1, d2);
 
-    esec1 = chn1->ssid;
-
     rc = security_transition_sid(sid1, sid2, SECCLASS_EVENT, &newsid);
     if ( rc )
     {
@@ -244,17 +238,14 @@ static int flask_evtchn_interdomain(struct domain *d1, struct evtchn *chn1,
     if ( rc )
         return rc;
 
-    esec1->sid = newsid;
+    chn1->ssid.flask_sid = newsid;
 
     return rc;
 }
 
 static void flask_evtchn_close_post(struct evtchn *chn)
 {
-    struct evtchn_security_struct *esec;
-    esec = chn->ssid;
-
-    esec->sid = SECINITSID_UNLABELED;
+    chn->ssid.flask_sid = SECINITSID_UNLABELED;
 }
 
 static int flask_evtchn_send(struct domain *d, struct evtchn *chn)
@@ -289,33 +280,17 @@ static int flask_evtchn_reset(struct domain *d1, struct domain *d2)
 
 static int flask_alloc_security_evtchn(struct evtchn *chn)
 {
-    struct evtchn_security_struct *esec;
-
-    esec = xzalloc(struct evtchn_security_struct);
-    if ( !esec )
-        return -ENOMEM;
-
-    esec->sid = SECINITSID_UNLABELED;
-
-    chn->ssid = esec;
+    chn->ssid.flask_sid = SECINITSID_UNLABELED;
 
     return 0;    
 }
 
 static void flask_free_security_evtchn(struct evtchn *chn)
 {
-    struct evtchn_security_struct *esec;
-
     if ( !chn )
         return;
 
-    esec = chn->ssid;
-
-    if ( !esec )
-        return;
-
-    chn->ssid = NULL;
-    xfree(esec);
+    chn->ssid.flask_sid = SECINITSID_UNLABELED;
 }
 
 static char *flask_show_security_evtchn(struct domain *d, const struct evtchn *chn)
diff --git a/xen/xsm/flask/include/objsec.h b/xen/xsm/flask/include/objsec.h
index 6595dc3..b576a5d 100644
--- a/xen/xsm/flask/include/objsec.h
+++ b/xen/xsm/flask/include/objsec.h
@@ -23,10 +23,4 @@ struct domain_security_struct {
     u32 target_sid;        /* SID for device model target domain */
 };
 
-struct evtchn_security_struct {
-    u32 sid;                 /* current SID */
-};
-
-extern unsigned int selinux_checkreqprot;
-
 #endif /* _FLASK_OBJSEC_H_ */
-- 
1.8.5.3

Re: [PATCH v2 1/2] xen/xsm: Reduce compiler command line clutter

[Jan Beulich]

>>> On 20.03.14 at 16:29, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> Move the preprocessor definitions for all FLASK parameters other than
> the enable flag off the compiler command line and into config.h, which
> is the preferred location for such definitions.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> Cc: Keir Fraser <keir@xen.org>
> 
> ---
> Changes from v1: Leave FLASK_ENABLE on the command line to retain the
> ability to turn off FLASK while leaving XSM enabled.
> 
>  xen/Rules.mk             |  3 +--
>  xen/include/xen/config.h | 10 ++++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 3a6cec5..42c713f 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -47,8 +47,7 @@ CFLAGS += -pipe -g -D__XEN__ -include 
> $(BASEDIR)/include/xen/config.h
>  CFLAGS += -nostdinc
>  
>  CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
> -CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
> -CFLAGS-$(FLASK_ENABLE)  += -DFLASK_DEVELOP -DFLASK_BOOTPARAM -DFLASK_AVC_STATS
> +CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE
>  CFLAGS-$(verbose)       += -DVERBOSE
>  CFLAGS-$(crash_debug)   += -DCRASH_DEBUG
>  CFLAGS-$(perfc)         += -DPERF_COUNTERS
> diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
> index 8bae6e6..7bef8a6 100644
> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -98,4 +98,14 @@
>  #define __cpuinitdata
>  #define __cpuinit
>  
> +#ifdef FLASK_ENABLE
> +#define XSM_MAGIC 0xf97cff8c
> +/* Enable permissive mode (xl setenforce or flask_enforcing parameter) */
> +#define FLASK_DEVELOP 1
> +/* Allow runtime disabling of FLASK via the flask_enable parameter */
> +#define FLASK_BOOTPARAM 1
> +/* Maintain statistics on the access vector cache */
> +#define FLASK_AVC_STATS 1
> +#endif
> +
>  #endif /* __XEN_CONFIG_H__ */
> -- 
> 1.8.5.3

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Jan Beulich]

>>> On 20.03.14 at 16:29, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> When FLASK is the only enabled implementation of the XSM hooks in Xen,
> some of the abstractions required to handle multiple XSM providers are
> redundant and only produce unneeded overhead.  This patch reduces the
> memory overhead of enabling XSM on event channels by replacing the
> untyped ssid pointer from struct evtchn with a union containing the
> contents of the structure.  This avoids an additional heap allocation
> for every event channel, and on 64-bit systems, reduces the size of
> struct evtchn by 4 bytes.  If an out-of-tree XSM module needs the full
> flexibility of the generic evtcnn ssid pointer, defining the symbol
> XSM_NEED_GENERIC_EVTCHN_SSID will include a suitable pointer field.
> 
> This also cleans up the unused selinux_checkreqprot declaration left
> from the Linux port.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> ---
> Changes from v1:
>  - Enclose the security server fields in a union to make supporting
>    additional XSM modules that use this field less intrusive.
> 
>  xen/include/xen/sched.h        | 16 +++++++++++++++-
>  xen/xsm/flask/hooks.c          | 37 ++++++-------------------------------
>  xen/xsm/flask/include/objsec.h |  6 ------
>  3 files changed, 21 insertions(+), 38 deletions(-)
> 
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 00f0eba..d087e43 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -100,8 +100,22 @@ struct evtchn
>      u8 pending:1;
>      u16 last_vcpu_id;
>      u8 last_priority;
> +#ifdef XSM_ENABLE
> +    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;

Coding style - both for the comment and the placement of the *
right above. But I think if I'm the one to commit this (pending
Keir's ack) I could take care of these if no other comments show
up that would make another revision necessary.

> +#endif
>  #ifdef FLASK_ENABLE
> -    void *ssid;
> +        /* Inlining the contents of the structure for FLASK avoids unneeded
> +         * allocations, and on 64-bit platforms with only FLASK enabled,
> +         * reduces the size of struct evtchn.
> +         */

Again.

With those adjustments

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[David Vrabel]

On 20/03/14 15:29, Daniel De Graaf wrote:
> When FLASK is the only enabled implementation of the XSM hooks in Xen,
> some of the abstractions required to handle multiple XSM providers are
> redundant and only produce unneeded overhead.  This patch reduces the
> memory overhead of enabling XSM on event channels by replacing the
> untyped ssid pointer from struct evtchn with a union containing the
> contents of the structure.  This avoids an additional heap allocation
> for every event channel, and on 64-bit systems, reduces the size of
> struct evtchn by 4 bytes.

Without XSM the 64-bit structure is 29 bytes (so 32 including the
trailing padding).

Adding a 4 byte word or a 8 byte word both results in a 40 byte
structure which halves the number of evtchns per page (since each page
contains a power of two structs).

I think you could swap the order of the fields in u.interdomain to get
better packing with the 4 byte flask_sid and end up with a 32 byte
struct evtchn (there's 6 bytes of padding between remote_port and
remote_dom).

You may want to check the EVTCHNS_PER_BUCKET value before/after.

David

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Daniel De Graaf]

On 03/21/2014 09:28 AM, David Vrabel wrote:
> On 20/03/14 15:29, Daniel De Graaf wrote:
>> When FLASK is the only enabled implementation of the XSM hooks in Xen,
>> some of the abstractions required to handle multiple XSM providers are
>> redundant and only produce unneeded overhead.  This patch reduces the
>> memory overhead of enabling XSM on event channels by replacing the
>> untyped ssid pointer from struct evtchn with a union containing the
>> contents of the structure.  This avoids an additional heap allocation
>> for every event channel, and on 64-bit systems, reduces the size of
>> struct evtchn by 4 bytes.
>
> Without XSM the 64-bit structure is 29 bytes (so 32 including the
> trailing padding).
>
> Adding a 4 byte word or a 8 byte word both results in a 40 byte
> structure which halves the number of evtchns per page (since each page
> contains a power of two structs).
>
> I think you could swap the order of the fields in u.interdomain to get
> better packing with the 4 byte flask_sid and end up with a 32 byte
> struct evtchn (there's 6 bytes of padding between remote_port and
> remote_dom).
>
> You may want to check the EVTCHNS_PER_BUCKET value before/after.
>
> David

Making this work correctly requires adding__packed to the interdomain
structure (otherwise, the padding just remains at the end); with this,
the structure is 24 bytes without XSM, 28 with FLASK only, and 32 with
XSM_NEED_GENERIC_EVTCHN_SSID (all with one byte of padding remaining).

Using __packed on the structure containing only 8-byte field makes
alignof(struct evtchn) == 4.  I believe this needs to be fixed by adding
an __attribute__((__aligned__(alignof(void*)))) to the structure; should
this be wrapped in some type of macro similar to __packed?

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Jan Beulich]

>>> On 21.03.14 at 15:54, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> On 03/21/2014 09:28 AM, David Vrabel wrote:
>> On 20/03/14 15:29, Daniel De Graaf wrote:
>>> When FLASK is the only enabled implementation of the XSM hooks in Xen,
>>> some of the abstractions required to handle multiple XSM providers are
>>> redundant and only produce unneeded overhead.  This patch reduces the
>>> memory overhead of enabling XSM on event channels by replacing the
>>> untyped ssid pointer from struct evtchn with a union containing the
>>> contents of the structure.  This avoids an additional heap allocation
>>> for every event channel, and on 64-bit systems, reduces the size of
>>> struct evtchn by 4 bytes.
>>
>> Without XSM the 64-bit structure is 29 bytes (so 32 including the
>> trailing padding).
>>
>> Adding a 4 byte word or a 8 byte word both results in a 40 byte
>> structure which halves the number of evtchns per page (since each page
>> contains a power of two structs).
>>
>> I think you could swap the order of the fields in u.interdomain to get
>> better packing with the 4 byte flask_sid and end up with a 32 byte
>> struct evtchn (there's 6 bytes of padding between remote_port and
>> remote_dom).
>>
>> You may want to check the EVTCHNS_PER_BUCKET value before/after.
> 
> Making this work correctly requires adding__packed to the interdomain
> structure (otherwise, the padding just remains at the end); with this,
> the structure is 24 bytes without XSM, 28 with FLASK only, and 32 with
> XSM_NEED_GENERIC_EVTCHN_SSID (all with one byte of padding remaining).
> 
> Using __packed on the structure containing only 8-byte field makes
> alignof(struct evtchn) == 4.  I believe this needs to be fixed by adding
> an __attribute__((__aligned__(alignof(void*)))) to the structure; should
> this be wrapped in some type of macro similar to __packed?

No - please no packed attributes on non-interface structures in
common code, as that risks alignment issues.

Jan

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[David Vrabel]

On 21/03/14 14:54, Daniel De Graaf wrote:
> On 03/21/2014 09:28 AM, David Vrabel wrote:
>> On 20/03/14 15:29, Daniel De Graaf wrote:
>>> When FLASK is the only enabled implementation of the XSM hooks in Xen,
>>> some of the abstractions required to handle multiple XSM providers are
>>> redundant and only produce unneeded overhead.  This patch reduces the
>>> memory overhead of enabling XSM on event channels by replacing the
>>> untyped ssid pointer from struct evtchn with a union containing the
>>> contents of the structure.  This avoids an additional heap allocation
>>> for every event channel, and on 64-bit systems, reduces the size of
>>> struct evtchn by 4 bytes.
>>
>> Without XSM the 64-bit structure is 29 bytes (so 32 including the
>> trailing padding).
>>
>> Adding a 4 byte word or a 8 byte word both results in a 40 byte
>> structure which halves the number of evtchns per page (since each page
>> contains a power of two structs).
>>
>> I think you could swap the order of the fields in u.interdomain to get
>> better packing with the 4 byte flask_sid and end up with a 32 byte
>> struct evtchn (there's 6 bytes of padding between remote_port and
>> remote_dom).
>>
>> You may want to check the EVTCHNS_PER_BUCKET value before/after.
>>
>> David
> 
> Making this work correctly requires adding__packed to the interdomain
> structure (otherwise, the padding just remains at the end); with this,
> the structure is 24 bytes without XSM, 28 with FLASK only, and 32 with
> XSM_NEED_GENERIC_EVTCHN_SSID (all with one byte of padding remaining).

Oh, now I remember why I didn't do this before.  Sorry for wasting your
time.

David

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Daniel De Graaf]

On 03/21/2014 11:16 AM, David Vrabel wrote:
> On 21/03/14 14:54, Daniel De Graaf wrote:
>> On 03/21/2014 09:28 AM, David Vrabel wrote:
>>> On 20/03/14 15:29, Daniel De Graaf wrote:
>>>> When FLASK is the only enabled implementation of the XSM hooks in Xen,
>>>> some of the abstractions required to handle multiple XSM providers are
>>>> redundant and only produce unneeded overhead.  This patch reduces the
>>>> memory overhead of enabling XSM on event channels by replacing the
>>>> untyped ssid pointer from struct evtchn with a union containing the
>>>> contents of the structure.  This avoids an additional heap allocation
>>>> for every event channel, and on 64-bit systems, reduces the size of
>>>> struct evtchn by 4 bytes.
>>>
>>> Without XSM the 64-bit structure is 29 bytes (so 32 including the
>>> trailing padding).
>>>
>>> Adding a 4 byte word or a 8 byte word both results in a 40 byte
>>> structure which halves the number of evtchns per page (since each page
>>> contains a power of two structs).
>>>
>>> I think you could swap the order of the fields in u.interdomain to get
>>> better packing with the 4 byte flask_sid and end up with a 32 byte
>>> struct evtchn (there's 6 bytes of padding between remote_port and
>>> remote_dom).
>>>
>>> You may want to check the EVTCHNS_PER_BUCKET value before/after.
>>>
>>> David
>>
>> Making this work correctly requires adding__packed to the interdomain
>> structure (otherwise, the padding just remains at the end); with this,
>> the structure is 24 bytes without XSM, 28 with FLASK only, and 32 with
>> XSM_NEED_GENERIC_EVTCHN_SSID (all with one byte of padding remaining).
>
> Oh, now I remember why I didn't do this before.  Sorry for wasting your
> time.
>
> David

I'm guessing that swapping the fields and then slipping the flask_sid
field into the padding as another member of the union would also be
frowned upon (it seems far more fragile than the __packed method).

The pending bit could be combined with xen_consumer, which currently uses
only 3 bits (as an index tothe xen_consumers[8] array). This  would make
the non-XSM struct evtchn take up 28 bytes when arranged properly, and
the FLASK-only version would then fit in 32.  It seems that xen_consumer
is used rarely enough that the overhead from it being a bitfield would
not become an issue; any opinions on this?

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Jan Beulich]

>>> On 21.03.14 at 18:43, <dgdegra@tycho.nsa.gov> wrote:
> I'm guessing that swapping the fields and then slipping the flask_sid
> field into the padding as another member of the union would also be
> frowned upon (it seems far more fragile than the __packed method).

How could this be a member of the union in the first place? Isn't
this field applicable to all kinds of event channels?

> The pending bit could be combined with xen_consumer, which currently uses
> only 3 bits (as an index tothe xen_consumers[8] array). This  would make
> the non-XSM struct evtchn take up 28 bytes when arranged properly, and
> the FLASK-only version would then fit in 32.  It seems that xen_consumer
> is used rarely enough that the overhead from it being a bitfield would
> not become an issue; any opinions on this?

That sounds reasonable to me, despite your calculation being not
really correct (even the non-XSM variant of the structure always
occupies 32 bytes, due to the necessary padding at the end in
order to meet the structure's 8-byte alignment requirement).

In fact there are further optimizations possible: For one,
xen_consumer is used actively only for interdomain channels (and
obviously passively for unbound ones), so could be moved into the
union with a little care. And then we could "pickle" remote_dom rather
than storing it as a pointer, shrinking the union from 16 to 8 bytes and
the overall structure to 20/24/32 bytes (non-XSM, FLASK-only, generic-
XSM). But that wouldn't be worthwhile due to EVTCHNS_PER_BUCKET()'s
use of next_power_of_2(), unless we needed to fit in further fields.

While looking at this I started to wonder if the 100k event channel
promise in 4.4 isn't broken: All the *_port fields here are 16-bit only,
and hence only good for around 64k ports. If they all got widened to
32 bits, the structure (with the improvements above) would become
24/28/32 bytes. Widening them without any other adjustments
(thinking of a backport thereof to the 4.4 branch) would appear to
retain the current size.

Jan

Re: [PATCH v2 1/2] xen/xsm: Reduce compiler command line clutter

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]

On Thu, Mar 20, 2014 at 3:29 PM, Daniel De Graaf <dgdegra@tycho.nsa.gov>wrote:

> Move the preprocessor definitions for all FLASK parameters other than
> the enable flag off the compiler command line and into config.h, which
> is the preferred location for such definitions.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
>

Acked-by: Keir Fraser <keir@xen.org>

[-- Attachment #1.2: Type: text/html, Size: 988 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 1750 bytes --]

On Thu, Mar 20, 2014 at 3:29 PM, Daniel De Graaf <dgdegra@tycho.nsa.gov>wrote:

> When FLASK is the only enabled implementation of the XSM hooks in Xen,
> some of the abstractions required to handle multiple XSM providers are
> redundant and only produce unneeded overhead.  This patch reduces the
> memory overhead of enabling XSM on event channels by replacing the
> untyped ssid pointer from struct evtchn with a union containing the
> contents of the structure.  This avoids an additional heap allocation
> for every event channel, and on 64-bit systems, reduces the size of
> struct evtchn by 4 bytes.  If an out-of-tree XSM module needs the full
> flexibility of the generic evtcnn ssid pointer, defining the symbol
> XSM_NEED_GENERIC_EVTCHN_SSID will include a suitable pointer field.
>
> This also cleans up the unused selinux_checkreqprot declaration left
> from the Linux port.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
>

Acked-by: Keir Fraser <keir@xen.org>


> ---
> Changes from v1:
>  - Enclose the security server fields in a union to make supporting
>    additional XSM modules that use this field less intrusive.
>
>  xen/include/xen/sched.h        | 16 +++++++++++++++-
>  xen/xsm/flask/hooks.c          | 37 ++++++-------------------------------
>  xen/xsm/flask/include/objsec.h |  6 ------
>  3 files changed, 21 insertions(+), 38 deletions(-)
>
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 00f0eba..d087e43 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -100,8 +100,22 @@ struct evtchn
>      u8 pending:1;
>      u16 last_vcpu_id;
>      u8 last_priority;
> +#ifdef XSM_ENABLE
>
>

[-- Attachment #1.2: Type: text/html, Size: 2526 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 2/2] xen/evtchn: optimize XSM ssid field

[David Vrabel]

On 24/03/14 08:38, Jan Beulich wrote:
>
> While looking at this I started to wonder if the 100k event channel
> promise in 4.4 isn't broken: All the *_port fields here are 16-bit only,
> and hence only good for around 64k ports. If they all got widened to
> 32 bits, the structure (with the improvements above) would become
> 24/28/32 bytes. Widening them without any other adjustments
> (thinking of a backport thereof to the 4.4 branch) would appear to
> retain the current size.

Oops.  We did test binding the full number of event channels but we
obviously didn't test that they actually worked properly.

David