[PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM

[PATCH] xsm_remove_from_physmap is (so far) only defined for X86 architecture, not for ARM

[lra]

From: Lars Rasmusson <Lars.Rasmusson@sics.se>

Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
---
 xen/common/memory.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index c8c1ef2..8230565 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -676,12 +676,13 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( rc != 0 )
             return rc;
 
+#ifdef CONFIG_X86
         if ( xsm_remove_from_physmap(current->domain, d) )
         {
             rcu_unlock_domain(d);
             return -EPERM;
         }
-
+#endif
         domain_lock(d);
 
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
-- 
1.7.9.5

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

[Ian Campbell]

On Fri, 2013-01-11 at 13:32 +0000, lra@sics.se wrote:
> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
> 
> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>

Thanks but I think the right approach here would be to add the necessary
stuff to ARM rather than skip the permissions checks on ARM.

Ian.

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

[Keir Fraser]

On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:

> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
> 
> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>

If this is a build fix after my checkins this morning then:
Acked-by: Keir Fraser <keir@xen.org>

> ---
>  xen/common/memory.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index c8c1ef2..8230565 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -676,12 +676,13 @@ long do_memory_op(unsigned long cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( rc != 0 )
>              return rc;
>  
> +#ifdef CONFIG_X86
>          if ( xsm_remove_from_physmap(current->domain, d) )
>          {
>              rcu_unlock_domain(d);
>              return -EPERM;
>          }
> -
> +#endif
>          domain_lock(d);
>  
>          page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);

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

[Lars Rasmusson]

On Jan 11, 2013, at 14:47 , Keir Fraser <keir@xen.org> wrote:

> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:
> 
>> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
>> 
>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
> 
> If this is a build fix after my checkins this morning then:
> Acked-by: Keir Fraser <keir@xen.org>

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....)


/Lars


> 
>> ---
>> xen/common/memory.c |    3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/xen/common/memory.c b/xen/common/memory.c
>> index c8c1ef2..8230565 100644
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -676,12 +676,13 @@ long do_memory_op(unsigned long cmd,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>         if ( rc != 0 )
>>             return rc;
>> 
>> +#ifdef CONFIG_X86
>>         if ( xsm_remove_from_physmap(current->domain, d) )
>>         {
>>             rcu_unlock_domain(d);
>>             return -EPERM;
>>         }
>> -
>> +#endif
>>         domain_lock(d);
>> 
>>         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
> 
> 

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

[Keir Fraser]

On 11/01/2013 16:24, "Lars Rasmusson" <lra@sics.se> wrote:

>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:
>> 
>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
>>> 
>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
>> 
>> If this is a build fix after my checkins this morning then:
>> Acked-by: Keir Fraser <keir@xen.org>
> 
> 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.

 -- Keir

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

[Ian Campbell]

On Fri, 2013-01-11 at 16:36 +0000, Keir Fraser wrote:
> On 11/01/2013 16:24, "Lars Rasmusson" <lra@sics.se> wrote:
> 
> >> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:
> >> 
> >>> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
> >>> 
> >>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
> >> 
> >> If this is a build fix after my checkins this morning then:
> >> Acked-by: Keir Fraser <keir@xen.org>
> > 
> > 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@citrix.com>
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@sics.se>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/arch/arm/mm.c       |   13 +++++++++++++
 xen/include/xsm/dummy.h |   24 ++++++++++++------------
 2 files changed, 25 insertions(+), 12 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..870bd67 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -443,6 +443,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 +556,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);
-- 
1.7.9.1

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

[Daniel De Graaf]

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@sics.se> wrote:
>>
>>>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:
>>>>
>>>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
>>>>>
>>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
>>>>
>>>> If this is a build fix after my checkins this morning then:
>>>> Acked-by: Keir Fraser <keir@xen.org>
>>>
>>> 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@citrix.com>
> 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@sics.se>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  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.

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.

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

[Ian Campbell]

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@sics.se> wrote:
> >>
> >>>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:
> >>>>
> >>>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
> >>>>>
> >>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
> >>>>
> >>>> If this is a build fix after my checkins this morning then:
> >>>> Acked-by: Keir Fraser <keir@xen.org>
> >>>
> >>> 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@citrix.com>
> > 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@sics.se>
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> > ---
> >  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@citrix.com>
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@sics.se>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 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

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

[Stefano Stabellini]

On Tue, 15 Jan 2013, Ian Campbell wrote:
> From 469d20054b96b8bc749891c366659532b67d6031 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> 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@sics.se>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

the patch looks mostly to me


> 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 )

this change is correct


> 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 )

this part is a bit ugly, can we refactor the msi checks into separate,
arch specific, functions?

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

[Ian Campbell]

On Tue, 2013-01-15 at 11:52 +0000, Stefano Stabellini wrote:
> > @@ -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 )
> 
> this part is a bit ugly, can we refactor the msi checks into separate,
> arch specific, functions?

Yes that would be better. I'll give Daniel a chance to respond to the
rest before I respin with this change.

Ian.

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

[Daniel De Graaf]

On 01/15/2013 06:18 AM, Ian Campbell wrote:
> 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@sics.se> wrote:
>>>>
>>>>>> On 11/01/2013 13:32, "lra@sics.se" <lra@sics.se> wrote:
>>>>>>
>>>>>>> From: Lars Rasmusson <Lars.Rasmusson@sics.se>
>>>>>>>
>>>>>>> Signed-off-by: Lars Rasmusson <Lars.Rasmusson@sics.se>
>>>>>>
>>>>>> If this is a build fix after my checkins this morning then:
>>>>>> Acked-by: Keir Fraser <keir@xen.org>
>>>>>
>>>>> 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@citrix.com>
>>> 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@sics.se>
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> ---
>>>  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@citrix.com>
> 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@sics.se>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  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;
> +        }
> +

Using the return value of the xsm hook instead of -EPERM is preferred; this turns
an -ENOMEM inside FLASK into an -EPERM (and same below).

>          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;
> 

The rest of the changes look correct. The #ifdefs are a bit ugly, but
refactoring the MSI code into an arch-specific function should fix that.

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

[Ian Campbell]

On Tue, 2013-01-15 at 14:35 +0000, Daniel De Graaf wrote:
> The rest of the changes look correct. The #ifdefs are a bit ugly, but
> refactoring the MSI code into an arch-specific function should fix
> that. 

I've just sent out a new patch which takes are of only the build failure
on ARM with XSM disabled.

For fixing the flask build on ARM: what does security_device_sid return?
If I want to refactor this:
        #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;
                if (ad) {
                    AVC_AUDIT_DATA_INIT(ad, DEV);
                    ad->device = sbdf;
                }
                return security_device_sid(sbdf, sid);
            }
        #endif
into an arch specific function I need to be able to return something in
the !desc->msi_desc case. Can a sid be any integer or could I return
e.g. 0 in this case?

Ian.

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

[Daniel De Graaf]

On 01/17/2013 12:05 PM, Ian Campbell wrote:
> On Tue, 2013-01-15 at 14:35 +0000, Daniel De Graaf wrote:
>> The rest of the changes look correct. The #ifdefs are a bit ugly, but
>> refactoring the MSI code into an arch-specific function should fix
>> that. 
> 
> I've just sent out a new patch which takes are of only the build failure
> on ARM with XSM disabled.
> 
> For fixing the flask build on ARM: what does security_device_sid return?
> If I want to refactor this:
>         #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;
>                 if (ad) {
>                     AVC_AUDIT_DATA_INIT(ad, DEV);
>                     ad->device = sbdf;
>                 }
>                 return security_device_sid(sbdf, sid);
>             }
>         #endif
> into an arch specific function I need to be able to return something in
> the !desc->msi_desc case. Can a sid be any integer or could I return
> e.g. 0 in this case?
> 
> Ian.
> 

There are some reserved sids that might be useful here (SECINITSID_UNLABELED
if this case shouldn't be encountered). Zero might be the most suitable value,
since zero is not a valid sid and can be used to indicate "unable to resolve".
Zero will be treated as the unlabeled SID if passed to avc_has_perm.

security_device_sid returns either SECINITSID_DEVICE or the assigned SID of
the requested PCI device (as indexed by its sbdf number, and set either in
XSM policy or by flask-label-pci).

-- 
Daniel De Graaf
National Security Agency