[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM



On Fri, 2013-01-11 at 17:37 +0000, Daniel De Graaf wrote:
> On 01/11/2013 11:46 AM, Ian Campbell wrote:
> > On Fri, 2013-01-11 at 16:36 +0000, Keir Fraser wrote:
> >> On 11/01/2013 16:24, "Lars Rasmusson" <lra@xxxxxxx> wrote:
> >>
> >>>> On 11/01/2013 13:32, "lra@xxxxxxx" <lra@xxxxxxx> wrote:
> >>>>
> >>>>> From: Lars Rasmusson <Lars.Rasmusson@xxxxxxx>
> >>>>>
> >>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@xxxxxxx>
> >>>>
> >>>> If this is a build fix after my checkins this morning then:
> >>>> Acked-by: Keir Fraser <keir@xxxxxxx>
> >>>
> >>> Yes, the XEN_TARGET_ARCH=arm32 make  breaks when compiling memory.c
> >>>
> >>> In xen/include/xsm/dummy.h where many of the functions are used, some are
> >>> declared only for X86, so I picked the same #ifdef CONFIG_X86
> >>> as the header file uses.
> >>>
> >>> As Ian said, it's not pretty, but since ARM doesn't have xsm (yet?) I 
> >>> think
> >>> adding a dummy xsm_remove_from_physmap for arm also is ugly.
> >>>
> >>> Is there some other way to write memory.c so that it doesn't need
> >>> xsm_remove...?  (I mean, it does't need xsm_add....)
> >>
> >> The XSM infrastructure is not architecture dependent. It's probably a
> >> mistake that xsm_remove_from_physmap() is ifdef CONFIG_X86.
> > 
> > Agreed.
> > 
> > 8<---------------------
> > 
> >>From fb57be285e956cadea51dc48a28fba77d752044d Mon Sep 17 00:00:00 2001
> > From: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Date: Fri, 11 Jan 2013 16:44:14 +0000
> > Subject: [PATCH] xen arm: add XSM hooks to arch_memory_op
> > 
> > Treat XENMEM_add_to_physmap_range the same as
> > XENMEM_add_to_physmap.
> > 
> > Reported-by: Lars Rasmusson <Lars.Rasmusson@xxxxxxx>
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> > ---
> >  xen/arch/arm/mm.c       |   13 +++++++++++++
> >  xen/include/xsm/dummy.h |   24 ++++++++++++------------
> >  2 files changed, 25 insertions(+), 12 deletions(-)
> 
> I agree with the movement (neither of these hooks are x86-specific), but
> this patch also needs to move the hooks in xen/include/xsm/xsm.h and
> xen/xsm/flask/hooks.c out of the same #ifdefs.

Thanks, and dummy.c.

Turns out this is far from the only barrier to enabling FLASK on ARM,
flask_op.c has a few x86isms not included in #ifdef, e.g. MSIs
I also needed a few extra #includes on ARM for things which are picked
up indirectly on x86.

The patch is below, its a bit ugly and totally untested. I could pick
out the bits to do the rigth thing on ARM without FLASK on if that would
be preferred.

> On a mostly unrelated note, Ian: do you think it would be useful to add 
> build tests with XSM enabled to catch these types of issues during the
> automated testing?  It just requires "echo XSM_ENABLE=y > .config"
> before the build, and is apparently best done for both x86 and arm.

In general I think build tests with a variety of different compile time
options, xsm being one of them, would be very nice to have. e.g. I'd
also like to get at least a build test of ARM into the system prior to
suitable hardware arriving later in the year.

Perhaps a flight of build tests could be added?

Ian.

8<----------------

>From 469d20054b96b8bc749891c366659532b67d6031 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Fri, 11 Jan 2013 16:44:14 +0000
Subject: [PATCH] xen arm: add XSM hooks to arch_memory_op

Treat XENMEM_add_to_physmap_range the same as XENMEM_add_to_physmap.

Also conditionalise more x86-isms and add required headers such that
it compiles on ARM. Totally untested.

Reported-by: Lars Rasmusson <Lars.Rasmusson@xxxxxxx>
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
 xen/arch/arm/mm.c        |   13 +++++++++++++
 xen/include/xsm/dummy.h  |   25 +++++++++++++------------
 xen/include/xsm/xsm.h    |   12 ++++++------
 xen/xsm/dummy.c          |    5 +++--
 xen/xsm/flask/avc.c      |    1 +
 xen/xsm/flask/flask_op.c |    4 +++-
 xen/xsm/flask/hooks.c    |   44 ++++++++++++++++++++++++++++++--------------
 xen/xsm/xsm_policy.c     |    1 +
 8 files changed, 70 insertions(+), 35 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index d97b3ea..311ec97 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -35,6 +35,7 @@
 #include <asm/current.h>
 #include <public/memory.h>
 #include <xen/sched.h>
+#include <xsm/xsm.h>
 
 struct domain *dom_xen, *dom_io, *dom_cow;
 
@@ -651,6 +652,12 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) 
arg)
         if ( rc != 0 )
             return rc;
 
+        if ( xsm_add_to_physmap(XSM_TARGET, current->domain, d) )
+        {
+            rcu_unlock_domain(d);
+            return -EPERM;
+        }
+
         rc = xenmem_add_to_physmap_one(d, xatp.space, DOMID_INVALID,
                                        xatp.idx, xatp.gpfn);
 
@@ -671,6 +678,12 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) 
arg)
         if ( rc != 0 )
             return rc;
 
+        if ( xsm_add_to_physmap(XSM_TARGET, current->domain, d) )
+        {
+            rcu_unlock_domain(d);
+            return -EPERM;
+        }
+
         rc = xenmem_add_to_physmap_range(d, &xatpr);
 
         rcu_unlock_domain(d);
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 1ca82b0..f40b196 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -16,6 +16,7 @@
  */
 
 #include <xen/sched.h>
+#include <xen/errno.h>
 #include <xsm/xsm.h>
 
 /* Cannot use BUILD_BUG_ON here because the expressions we check are not
@@ -443,6 +444,18 @@ static XSM_INLINE int 
xsm_pci_config_permission(XSM_DEFAULT_ARG struct domain *d
     return xsm_default_action(action, current->domain, d);
 }
 
+static XSM_INLINE int xsm_add_to_physmap(XSM_DEFAULT_ARG struct domain *d1, 
struct domain *d2)
+{
+    XSM_ASSERT_ACTION(XSM_TARGET);
+    return xsm_default_action(action, d1, d2);
+}
+
+static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain 
*d1, struct domain *d2)
+{
+    XSM_ASSERT_ACTION(XSM_TARGET);
+    return xsm_default_action(action, d1, d2);
+}
+
 #ifdef CONFIG_X86
 static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, 
uint32_t op)
 {
@@ -544,18 +557,6 @@ static XSM_INLINE int 
xsm_update_va_mapping(XSM_DEFAULT_ARG struct domain *d, st
     return xsm_default_action(action, d, f);
 }
 
-static XSM_INLINE int xsm_add_to_physmap(XSM_DEFAULT_ARG struct domain *d1, 
struct domain *d2)
-{
-    XSM_ASSERT_ACTION(XSM_TARGET);
-    return xsm_default_action(action, d1, d2);
-}
-
-static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain 
*d1, struct domain *d2)
-{
-    XSM_ASSERT_ACTION(XSM_TARGET);
-    return xsm_default_action(action, d1, d2);
-}
-
 static XSM_INLINE int xsm_bind_pt_irq(XSM_DEFAULT_ARG struct domain *d, struct 
xen_domctl_bind_pt_irq *bind)
 {
     XSM_ASSERT_ACTION(XSM_HOOK);
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 8947372..5048344 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -90,6 +90,7 @@ struct xsm_operations {
     int (*memory_adjust_reservation) (struct domain *d1, struct domain *d2);
     int (*memory_stat_reservation) (struct domain *d1, struct domain *d2);
     int (*memory_pin_page) (struct domain *d1, struct domain *d2, struct 
page_info *page);
+    int (*add_to_physmap) (struct domain *d1, struct domain *d2);
     int (*remove_from_physmap) (struct domain *d1, struct domain *d2);
 
     int (*console_io) (struct domain *d, int cmd);
@@ -149,7 +150,6 @@ struct xsm_operations {
                        struct domain *f, uint32_t flags);
     int (*mmuext_op) (struct domain *d, struct domain *f);
     int (*update_va_mapping) (struct domain *d, struct domain *f, l1_pgentry_t 
pte);
-    int (*add_to_physmap) (struct domain *d1, struct domain *d2);
     int (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind);
     int (*unbind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq 
*bind);
     int (*ioport_permission) (struct domain *d, uint32_t s, uint32_t e, 
uint8_t allow);
@@ -335,6 +335,11 @@ static inline int xsm_memory_pin_page(xsm_default_t def, 
struct domain *d1, stru
     return xsm_ops->memory_pin_page(d1, d2, page);
 }
 
+static inline int xsm_add_to_physmap(xsm_default_t def, struct domain *d1, 
struct domain *d2)
+{
+    return xsm_ops->add_to_physmap(d1, d2);
+}
+
 static inline int xsm_remove_from_physmap(xsm_default_t def, struct domain 
*d1, struct domain *d2)
 {
     return xsm_ops->remove_from_physmap(d1, d2);
@@ -558,11 +563,6 @@ static inline int xsm_update_va_mapping(xsm_default_t def, 
struct domain *d, str
     return xsm_ops->update_va_mapping(d, f, pte);
 }
 
-static inline int xsm_add_to_physmap(xsm_default_t def, struct domain *d1, 
struct domain *d2)
-{
-    return xsm_ops->add_to_physmap(d1, d2);
-}
-
 static inline int xsm_bind_pt_irq(xsm_default_t def, struct domain *d,
                                                 struct xen_domctl_bind_pt_irq 
*bind)
 {
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 529a724..5031e16 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -101,6 +101,9 @@ void xsm_fixup_ops (struct xsm_operations *ops)
 
     set_to_dummy_if_null(ops, do_xsm_op);
 
+    set_to_dummy_if_null(ops, add_to_physmap);
+    set_to_dummy_if_null(ops, remove_from_physmap);
+
 #ifdef CONFIG_X86
     set_to_dummy_if_null(ops, shadow_control);
     set_to_dummy_if_null(ops, hvm_param);
@@ -118,8 +121,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, mmu_update);
     set_to_dummy_if_null(ops, mmuext_op);
     set_to_dummy_if_null(ops, update_va_mapping);
-    set_to_dummy_if_null(ops, add_to_physmap);
-    set_to_dummy_if_null(ops, remove_from_physmap);
     set_to_dummy_if_null(ops, bind_pt_irq);
     set_to_dummy_if_null(ops, unbind_pt_irq);
     set_to_dummy_if_null(ops, ioport_permission);
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index 7fede00..f994fd9 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -28,6 +28,7 @@
 #include <xen/rcupdate.h>
 #include <asm/atomic.h>
 #include <asm/current.h>
+#include <public/event_channel.h>
 #include <public/xsm/flask_op.h>
 
 #include "avc.h"
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 4426ab9..f9b16f2 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -10,9 +10,11 @@
 
 #include <xen/errno.h>
 #include <xen/event.h>
+#include <xen/init.h>
 #include <xsm/xsm.h>
 #include <xen/guest_access.h>
 
+#include <public/event_channel.h>
 #include <public/xsm/flask_op.h>
 
 #include <avc.h>
@@ -71,7 +73,7 @@ static int domain_has_security(struct domain *d, u32 perms)
                         perms, NULL);
 }
 
-static int flask_copyin_string(XEN_GUEST_HANDLE_PARAM(char) u_buf, char **buf, 
uint32_t size)
+static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf, 
uint32_t size)
 {
     char *tmp = xmalloc_bytes(size + 1);
     if ( !tmp )
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index ba67502..c39f129 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -19,11 +19,15 @@
 #include <xen/errno.h>
 #include <xen/guest_access.h>
 #include <xen/xenoprof.h>
+#ifdef CONFIG_X86
 #include <asm/msi.h>
+#endif
+#include <asm/irq.h>
 #include <public/xen.h>
 #include <public/physdev.h>
 #include <public/platform.h>
 
+#include <public/event_channel.h>
 #include <public/xsm/flask_op.h>
 
 #include <avc.h>
@@ -100,7 +104,9 @@ static int domain_has_xen(struct domain *d, u32 perms)
 
 static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
 {
+#ifdef CONFIG_X86
     struct irq_desc *desc = irq_to_desc(irq);
+#endif
     if ( irq >= nr_irqs || irq < 0 )
         return -EINVAL;
     if ( irq < nr_static_irqs ) {
@@ -110,6 +116,7 @@ static int get_irq_sid(int irq, u32 *sid, struct 
avc_audit_data *ad)
         }
         return security_irq_sid(irq, sid);
     }
+#ifdef CONFIG_X86
     if ( desc->msi_desc ) {
         struct pci_dev *dev = desc->msi_desc->dev;
         u32 sbdf = (dev->seg << 16) | (dev->bus << 8) | dev->devfn;
@@ -119,6 +126,7 @@ static int get_irq_sid(int irq, u32 *sid, struct 
avc_audit_data *ad)
         }
         return security_device_sid(sbdf, sid);
     }
+#endif
     if (ad) {
         AVC_AUDIT_DATA_INIT(ad, IRQ);
         ad->irq = irq;
@@ -822,7 +830,9 @@ static int flask_map_domain_pirq (struct domain *d, int 
irq, void *data)
 {
     u32 sid, dsid;
     int rc = -EPERM;
+#ifdef CONFIG_X86
     struct msi_info *msi = data;
+#endif
     struct avc_audit_data ad;
 
     rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
@@ -830,12 +840,17 @@ static int flask_map_domain_pirq (struct domain *d, int 
irq, void *data)
     if ( rc )
         return rc;
 
-    if ( irq >= nr_static_irqs && msi ) {
+#ifdef CONFIG_X86
+    if ( irq >= nr_static_irqs && msi )
+    {
         u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
         AVC_AUDIT_DATA_INIT(&ad, DEV);
         ad.device = machine_bdf;
         rc = security_device_sid(machine_bdf, &sid);
-    } else {
+    }
+    else
+#endif
+    {
         rc = get_irq_sid(irq, &sid, &ad);
     }
     if ( rc )
@@ -1055,6 +1070,16 @@ static inline int flask_tmem_control(void)
     return domain_has_xen(current->domain, XEN__TMEM_CONTROL);
 }
 
+static int flask_add_to_physmap(struct domain *d1, struct domain *d2)
+{
+    return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
+}
+
+static int flask_remove_from_physmap(struct domain *d1, struct domain *d2)
+{
+    return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
+}
+
 #ifdef CONFIG_X86
 static int flask_shadow_control(struct domain *d, uint32_t op)
 {
@@ -1325,16 +1350,6 @@ static int flask_update_va_mapping(struct domain *d, 
struct domain *f,
     return domain_has_perm(d, f, SECCLASS_MMU, map_perms);
 }
 
-static int flask_add_to_physmap(struct domain *d1, struct domain *d2)
-{
-    return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
-}
-
-static int flask_remove_from_physmap(struct domain *d1, struct domain *d2)
-{
-    return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP);
-}
-
 static int flask_get_device_group(uint32_t machine_bdf)
 {
     u32 rsid;
@@ -1501,6 +1516,9 @@ static struct xsm_operations flask_ops = {
 
     .do_xsm_op = do_flask_op,
 
+    .add_to_physmap = flask_add_to_physmap,
+    .remove_from_physmap = flask_remove_from_physmap,
+
 #ifdef CONFIG_X86
     .shadow_control = flask_shadow_control,
     .hvm_param = flask_hvm_param,
@@ -1518,8 +1536,6 @@ static struct xsm_operations flask_ops = {
     .mmu_update = flask_mmu_update,
     .mmuext_op = flask_mmuext_op,
     .update_va_mapping = flask_update_va_mapping,
-    .add_to_physmap = flask_add_to_physmap,
-    .remove_from_physmap = flask_remove_from_physmap,
     .get_device_group = flask_get_device_group,
     .test_assign_device = flask_test_assign_device,
     .assign_device = flask_assign_device,
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index a419cf4..c1a3fc9 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -20,6 +20,7 @@
 
 #include <xsm/xsm.h>
 #include <xen/multiboot.h>
+#include <xen/init.h>
 #include <asm/bitops.h>
 
 char *__initdata policy_buffer = NULL;
-- 
1.7.9.1




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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.