[RFC 00/14] xen/arm: Add support for XSM

[RFC 00/14] xen/arm: Add support for XSM

[Julien Grall]

Hello all,

This patch series aims to add support for XSM on ARM.
I've tried to boot Xen on my board with the policy provided in the repository
(see tools/flask/policy). It boots with few warnings because some hypercall
are not describe in the policy.

The series is divided following:
    - #1-2: It's unrelated to this series, but the function copy_paddr
    will be used later
    - #3: Define correctly size_t on ARM
    - #4: Bug fix when xen is booting with XSM blob and without initrd
    - #5-6: XSM movement to be able to use it on ARM
    - #7-11: Compilation fix for ARM
    - #12-13: Rework XSM initialization
    - #14: Add XSM support for ARM

A working tree can be found here:
    git://xenbits.xen.org/people/julieng/xen-unstable.git branch xsm

Sincerely yours,

Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Julien Grall (14):
  xen/arm: kernel: Don't harcode flash address
  xen/arm: Remove the parameter "attrindx" in copy_paddr
  xen/arm: Correctly define size_t
  xen/arm: next_module: Skip module if the size is 0
  xen/xsm: xsm functions for PCI passthrough is not x86 specific
  xen/xsm: xsm_do_mca is x86 specific
  xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y
  xen/xsm: flask: Rename variable "bool" in "b"
  xen/xsm: flask: MSI is PCI specific
  xen/xsm: flask: flask_copying_string is taking a XEN_GUEST_HANDLE as
    first param
  xen/xsm: flask: Add missing header in hooks.c
  xen/xsm: Don't use multiboot by default to initialize XSM
  xen/xsm: Add support for device tree
  xen/arm: Add support for XSM

 docs/misc/arm/device-tree/booting.txt |    1 +
 xen/arch/arm/domain_build.c           |    2 +-
 xen/arch/arm/kernel.c                 |   40 +++---
 xen/arch/arm/kernel.h                 |    1 -
 xen/arch/arm/setup.c                  |    7 +-
 xen/arch/arm/traps.c                  |    1 +
 xen/arch/x86/setup.c                  |    2 +-
 xen/common/device_tree.c              |    2 +
 xen/include/asm-arm/setup.h           |    2 +-
 xen/include/asm-arm/types.h           |    4 +
 xen/include/asm-x86/config.h          |    2 +
 xen/include/xen/device_tree.h         |    3 +-
 xen/include/xsm/dummy.h               |   15 ++-
 xen/include/xsm/xsm.h                 |   51 ++++++--
 xen/xsm/dummy.c                       |    4 +-
 xen/xsm/flask/flask_op.c              |    4 +-
 xen/xsm/flask/hooks.c                 |  228 +++++++++++++++++++--------------
 xen/xsm/flask/include/conditional.h   |    4 +-
 xen/xsm/flask/ss/services.c           |   14 +-
 xen/xsm/xsm_core.c                    |   46 ++++++-
 xen/xsm/xsm_policy.c                  |   47 ++++++-
 21 files changed, 317 insertions(+), 163 deletions(-)

-- 
1.7.10.4

[RFC 01/14] xen/arm: kernel: Don't harcode flash address

[Julien Grall]

Xen is loaded either by U-boot or the bootwrapper. Both of them
correctly set xen multiboot module for the kernel in the device tree.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/kernel.c |   29 ++++++++++-------------------
 xen/arch/arm/kernel.h |    1 -
 2 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 1e3107d..492ce6d 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -16,10 +16,6 @@
 
 #include "kernel.h"
 
-/* Store kernel in first 8M of flash */
-#define KERNEL_FLASH_ADDRESS 0x00000000UL
-#define KERNEL_FLASH_SIZE    0x00800000UL
-
 #define ZIMAGE32_MAGIC_OFFSET 0x24
 #define ZIMAGE32_START_OFFSET 0x28
 #define ZIMAGE32_END_OFFSET   0x2c
@@ -124,7 +120,6 @@ static void kernel_zimage_load(struct kernel_info *info)
 {
     paddr_t load_addr = info->zimage.load_addr;
     paddr_t paddr = info->zimage.kernel_addr;
-    paddr_t attr = info->load_attr;
     paddr_t len = info->zimage.len;
     unsigned long offs;
 
@@ -150,7 +145,7 @@ static void kernel_zimage_load(struct kernel_info *info)
 
         dst = map_domain_page(ma>>PAGE_SHIFT);
 
-        copy_from_paddr(dst + s, paddr + offs, l, attr);
+        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
 
         unmap_domain_page(dst);
         offs += l;
@@ -316,7 +311,7 @@ static int kernel_try_elf_prepare(struct kernel_info *info,
     if ( info->kernel_img == NULL )
         panic("Cannot allocate temporary buffer for kernel");
 
-    copy_from_paddr(info->kernel_img, addr, size, info->load_attr);
+    copy_from_paddr(info->kernel_img, addr, size, BUFFERABLE);
 
     if ( (rc = elf_init(&info->elf.elf, info->kernel_img, size )) != 0 )
         goto err;
@@ -367,21 +362,17 @@ int kernel_prepare(struct kernel_info *info)
 
     paddr_t start, size;
 
-    if ( early_info.modules.nr_mods < MOD_KERNEL )
-    {
-        printk("No boot modules found, trying flash\n");
-        start = KERNEL_FLASH_ADDRESS;
-        size = KERNEL_FLASH_SIZE;
-        info->load_attr = DEV_SHARED;
-    }
-    else
+    start = early_info.modules.module[MOD_KERNEL].start;
+    size = early_info.modules.module[MOD_KERNEL].size;
+
+    if ( !size )
     {
-        printk("Loading kernel from boot module %d\n", MOD_KERNEL);
-        start = early_info.modules.module[MOD_KERNEL].start;
-        size = early_info.modules.module[MOD_KERNEL].size;
-        info->load_attr = BUFFERABLE;
+        printk(XENLOG_ERR "Missing kernel boot module?\n");
+        return -ENOENT;
     }
 
+    printk("Loading kernel from boot module %d\n", MOD_KERNEL);
+
 #ifdef CONFIG_ARM_64
     rc = kernel_try_zimage64_prepare(info, start, size);
     if (rc < 0)
diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h
index b48c2c9..ad2956b 100644
--- a/xen/arch/arm/kernel.h
+++ b/xen/arch/arm/kernel.h
@@ -40,7 +40,6 @@ struct kernel_info {
     };
 
     void (*load)(struct kernel_info *info);
-    int load_attr;
 };
 
 int kernel_prepare(struct kernel_info *info);
-- 
1.7.10.4

Re: [RFC 01/14] xen/arm: kernel: Don't harcode flash address

[Ian Campbell]

On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> Xen is loaded either by U-boot or the bootwrapper. Both of them
> correctly set xen multiboot module for the kernel in the device tree.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Apart from the typo in the subject:
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Mind taking a peek through the wiki for stray references to this old
mechanism?

Re: [RFC 01/14] xen/arm: kernel: Don't harcode flash address

[Julien Grall]

Hi Ian,

On 03/14/2014 05:10 PM, Ian Campbell wrote:
> On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
>> Xen is loaded either by U-boot or the bootwrapper. Both of them
>> correctly set xen multiboot module for the kernel in the device tree.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> Apart from the typo in the subject:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

> Mind taking a peek through the wiki for stray references to this old
> mechanism?

Sure.

Regards,

-- 
Julien Grall

[RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Julien Grall]

copy_addr is only used with BUFFERABLE, there is some place where DEV_SHARED
was used by mistake.

The parameter "attrindx" can be safely remove and let copy_paddr to map
every page with BUFFERABLE attribute.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/domain_build.c |    2 +-
 xen/arch/arm/kernel.c       |   15 +++++++--------
 xen/arch/arm/setup.c        |    2 +-
 xen/include/asm-arm/setup.h |    2 +-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5ca2f15..7bb2c28 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -976,7 +976,7 @@ static void initrd_load(struct kernel_info *kinfo)
 
         dst = map_domain_page(ma>>PAGE_SHIFT);
 
-        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
+        copy_from_paddr(dst + s, paddr + offs, l);
 
         unmap_domain_page(dst);
         offs += l;
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 492ce6d..0bc7eb1 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -40,7 +40,7 @@ struct minimal_dtb_header {
  * @paddr: source physical address
  * @len: length to copy
  */
-void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx)
+void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
 {
     void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
 
@@ -52,7 +52,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx)
         s = paddr & (PAGE_SIZE-1);
         l = min(PAGE_SIZE - s, len);
 
-        set_fixmap(FIXMAP_MISC, p, attrindx);
+        set_fixmap(FIXMAP_MISC, p, BUFFERABLE);
         memcpy(dst, src + s, l);
         clean_xen_dcache_va_range(dst, l);
 
@@ -145,7 +145,7 @@ static void kernel_zimage_load(struct kernel_info *info)
 
         dst = map_domain_page(ma>>PAGE_SHIFT);
 
-        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
+        copy_from_paddr(dst + s, paddr + offs, l);
 
         unmap_domain_page(dst);
         offs += l;
@@ -178,7 +178,7 @@ static int kernel_try_zimage64_prepare(struct kernel_info *info,
     if ( size < sizeof(zimage) )
         return -EINVAL;
 
-    copy_from_paddr(&zimage, addr, sizeof(zimage), DEV_SHARED);
+    copy_from_paddr(&zimage, addr, sizeof(zimage));
 
     if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
          zimage.magic1 != ZIMAGE64_MAGIC_V1 )
@@ -223,7 +223,7 @@ static int kernel_try_zimage32_prepare(struct kernel_info *info,
     if ( size < ZIMAGE32_HEADER_LEN )
         return -EINVAL;
 
-    copy_from_paddr(zimage, addr, sizeof(zimage), DEV_SHARED);
+    copy_from_paddr(zimage, addr, sizeof(zimage));
 
     if (zimage[ZIMAGE32_MAGIC_OFFSET/4] != ZIMAGE32_MAGIC)
         return -EINVAL;
@@ -239,8 +239,7 @@ static int kernel_try_zimage32_prepare(struct kernel_info *info,
      */
     if ( addr + end - start + sizeof(dtb_hdr) <= size )
     {
-        copy_from_paddr(&dtb_hdr, addr + end - start,
-                        sizeof(dtb_hdr), DEV_SHARED);
+        copy_from_paddr(&dtb_hdr, addr + end - start, sizeof(dtb_hdr));
         if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
             end += be32_to_cpu(dtb_hdr.total_size);
 
@@ -311,7 +310,7 @@ static int kernel_try_elf_prepare(struct kernel_info *info,
     if ( info->kernel_img == NULL )
         panic("Cannot allocate temporary buffer for kernel");
 
-    copy_from_paddr(info->kernel_img, addr, size, BUFFERABLE);
+    copy_from_paddr(info->kernel_img, addr, size);
 
     if ( (rc = elf_init(&info->elf.elf, info->kernel_img, size )) != 0 )
         goto err;
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 9480f42..959744e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -488,7 +488,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
 
     /* Copy the DTB. */
     fdt = mfn_to_virt(alloc_boot_pages(dtb_pages, 1));
-    copy_from_paddr(fdt, dtb_paddr, dtb_size, BUFFERABLE);
+    copy_from_paddr(fdt, dtb_paddr, dtb_size);
     device_tree_flattened = fdt;
 
     /* Add non-xenheap memory */
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 44a3b4d..b09f688 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -5,7 +5,7 @@
 
 void arch_init_memory(void);
 
-void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx);
+void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
 void arch_get_xen_caps(xen_capabilities_info_t *info);
 
-- 
1.7.10.4

Re: [RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Ian Campbell]

On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> copy_addr

s/copy_p?addr/copy_from_paddr/ throughout the subject and changelog.

>  is only used with BUFFERABLE, there is some place where DEV_SHARED
> was used by mistake.

A hangover from flash support or something else?

> The parameter "attrindx" can be safely remove and let copy_paddr to map
> every page with BUFFERABLE attribute.

What about if e.g. the firmware provides a dtb which is in flash or
where the kernel/initrd which it references are in flash. There's
nothing inherently wrong with that is there? Maybe BUFFERABLE is still
appropriate there for r/o use.

> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/domain_build.c |    2 +-
>  xen/arch/arm/kernel.c       |   15 +++++++--------
>  xen/arch/arm/setup.c        |    2 +-
>  xen/include/asm-arm/setup.h |    2 +-
>  4 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5ca2f15..7bb2c28 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -976,7 +976,7 @@ static void initrd_load(struct kernel_info *kinfo)
>  
>          dst = map_domain_page(ma>>PAGE_SHIFT);
>  
> -        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
> +        copy_from_paddr(dst + s, paddr + offs, l);
>  
>          unmap_domain_page(dst);
>          offs += l;
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 492ce6d..0bc7eb1 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -40,7 +40,7 @@ struct minimal_dtb_header {
>   * @paddr: source physical address
>   * @len: length to copy
>   */
> -void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx)
> +void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
>  {
>      void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
>  
> @@ -52,7 +52,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx)
>          s = paddr & (PAGE_SIZE-1);
>          l = min(PAGE_SIZE - s, len);
>  
> -        set_fixmap(FIXMAP_MISC, p, attrindx);
> +        set_fixmap(FIXMAP_MISC, p, BUFFERABLE);
>          memcpy(dst, src + s, l);
>          clean_xen_dcache_va_range(dst, l);
>  
> @@ -145,7 +145,7 @@ static void kernel_zimage_load(struct kernel_info *info)
>  
>          dst = map_domain_page(ma>>PAGE_SHIFT);
>  
> -        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
> +        copy_from_paddr(dst + s, paddr + offs, l);
>  
>          unmap_domain_page(dst);
>          offs += l;
> @@ -178,7 +178,7 @@ static int kernel_try_zimage64_prepare(struct kernel_info *info,
>      if ( size < sizeof(zimage) )
>          return -EINVAL;
>  
> -    copy_from_paddr(&zimage, addr, sizeof(zimage), DEV_SHARED);
> +    copy_from_paddr(&zimage, addr, sizeof(zimage));
>  
>      if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>           zimage.magic1 != ZIMAGE64_MAGIC_V1 )
> @@ -223,7 +223,7 @@ static int kernel_try_zimage32_prepare(struct kernel_info *info,
>      if ( size < ZIMAGE32_HEADER_LEN )
>          return -EINVAL;
>  
> -    copy_from_paddr(zimage, addr, sizeof(zimage), DEV_SHARED);
> +    copy_from_paddr(zimage, addr, sizeof(zimage));
>  
>      if (zimage[ZIMAGE32_MAGIC_OFFSET/4] != ZIMAGE32_MAGIC)
>          return -EINVAL;
> @@ -239,8 +239,7 @@ static int kernel_try_zimage32_prepare(struct kernel_info *info,
>       */
>      if ( addr + end - start + sizeof(dtb_hdr) <= size )
>      {
> -        copy_from_paddr(&dtb_hdr, addr + end - start,
> -                        sizeof(dtb_hdr), DEV_SHARED);
> +        copy_from_paddr(&dtb_hdr, addr + end - start, sizeof(dtb_hdr));
>          if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
>              end += be32_to_cpu(dtb_hdr.total_size);
>  
> @@ -311,7 +310,7 @@ static int kernel_try_elf_prepare(struct kernel_info *info,
>      if ( info->kernel_img == NULL )
>          panic("Cannot allocate temporary buffer for kernel");
>  
> -    copy_from_paddr(info->kernel_img, addr, size, BUFFERABLE);
> +    copy_from_paddr(info->kernel_img, addr, size);
>  
>      if ( (rc = elf_init(&info->elf.elf, info->kernel_img, size )) != 0 )
>          goto err;
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 9480f42..959744e 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -488,7 +488,7 @@ static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
>  
>      /* Copy the DTB. */
>      fdt = mfn_to_virt(alloc_boot_pages(dtb_pages, 1));
> -    copy_from_paddr(fdt, dtb_paddr, dtb_size, BUFFERABLE);
> +    copy_from_paddr(fdt, dtb_paddr, dtb_size);
>      device_tree_flattened = fdt;
>  
>      /* Add non-xenheap memory */
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 44a3b4d..b09f688 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -5,7 +5,7 @@
>  
>  void arch_init_memory(void);
>  
> -void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int attrindx);
> +void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
>  
>  void arch_get_xen_caps(xen_capabilities_info_t *info);
>  

Re: [RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Julien Grall]

Hi Ian,

On 03/14/2014 05:14 PM, Ian Campbell wrote:
> On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
>> copy_addr
> 
> s/copy_p?addr/copy_from_paddr/ throughout the subject and changelog.

Right. I will fix it in the next series.

>>  is only used with BUFFERABLE, there is some place where DEV_SHARED
>> was used by mistake.
> 
> A hangover from flash support or something else?

I guess, DEV_SHARED was using in few place mainly when Xen is loading a
small amount of data. Even before this patch, info->load_attr should
have been used.

Do you think I need to send a patch for Xen 4.4?

>> The parameter "attrindx" can be safely remove and let copy_paddr to map
>> every page with BUFFERABLE attribute.
> 
> What about if e.g. the firmware provides a dtb which is in flash or
> where the kernel/initrd which it references are in flash. There's
> nothing inherently wrong with that is there? Maybe BUFFERABLE is still
> appropriate there for r/o use.

Even before this patch, Xen was using BUFFERABLE when the kernel/initrd
is described in the device tree. I don't think it's easily possible to
know if the range belongs to the RAM or not. Maybe we can use UNCACHE in
this context?

Regards,

-- 
Julien Grall

Re: [RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Ian Campbell]

On Fri, 2014-03-14 at 18:02 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/14/2014 05:14 PM, Ian Campbell wrote:
> > On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> >> copy_addr
> > 
> > s/copy_p?addr/copy_from_paddr/ throughout the subject and changelog.
> 
> Right. I will fix it in the next series.
> 
> >>  is only used with BUFFERABLE, there is some place where DEV_SHARED
> >> was used by mistake.
> > 
> > A hangover from flash support or something else?
> 
> I guess, DEV_SHARED was using in few place mainly when Xen is loading a
> small amount of data. Even before this patch, info->load_attr should
> have been used.
> 
> Do you think I need to send a patch for Xen 4.4?

Is it actively harmful or just not ideal?

> >> The parameter "attrindx" can be safely remove and let copy_paddr to map
> >> every page with BUFFERABLE attribute.
> > 
> > What about if e.g. the firmware provides a dtb which is in flash or
> > where the kernel/initrd which it references are in flash. There's
> > nothing inherently wrong with that is there? Maybe BUFFERABLE is still
> > appropriate there for r/o use.
> 
> Even before this patch, Xen was using BUFFERABLE when the kernel/initrd
> is described in the device tree. I don't think it's easily possible to
> know if the range belongs to the RAM or not. Maybe we can use UNCACHE in
> this context?

True. Lets stick with BUFFERABLE (i.e. assume RAM) until someone comes
along with things in flash, then we can actually test things. (or they
never show up and we save ourselves some effort).

Ian.

Re: [RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Julien Grall]

Hi Ian,

On 03/17/2014 10:13 AM, Ian Campbell wrote:
> On Fri, 2014-03-14 at 18:02 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 03/14/2014 05:14 PM, Ian Campbell wrote:
>>> On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
>>>> copy_addr
>>>
>>> s/copy_p?addr/copy_from_paddr/ throughout the subject and changelog.
>>
>> Right. I will fix it in the next series.
>>
>>>>  is only used with BUFFERABLE, there is some place where DEV_SHARED
>>>> was used by mistake.
>>>
>>> A hangover from flash support or something else?
>>
>> I guess, DEV_SHARED was using in few place mainly when Xen is loading a
>> small amount of data. Even before this patch, info->load_attr should
>> have been used.
>>
>> Do you think I need to send a patch for Xen 4.4?
> 
> Is it actively harmful or just not ideal?

>From the ARM ARM A3.5.1, we should use Normal memory attribute even for
the flash.

I'm not sure what is the behavior when Normal memory is access with
Device memory attribute.

Regards,

-- 
Julien Grall

Re: [RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Ian Campbell]

On Mon, 2014-03-17 at 11:53 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/17/2014 10:13 AM, Ian Campbell wrote:
> > On Fri, 2014-03-14 at 18:02 +0000, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 03/14/2014 05:14 PM, Ian Campbell wrote:
> >>> On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> >>>> copy_addr
> >>>
> >>> s/copy_p?addr/copy_from_paddr/ throughout the subject and changelog.
> >>
> >> Right. I will fix it in the next series.
> >>
> >>>>  is only used with BUFFERABLE, there is some place where DEV_SHARED
> >>>> was used by mistake.
> >>>
> >>> A hangover from flash support or something else?
> >>
> >> I guess, DEV_SHARED was using in few place mainly when Xen is loading a
> >> small amount of data. Even before this patch, info->load_attr should
> >> have been used.
> >>
> >> Do you think I need to send a patch for Xen 4.4?
> > 
> > Is it actively harmful or just not ideal?
> 
> From the ARM ARM A3.5.1, we should use Normal memory attribute even for
> the flash.

For r/o access that does make sense. I expect when programming or
sending control commands more cleverness is needed, but we needn't worry
here. Good.

> I'm not sure what is the behavior when Normal memory is access with
> Device memory attribute.

It's just pointlessly uncached and strongly ordered I think, so it's a
performance hit.

Ian.

[RFC 03/14] xen/arm: Correctly define size_t

[Julien Grall]

Flask code use %zu to print size_t variable. On ARM, size_t always defined
to unsigned long which make ARM build failed.

Fix it by using __SIZE_TYPE__.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/include/asm-arm/types.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
index 6875a62..183f3e0 100644
--- a/xen/include/asm-arm/types.h
+++ b/xen/include/asm-arm/types.h
@@ -55,7 +55,11 @@ typedef u64 register_t;
 #define PRIregister "lx"
 #endif
 
+#if defined(__SIZE_TYPE__)
+typedef __SIZE_TYPE__ size_t;
+#else
 typedef unsigned long size_t;
+#endif
 
 typedef char bool_t;
 #define test_and_set_bool(b)   xchg(&(b), 1)
-- 
1.7.10.4

Re: [RFC 03/14] xen/arm: Correctly define size_t

[Ian Campbell]

On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> Flask code use %zu to print size_t variable. On ARM, size_t always defined
> to unsigned long which make ARM build failed.
> 
> Fix it by using __SIZE_TYPE__.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/include/asm-arm/types.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
> index 6875a62..183f3e0 100644
> --- a/xen/include/asm-arm/types.h
> +++ b/xen/include/asm-arm/types.h
> @@ -55,7 +55,11 @@ typedef u64 register_t;
>  #define PRIregister "lx"
>  #endif
>  
> +#if defined(__SIZE_TYPE__)
> +typedef __SIZE_TYPE__ size_t;
> +#else
>  typedef unsigned long size_t;
> +#endif
>  
>  typedef char bool_t;
>  #define test_and_set_bool(b)   xchg(&(b), 1)

[RFC 04/14] xen/arm: next_module: Skip module if the size is 0

[Julien Grall]

When the the module size is 0, it means that the module was not provided by
the user. It can happen, if the user choose to boot without initrd.

In this case, both fields (start and size) are zeroed. Therefore, next_module
will return 0 every time if there is other non-zero module after this one. It
will happend when the XSM module will be added.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/setup.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 959744e..31d853d 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -281,6 +281,9 @@ static paddr_t __init next_module(paddr_t s, paddr_t *end)
         paddr_t mod_s = mi->module[i].start;
         paddr_t mod_e = mod_s + mi->module[i].size;
 
+        if ( !mi->module[i].size )
+            continue;
+
         if ( mod_s < s )
             continue;
         if ( mod_s > lowest )
-- 
1.7.10.4

Re: [RFC 04/14] xen/arm: next_module: Skip module if the size is 0

[Ian Campbell]

On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> When the the module size is 0, it means that the module was not provided by
> the user. It can happen, if the user choose to boot without initrd.
> 
> In this case, both fields (start and size) are zeroed. Therefore, next_module
> will return 0 every time if there is other non-zero module after this one. It
> will happend 

"happen" or better "This can happen"

> when the XSM module will be added.

s/will be/is/

> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/arch/arm/setup.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 959744e..31d853d 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -281,6 +281,9 @@ static paddr_t __init next_module(paddr_t s, paddr_t *end)
>          paddr_t mod_s = mi->module[i].start;
>          paddr_t mod_e = mod_s + mi->module[i].size;
>  
> +        if ( !mi->module[i].size )
> +            continue;
> +
>          if ( mod_s < s )
>              continue;
>          if ( mod_s > lowest )

[RFC 05/14] xen/xsm: xsm functions for PCI passthrough is not x86 specific

[Julien Grall]

Protect xsm functions for PCI passthrough by HAS_PASSTHROUGH && HAS_PCI

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/include/xsm/dummy.h |    3 +
 xen/include/xsm/xsm.h   |    4 ++
 xen/xsm/dummy.c         |    2 +
 xen/xsm/flask/hooks.c   |  143 +++++++++++++++++++++++++----------------------
 4 files changed, 84 insertions(+), 68 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 3bcd941..76f9280 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -317,6 +317,7 @@ static XSM_INLINE int xsm_set_pod_target(XSM_DEFAULT_ARG struct domain *d)
     return xsm_default_action(action, current->domain, d);
 }
 
+#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
 static XSM_INLINE int xsm_get_device_group(XSM_DEFAULT_ARG uint32_t machine_bdf)
 {
     XSM_ASSERT_ACTION(XSM_HOOK);
@@ -341,6 +342,8 @@ static XSM_INLINE int xsm_deassign_device(XSM_DEFAULT_ARG struct domain *d, uint
     return xsm_default_action(action, current->domain, d);
 }
 
+#endif /* HAS_PASSTHROUGH && HAS_PCI */
+
 static XSM_INLINE int xsm_resource_plug_core(XSM_DEFAULT_VOID)
 {
     XSM_ASSERT_ACTION(XSM_HOOK);
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index de9cf86..11218b6 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -111,10 +111,12 @@ struct xsm_operations {
     int (*iomem_mapping) (struct domain *d, uint64_t s, uint64_t e, uint8_t allow);
     int (*pci_config_permission) (struct domain *d, uint32_t machine_bdf, uint16_t start, uint16_t end, uint8_t access);
 
+#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
     int (*get_device_group) (uint32_t machine_bdf);
     int (*test_assign_device) (uint32_t machine_bdf);
     int (*assign_device) (struct domain *d, uint32_t machine_bdf);
     int (*deassign_device) (struct domain *d, uint32_t machine_bdf);
+#endif
 
     int (*resource_plug_core) (void);
     int (*resource_unplug_core) (void);
@@ -427,6 +429,7 @@ static inline int xsm_pci_config_permission (xsm_default_t def, struct domain *d
     return xsm_ops->pci_config_permission(d, machine_bdf, start, end, access);
 }
 
+#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
 static inline int xsm_get_device_group(xsm_default_t def, uint32_t machine_bdf)
 {
     return xsm_ops->get_device_group(machine_bdf);
@@ -446,6 +449,7 @@ static inline int xsm_deassign_device(xsm_default_t def, struct domain *d, uint3
 {
     return xsm_ops->deassign_device(d, machine_bdf);
 }
+#endif /* HAS_PASSTHROUGH && HAS_PCI) */
 
 static inline int xsm_resource_plug_pci (xsm_default_t def, uint32_t machine_bdf)
 {
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 3fe4c59..627edcc 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -85,10 +85,12 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, iomem_mapping);
     set_to_dummy_if_null(ops, pci_config_permission);
 
+#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
     set_to_dummy_if_null(ops, get_device_group);
     set_to_dummy_if_null(ops, test_assign_device);
     set_to_dummy_if_null(ops, assign_device);
     set_to_dummy_if_null(ops, deassign_device);
+#endif
 
     set_to_dummy_if_null(ops, resource_plug_core);
     set_to_dummy_if_null(ops, resource_unplug_core);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 96276ac..7329f31 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1102,6 +1102,72 @@ static int flask_hvm_param_nested(struct domain *d)
     return current_has_perm(d, SECCLASS_HVM, HVM__NESTED);
 }
 
+#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
+static int flask_get_device_group(uint32_t machine_bdf)
+{
+    u32 rsid;
+    int rc = -EPERM;
+
+    rc = security_device_sid(machine_bdf, &rsid);
+    if ( rc )
+        return rc;
+
+    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
+}
+
+static int flask_test_assign_device(uint32_t machine_bdf)
+{
+    u32 rsid;
+    int rc = -EPERM;
+
+    rc = security_device_sid(machine_bdf, &rsid);
+    if ( rc )
+        return rc;
+
+    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
+}
+
+static int flask_assign_device(struct domain *d, uint32_t machine_bdf)
+{
+    u32 dsid, rsid;
+    int rc = -EPERM;
+    struct avc_audit_data ad;
+
+    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
+    if ( rc )
+        return rc;
+
+    rc = security_device_sid(machine_bdf, &rsid);
+    if ( rc )
+        return rc;
+
+    AVC_AUDIT_DATA_INIT(&ad, DEV);
+    ad.device = (unsigned long) machine_bdf;
+    rc = avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__ADD_DEVICE, &ad);
+    if ( rc )
+        return rc;
+
+    dsid = domain_sid(d);
+    return avc_has_perm(dsid, rsid, SECCLASS_RESOURCE, RESOURCE__USE, &ad);
+}
+
+static int flask_deassign_device(struct domain *d, uint32_t machine_bdf)
+{
+    u32 rsid;
+    int rc = -EPERM;
+
+    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
+    if ( rc )
+        return rc;
+
+    rc = security_device_sid(machine_bdf, &rsid);
+    if ( rc )
+        return rc;
+
+    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__REMOVE_DEVICE, NULL);
+}
+#endif /* HAS_PASSTHROUGH && HAS_PCI */
+
 #ifdef CONFIG_X86
 static int flask_shadow_control(struct domain *d, uint32_t op)
 {
@@ -1355,70 +1421,6 @@ static int flask_priv_mapping(struct domain *d, struct domain *t)
     return domain_has_perm(d, t, SECCLASS_MMU, MMU__TARGET_HACK);
 }
 
-static int flask_get_device_group(uint32_t machine_bdf)
-{
-    u32 rsid;
-    int rc = -EPERM;
-
-    rc = security_device_sid(machine_bdf, &rsid);
-    if ( rc )
-        return rc;
-
-    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
-}
-
-static int flask_test_assign_device(uint32_t machine_bdf)
-{
-    u32 rsid;
-    int rc = -EPERM;
-
-    rc = security_device_sid(machine_bdf, &rsid);
-    if ( rc )
-        return rc;
-
-    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
-}
-
-static int flask_assign_device(struct domain *d, uint32_t machine_bdf)
-{
-    u32 dsid, rsid;
-    int rc = -EPERM;
-    struct avc_audit_data ad;
-
-    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
-    if ( rc )
-        return rc;
-
-    rc = security_device_sid(machine_bdf, &rsid);
-    if ( rc )
-        return rc;
-
-    AVC_AUDIT_DATA_INIT(&ad, DEV);
-    ad.device = (unsigned long) machine_bdf;
-    rc = avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__ADD_DEVICE, &ad);
-    if ( rc )
-        return rc;
-
-    dsid = domain_sid(d);
-    return avc_has_perm(dsid, rsid, SECCLASS_RESOURCE, RESOURCE__USE, &ad);
-}
-
-static int flask_deassign_device(struct domain *d, uint32_t machine_bdf)
-{
-    u32 rsid;
-    int rc = -EPERM;
-
-    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
-    if ( rc )
-        return rc;
-
-    rc = security_device_sid(machine_bdf, &rsid);
-    if ( rc )
-        return rc;
-
-    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__REMOVE_DEVICE, NULL);
-}
-
 static int flask_bind_pt_irq (struct domain *d, struct xen_domctl_bind_pt_irq *bind)
 {
     u32 dsid, rsid;
@@ -1540,6 +1542,14 @@ static struct xsm_operations flask_ops = {
     .add_to_physmap = flask_add_to_physmap,
     .remove_from_physmap = flask_remove_from_physmap,
 
+
+#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
+    .get_device_group = flask_get_device_group,
+    .test_assign_device = flask_test_assign_device,
+    .assign_device = flask_assign_device,
+    .deassign_device = flask_deassign_device,
+#endif
+
 #ifdef CONFIG_X86
     .shadow_control = flask_shadow_control,
     .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level,
@@ -1557,15 +1567,12 @@ static struct xsm_operations flask_ops = {
     .mmuext_op = flask_mmuext_op,
     .update_va_mapping = flask_update_va_mapping,
     .priv_mapping = flask_priv_mapping,
-    .get_device_group = flask_get_device_group,
-    .test_assign_device = flask_test_assign_device,
-    .assign_device = flask_assign_device,
-    .deassign_device = flask_deassign_device,
     .bind_pt_irq = flask_bind_pt_irq,
     .unbind_pt_irq = flask_unbind_pt_irq,
     .ioport_permission = flask_ioport_permission,
     .ioport_mapping = flask_ioport_mapping,
 #endif
+
 #ifdef CONFIG_ARM
     .map_gmfn_foreign = flask_map_gmfn_foreign,
 #endif
-- 
1.7.10.4

Re: [RFC 05/14] xen/xsm: xsm functions for PCI passthrough is not x86 specific

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> Protect xsm functions for PCI passthrough by HAS_PASSTHROUGH && HAS_PCI
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/include/xsm/dummy.h |    3 +
>   xen/include/xsm/xsm.h   |    4 ++
>   xen/xsm/dummy.c         |    2 +
>   xen/xsm/flask/hooks.c   |  143 +++++++++++++++++++++++++----------------------
>   4 files changed, 84 insertions(+), 68 deletions(-)
>
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 3bcd941..76f9280 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -317,6 +317,7 @@ static XSM_INLINE int xsm_set_pod_target(XSM_DEFAULT_ARG struct domain *d)
>       return xsm_default_action(action, current->domain, d);
>   }
>
> +#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
>   static XSM_INLINE int xsm_get_device_group(XSM_DEFAULT_ARG uint32_t machine_bdf)
>   {
>       XSM_ASSERT_ACTION(XSM_HOOK);
> @@ -341,6 +342,8 @@ static XSM_INLINE int xsm_deassign_device(XSM_DEFAULT_ARG struct domain *d, uint
>       return xsm_default_action(action, current->domain, d);
>   }
>
> +#endif /* HAS_PASSTHROUGH && HAS_PCI */
> +
>   static XSM_INLINE int xsm_resource_plug_core(XSM_DEFAULT_VOID)
>   {
>       XSM_ASSERT_ACTION(XSM_HOOK);
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index de9cf86..11218b6 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -111,10 +111,12 @@ struct xsm_operations {
>       int (*iomem_mapping) (struct domain *d, uint64_t s, uint64_t e, uint8_t allow);
>       int (*pci_config_permission) (struct domain *d, uint32_t machine_bdf, uint16_t start, uint16_t end, uint8_t access);
>
> +#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
>       int (*get_device_group) (uint32_t machine_bdf);
>       int (*test_assign_device) (uint32_t machine_bdf);
>       int (*assign_device) (struct domain *d, uint32_t machine_bdf);
>       int (*deassign_device) (struct domain *d, uint32_t machine_bdf);
> +#endif
>
>       int (*resource_plug_core) (void);
>       int (*resource_unplug_core) (void);
> @@ -427,6 +429,7 @@ static inline int xsm_pci_config_permission (xsm_default_t def, struct domain *d
>       return xsm_ops->pci_config_permission(d, machine_bdf, start, end, access);
>   }
>
> +#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
>   static inline int xsm_get_device_group(xsm_default_t def, uint32_t machine_bdf)
>   {
>       return xsm_ops->get_device_group(machine_bdf);
> @@ -446,6 +449,7 @@ static inline int xsm_deassign_device(xsm_default_t def, struct domain *d, uint3
>   {
>       return xsm_ops->deassign_device(d, machine_bdf);
>   }
> +#endif /* HAS_PASSTHROUGH && HAS_PCI) */
>
>   static inline int xsm_resource_plug_pci (xsm_default_t def, uint32_t machine_bdf)
>   {
> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
> index 3fe4c59..627edcc 100644
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -85,10 +85,12 @@ void xsm_fixup_ops (struct xsm_operations *ops)
>       set_to_dummy_if_null(ops, iomem_mapping);
>       set_to_dummy_if_null(ops, pci_config_permission);
>
> +#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
>       set_to_dummy_if_null(ops, get_device_group);
>       set_to_dummy_if_null(ops, test_assign_device);
>       set_to_dummy_if_null(ops, assign_device);
>       set_to_dummy_if_null(ops, deassign_device);
> +#endif
>
>       set_to_dummy_if_null(ops, resource_plug_core);
>       set_to_dummy_if_null(ops, resource_unplug_core);
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 96276ac..7329f31 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1102,6 +1102,72 @@ static int flask_hvm_param_nested(struct domain *d)
>       return current_has_perm(d, SECCLASS_HVM, HVM__NESTED);
>   }
>
> +#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
> +static int flask_get_device_group(uint32_t machine_bdf)
> +{
> +    u32 rsid;
> +    int rc = -EPERM;
> +
> +    rc = security_device_sid(machine_bdf, &rsid);
> +    if ( rc )
> +        return rc;
> +
> +    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
> +}
> +
> +static int flask_test_assign_device(uint32_t machine_bdf)
> +{
> +    u32 rsid;
> +    int rc = -EPERM;
> +
> +    rc = security_device_sid(machine_bdf, &rsid);
> +    if ( rc )
> +        return rc;
> +
> +    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
> +}
> +
> +static int flask_assign_device(struct domain *d, uint32_t machine_bdf)
> +{
> +    u32 dsid, rsid;
> +    int rc = -EPERM;
> +    struct avc_audit_data ad;
> +
> +    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> +    if ( rc )
> +        return rc;
> +
> +    rc = security_device_sid(machine_bdf, &rsid);
> +    if ( rc )
> +        return rc;
> +
> +    AVC_AUDIT_DATA_INIT(&ad, DEV);
> +    ad.device = (unsigned long) machine_bdf;
> +    rc = avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__ADD_DEVICE, &ad);
> +    if ( rc )
> +        return rc;
> +
> +    dsid = domain_sid(d);
> +    return avc_has_perm(dsid, rsid, SECCLASS_RESOURCE, RESOURCE__USE, &ad);
> +}
> +
> +static int flask_deassign_device(struct domain *d, uint32_t machine_bdf)
> +{
> +    u32 rsid;
> +    int rc = -EPERM;
> +
> +    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
> +    if ( rc )
> +        return rc;
> +
> +    rc = security_device_sid(machine_bdf, &rsid);
> +    if ( rc )
> +        return rc;
> +
> +    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__REMOVE_DEVICE, NULL);
> +}
> +#endif /* HAS_PASSTHROUGH && HAS_PCI */
> +
>   #ifdef CONFIG_X86
>   static int flask_shadow_control(struct domain *d, uint32_t op)
>   {
> @@ -1355,70 +1421,6 @@ static int flask_priv_mapping(struct domain *d, struct domain *t)
>       return domain_has_perm(d, t, SECCLASS_MMU, MMU__TARGET_HACK);
>   }
>
> -static int flask_get_device_group(uint32_t machine_bdf)
> -{
> -    u32 rsid;
> -    int rc = -EPERM;
> -
> -    rc = security_device_sid(machine_bdf, &rsid);
> -    if ( rc )
> -        return rc;
> -
> -    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
> -}
> -
> -static int flask_test_assign_device(uint32_t machine_bdf)
> -{
> -    u32 rsid;
> -    int rc = -EPERM;
> -
> -    rc = security_device_sid(machine_bdf, &rsid);
> -    if ( rc )
> -        return rc;
> -
> -    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__STAT_DEVICE, NULL);
> -}
> -
> -static int flask_assign_device(struct domain *d, uint32_t machine_bdf)
> -{
> -    u32 dsid, rsid;
> -    int rc = -EPERM;
> -    struct avc_audit_data ad;
> -
> -    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
> -    if ( rc )
> -        return rc;
> -
> -    rc = security_device_sid(machine_bdf, &rsid);
> -    if ( rc )
> -        return rc;
> -
> -    AVC_AUDIT_DATA_INIT(&ad, DEV);
> -    ad.device = (unsigned long) machine_bdf;
> -    rc = avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__ADD_DEVICE, &ad);
> -    if ( rc )
> -        return rc;
> -
> -    dsid = domain_sid(d);
> -    return avc_has_perm(dsid, rsid, SECCLASS_RESOURCE, RESOURCE__USE, &ad);
> -}
> -
> -static int flask_deassign_device(struct domain *d, uint32_t machine_bdf)
> -{
> -    u32 rsid;
> -    int rc = -EPERM;
> -
> -    rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
> -    if ( rc )
> -        return rc;
> -
> -    rc = security_device_sid(machine_bdf, &rsid);
> -    if ( rc )
> -        return rc;
> -
> -    return avc_current_has_perm(rsid, SECCLASS_RESOURCE, RESOURCE__REMOVE_DEVICE, NULL);
> -}
> -
>   static int flask_bind_pt_irq (struct domain *d, struct xen_domctl_bind_pt_irq *bind)
>   {
>       u32 dsid, rsid;
> @@ -1540,6 +1542,14 @@ static struct xsm_operations flask_ops = {
>       .add_to_physmap = flask_add_to_physmap,
>       .remove_from_physmap = flask_remove_from_physmap,
>
> +
> +#if defined(HAS_PASSTHROUGH) && defined(HAS_PCI)
> +    .get_device_group = flask_get_device_group,
> +    .test_assign_device = flask_test_assign_device,
> +    .assign_device = flask_assign_device,
> +    .deassign_device = flask_deassign_device,
> +#endif
> +
>   #ifdef CONFIG_X86
>       .shadow_control = flask_shadow_control,
>       .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level,
> @@ -1557,15 +1567,12 @@ static struct xsm_operations flask_ops = {
>       .mmuext_op = flask_mmuext_op,
>       .update_va_mapping = flask_update_va_mapping,
>       .priv_mapping = flask_priv_mapping,
> -    .get_device_group = flask_get_device_group,
> -    .test_assign_device = flask_test_assign_device,
> -    .assign_device = flask_assign_device,
> -    .deassign_device = flask_deassign_device,
>       .bind_pt_irq = flask_bind_pt_irq,
>       .unbind_pt_irq = flask_unbind_pt_irq,
>       .ioport_permission = flask_ioport_permission,
>       .ioport_mapping = flask_ioport_mapping,
>   #endif
> +
>   #ifdef CONFIG_ARM
>       .map_gmfn_foreign = flask_map_gmfn_foreign,
>   #endif
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 05/14] xen/xsm: xsm functions for PCI passthrough is not x86 specific

[Ian Campbell]

On Thu, 2014-03-13 at 10:25 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > Protect xsm functions for PCI passthrough by HAS_PASSTHROUGH && HAS_PCI
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[RFC 06/14] xen/xsm: xsm_do_mca is x86 specific

[Julien Grall]

xsm_do_mca is only used by x86. Only define the function for x86 to
avoid usage on ARM.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/include/xsm/dummy.h |   12 ++++++------
 xen/include/xsm/xsm.h   |   12 ++++++------
 xen/xsm/dummy.c         |    2 +-
 xen/xsm/flask/hooks.c   |   12 ++++++------
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 76f9280..e722155 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -140,12 +140,6 @@ static XSM_INLINE int xsm_readconsole(XSM_DEFAULT_ARG uint32_t clear)
     return xsm_default_action(action, current->domain, NULL);
 }
 
-static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
-{
-    XSM_ASSERT_ACTION(XSM_PRIV);
-    return xsm_default_action(action, current->domain, NULL);
-}
-
 static XSM_INLINE int xsm_alloc_security_domain(struct domain *d)
 {
     return 0;
@@ -496,6 +490,12 @@ static XSM_INLINE int xsm_hvm_param_nested(XSM_DEFAULT_ARG struct domain *d)
 }
 
 #ifdef CONFIG_X86
+static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
+{
+    XSM_ASSERT_ACTION(XSM_PRIV);
+    return xsm_default_action(action, current->domain, NULL);
+}
+
 static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op)
 {
     XSM_ASSERT_ACTION(XSM_HOOK);
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 11218b6..15acb3b 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -61,7 +61,6 @@ struct xsm_operations {
     int (*domctl) (struct domain *d, int cmd);
     int (*sysctl) (int cmd);
     int (*readconsole) (uint32_t clear);
-    int (*do_mca) (void);
 
     int (*evtchn_unbound) (struct domain *d, struct evtchn *chn, domid_t id2);
     int (*evtchn_interdomain) (struct domain *d1, struct evtchn *chn1,
@@ -139,6 +138,7 @@ struct xsm_operations {
     int (*hvm_param_nested) (struct domain *d);
 
 #ifdef CONFIG_X86
+    int (*do_mca) (void);
     int (*shadow_control) (struct domain *d, uint32_t op);
     int (*hvm_set_pci_intx_level) (struct domain *d);
     int (*hvm_set_isa_irq_level) (struct domain *d);
@@ -223,11 +223,6 @@ static inline int xsm_readconsole (xsm_default_t def, uint32_t clear)
     return xsm_ops->readconsole(clear);
 }
 
-static inline int xsm_do_mca(xsm_default_t def)
-{
-    return xsm_ops->do_mca();
-}
-
 static inline int xsm_evtchn_unbound (xsm_default_t def, struct domain *d1, struct evtchn *chn,
                                                                     domid_t id2)
 {
@@ -524,6 +519,11 @@ static inline int xsm_hvm_param_nested (xsm_default_t def, struct domain *d)
 }
 
 #ifdef CONFIG_X86
+static inline int xsm_do_mca(xsm_default_t def)
+{
+    return xsm_ops->do_mca();
+}
+
 static inline int xsm_shadow_control (xsm_default_t def, struct domain *d, uint32_t op)
 {
     return xsm_ops->shadow_control(d, op);
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 627edcc..b79e10f 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -38,7 +38,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, domctl);
     set_to_dummy_if_null(ops, sysctl);
     set_to_dummy_if_null(ops, readconsole);
-    set_to_dummy_if_null(ops, do_mca);
 
     set_to_dummy_if_null(ops, evtchn_unbound);
     set_to_dummy_if_null(ops, evtchn_interdomain);
@@ -115,6 +114,7 @@ void xsm_fixup_ops (struct xsm_operations *ops)
     set_to_dummy_if_null(ops, remove_from_physmap);
 
 #ifdef CONFIG_X86
+    set_to_dummy_if_null(ops, do_mca);
     set_to_dummy_if_null(ops, shadow_control);
     set_to_dummy_if_null(ops, hvm_set_pci_intx_level);
     set_to_dummy_if_null(ops, hvm_set_isa_irq_level);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 7329f31..65343f3 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -798,11 +798,6 @@ static int flask_readconsole(uint32_t clear)
     return domain_has_xen(current->domain, perms);
 }
 
-static int flask_do_mca(void)
-{
-    return domain_has_xen(current->domain, XEN__MCA_OP);
-}
-
 static inline u32 resource_to_perm(uint8_t access)
 {
     if ( access )
@@ -1169,6 +1164,11 @@ static int flask_deassign_device(struct domain *d, uint32_t machine_bdf)
 #endif /* HAS_PASSTHROUGH && HAS_PCI */
 
 #ifdef CONFIG_X86
+static int flask_do_mca(void)
+{
+    return domain_has_xen(current->domain, XEN__MCA_OP);
+}
+
 static int flask_shadow_control(struct domain *d, uint32_t op)
 {
     u32 perm;
@@ -1472,7 +1472,6 @@ static struct xsm_operations flask_ops = {
     .domctl = flask_domctl,
     .sysctl = flask_sysctl,
     .readconsole = flask_readconsole,
-    .do_mca = flask_do_mca,
 
     .evtchn_unbound = flask_evtchn_unbound,
     .evtchn_interdomain = flask_evtchn_interdomain,
@@ -1551,6 +1550,7 @@ static struct xsm_operations flask_ops = {
 #endif
 
 #ifdef CONFIG_X86
+    .do_mca = flask_do_mca,
     .shadow_control = flask_shadow_control,
     .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level,
     .hvm_set_isa_irq_level = flask_hvm_set_isa_irq_level,
-- 
1.7.10.4

Re: [RFC 06/14] xen/xsm: xsm_do_mca is x86 specific

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> xsm_do_mca is only used by x86. Only define the function for x86 to
> avoid usage on ARM.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/include/xsm/dummy.h |   12 ++++++------
>   xen/include/xsm/xsm.h   |   12 ++++++------
>   xen/xsm/dummy.c         |    2 +-
>   xen/xsm/flask/hooks.c   |   12 ++++++------
>   4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 76f9280..e722155 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -140,12 +140,6 @@ static XSM_INLINE int xsm_readconsole(XSM_DEFAULT_ARG uint32_t clear)
>       return xsm_default_action(action, current->domain, NULL);
>   }
>
> -static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
> -{
> -    XSM_ASSERT_ACTION(XSM_PRIV);
> -    return xsm_default_action(action, current->domain, NULL);
> -}
> -
>   static XSM_INLINE int xsm_alloc_security_domain(struct domain *d)
>   {
>       return 0;
> @@ -496,6 +490,12 @@ static XSM_INLINE int xsm_hvm_param_nested(XSM_DEFAULT_ARG struct domain *d)
>   }
>
>   #ifdef CONFIG_X86
> +static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
> +{
> +    XSM_ASSERT_ACTION(XSM_PRIV);
> +    return xsm_default_action(action, current->domain, NULL);
> +}
> +
>   static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op)
>   {
>       XSM_ASSERT_ACTION(XSM_HOOK);
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 11218b6..15acb3b 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -61,7 +61,6 @@ struct xsm_operations {
>       int (*domctl) (struct domain *d, int cmd);
>       int (*sysctl) (int cmd);
>       int (*readconsole) (uint32_t clear);
> -    int (*do_mca) (void);
>
>       int (*evtchn_unbound) (struct domain *d, struct evtchn *chn, domid_t id2);
>       int (*evtchn_interdomain) (struct domain *d1, struct evtchn *chn1,
> @@ -139,6 +138,7 @@ struct xsm_operations {
>       int (*hvm_param_nested) (struct domain *d);
>
>   #ifdef CONFIG_X86
> +    int (*do_mca) (void);
>       int (*shadow_control) (struct domain *d, uint32_t op);
>       int (*hvm_set_pci_intx_level) (struct domain *d);
>       int (*hvm_set_isa_irq_level) (struct domain *d);
> @@ -223,11 +223,6 @@ static inline int xsm_readconsole (xsm_default_t def, uint32_t clear)
>       return xsm_ops->readconsole(clear);
>   }
>
> -static inline int xsm_do_mca(xsm_default_t def)
> -{
> -    return xsm_ops->do_mca();
> -}
> -
>   static inline int xsm_evtchn_unbound (xsm_default_t def, struct domain *d1, struct evtchn *chn,
>                                                                       domid_t id2)
>   {
> @@ -524,6 +519,11 @@ static inline int xsm_hvm_param_nested (xsm_default_t def, struct domain *d)
>   }
>
>   #ifdef CONFIG_X86
> +static inline int xsm_do_mca(xsm_default_t def)
> +{
> +    return xsm_ops->do_mca();
> +}
> +
>   static inline int xsm_shadow_control (xsm_default_t def, struct domain *d, uint32_t op)
>   {
>       return xsm_ops->shadow_control(d, op);
> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
> index 627edcc..b79e10f 100644
> --- a/xen/xsm/dummy.c
> +++ b/xen/xsm/dummy.c
> @@ -38,7 +38,6 @@ void xsm_fixup_ops (struct xsm_operations *ops)
>       set_to_dummy_if_null(ops, domctl);
>       set_to_dummy_if_null(ops, sysctl);
>       set_to_dummy_if_null(ops, readconsole);
> -    set_to_dummy_if_null(ops, do_mca);
>
>       set_to_dummy_if_null(ops, evtchn_unbound);
>       set_to_dummy_if_null(ops, evtchn_interdomain);
> @@ -115,6 +114,7 @@ void xsm_fixup_ops (struct xsm_operations *ops)
>       set_to_dummy_if_null(ops, remove_from_physmap);
>
>   #ifdef CONFIG_X86
> +    set_to_dummy_if_null(ops, do_mca);
>       set_to_dummy_if_null(ops, shadow_control);
>       set_to_dummy_if_null(ops, hvm_set_pci_intx_level);
>       set_to_dummy_if_null(ops, hvm_set_isa_irq_level);
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 7329f31..65343f3 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -798,11 +798,6 @@ static int flask_readconsole(uint32_t clear)
>       return domain_has_xen(current->domain, perms);
>   }
>
> -static int flask_do_mca(void)
> -{
> -    return domain_has_xen(current->domain, XEN__MCA_OP);
> -}
> -
>   static inline u32 resource_to_perm(uint8_t access)
>   {
>       if ( access )
> @@ -1169,6 +1164,11 @@ static int flask_deassign_device(struct domain *d, uint32_t machine_bdf)
>   #endif /* HAS_PASSTHROUGH && HAS_PCI */
>
>   #ifdef CONFIG_X86
> +static int flask_do_mca(void)
> +{
> +    return domain_has_xen(current->domain, XEN__MCA_OP);
> +}
> +
>   static int flask_shadow_control(struct domain *d, uint32_t op)
>   {
>       u32 perm;
> @@ -1472,7 +1472,6 @@ static struct xsm_operations flask_ops = {
>       .domctl = flask_domctl,
>       .sysctl = flask_sysctl,
>       .readconsole = flask_readconsole,
> -    .do_mca = flask_do_mca,
>
>       .evtchn_unbound = flask_evtchn_unbound,
>       .evtchn_interdomain = flask_evtchn_interdomain,
> @@ -1551,6 +1550,7 @@ static struct xsm_operations flask_ops = {
>   #endif
>
>   #ifdef CONFIG_X86
> +    .do_mca = flask_do_mca,
>       .shadow_control = flask_shadow_control,
>       .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level,
>       .hvm_set_isa_irq_level = flask_hvm_set_isa_irq_level,
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 06/14] xen/xsm: xsm_do_mca is x86 specific

[Ian Campbell]

On Thu, 2014-03-13 at 10:26 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > xsm_do_mca is only used by x86. Only define the function for x86 to
> > avoid usage on ARM.
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Julien Grall]

The commit f7d29f7b "flask: add compat mode guest support" introduces
build breakage on ARM when XSM is enabled. It's because ARM doesn't use
compat mode.

flask_op.c:794:34: fatal error: compat/event_channel.h: No such file or directory
 #include <compat/event_channel.h>

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/xsm/flask/flask_op.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 3b84679..3eb3a01 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -785,7 +785,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
     return rv;
 }
 
-#ifndef COMPAT
+#if !defined(COMPAT) && defined(CONFIG_COMPAT)
 #undef _copy_to_guest
 #define _copy_to_guest copy_to_compat
 #undef _copy_from_guest
-- 
1.7.10.4

Re: [RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> The commit f7d29f7b "flask: add compat mode guest support" introduces
> build breakage on ARM when XSM is enabled. It's because ARM doesn't use
> compat mode.
>
> flask_op.c:794:34: fatal error: compat/event_channel.h: No such file or directory
>   #include <compat/event_channel.h>
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/xsm/flask/flask_op.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index 3b84679..3eb3a01 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -785,7 +785,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
>       return rv;
>   }
>
> -#ifndef COMPAT
> +#if !defined(COMPAT) && defined(CONFIG_COMPAT)
>   #undef _copy_to_guest
>   #define _copy_to_guest copy_to_compat
>   #undef _copy_from_guest
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Ian Campbell]

On Thu, 2014-03-13 at 10:26 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:

Julien, Did you mean =n in the subject?

> > The commit f7d29f7b "flask: add compat mode guest support" introduces
> > build breakage on ARM when XSM is enabled. It's because ARM doesn't use
> > compat mode.
> >
> > flask_op.c:794:34: fatal error: compat/event_channel.h: No such file or directory
> >   #include <compat/event_channel.h>
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> > ---
> >   xen/xsm/flask/flask_op.c |    2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> > index 3b84679..3eb3a01 100644
> > --- a/xen/xsm/flask/flask_op.c
> > +++ b/xen/xsm/flask/flask_op.c
> > @@ -785,7 +785,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
> >       return rv;
> >   }
> >
> > -#ifndef COMPAT
> > +#if !defined(COMPAT) && defined(CONFIG_COMPAT)

I don't see this construct very much in the existing code. It looks like
the common idiom is just #ifdef CONFIG_COMPAT.

Jan, what's the right thing to do here?

Ian.

Re: [RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Julien Grall]

Hi Ian,

On 03/14/2014 05:23 PM, Ian Campbell wrote:
> On Thu, 2014-03-13 at 10:26 -0400, Daniel De Graaf wrote:
>> On 03/12/2014 12:16 PM, Julien Grall wrote:
> 
> Julien, Did you mean =n in the subject?

Oops, right. I will fix it in the next version.

Regards,

-- 
Julien Grall

Re: [RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Jan Beulich]

>>> On 14.03.14 at 18:23, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2014-03-13 at 10:26 -0400, Daniel De Graaf wrote:
>> On 03/12/2014 12:16 PM, Julien Grall wrote:
> 
> Julien, Did you mean =n in the subject?
> 
>> > The commit f7d29f7b "flask: add compat mode guest support" introduces
>> > build breakage on ARM when XSM is enabled. It's because ARM doesn't use
>> > compat mode.
>> >
>> > flask_op.c:794:34: fatal error: compat/event_channel.h: No such file or 
> directory
>> >   #include <compat/event_channel.h>
>> >
>> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> 
>> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> 
>> > ---
>> >   xen/xsm/flask/flask_op.c |    2 +-
>> >   1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
>> > index 3b84679..3eb3a01 100644
>> > --- a/xen/xsm/flask/flask_op.c
>> > +++ b/xen/xsm/flask/flask_op.c
>> > @@ -785,7 +785,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) 
> u_flask_op)
>> >       return rv;
>> >   }
>> >
>> > -#ifndef COMPAT
>> > +#if !defined(COMPAT) && defined(CONFIG_COMPAT)
> 
> I don't see this construct very much in the existing code. It looks like
> the common idiom is just #ifdef CONFIG_COMPAT.
> 
> Jan, what's the right thing to do here?

The construct is appropriate here (albeit I'd switch the operands
of &&) - elsewhere inclusion goes indirectly, with the two checks
in distinct places. It didn't seem worthwhile introducing a separate
file here for the compat mode wrapping, so the check can validly
be done in a single conditional.

Jan

Re: [RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Ian Campbell]

On Mon, 2014-03-17 at 07:22 +0000, Jan Beulich wrote:
> >>> On 14.03.14 at 18:23, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2014-03-13 at 10:26 -0400, Daniel De Graaf wrote:
> >> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > 
> > Julien, Did you mean =n in the subject?
> > 
> >> > The commit f7d29f7b "flask: add compat mode guest support" introduces
> >> > build breakage on ARM when XSM is enabled. It's because ARM doesn't use
> >> > compat mode.
> >> >
> >> > flask_op.c:794:34: fatal error: compat/event_channel.h: No such file or 
> > directory
> >> >   #include <compat/event_channel.h>
> >> >
> >> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> >> 
> >> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> >> 
> >> > ---
> >> >   xen/xsm/flask/flask_op.c |    2 +-
> >> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> >> > index 3b84679..3eb3a01 100644
> >> > --- a/xen/xsm/flask/flask_op.c
> >> > +++ b/xen/xsm/flask/flask_op.c
> >> > @@ -785,7 +785,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) 
> > u_flask_op)
> >> >       return rv;
> >> >   }
> >> >
> >> > -#ifndef COMPAT
> >> > +#if !defined(COMPAT) && defined(CONFIG_COMPAT)
> > 
> > I don't see this construct very much in the existing code. It looks like
> > the common idiom is just #ifdef CONFIG_COMPAT.
> > 
> > Jan, what's the right thing to do here?
> 
> The construct is appropriate here (albeit I'd switch the operands
> of &&) - elsewhere inclusion goes indirectly, with the two checks
> in distinct places. It didn't seem worthwhile introducing a separate
> file here for the compat mode wrapping, so the check can validly
> be done in a single conditional.

Makes sense, thanks.

Ian.

Re: [RFC 07/14] xen/xsm: flask: Fix compilation when CONFIG_COMPAT=y

[Julien Grall]

Hi Jan,

On 03/17/2014 07:22 AM, Jan Beulich wrote:
> The construct is appropriate here (albeit I'd switch the operands
> of &&) - elsewhere inclusion goes indirectly, with the two checks
> in distinct places. It didn't seem worthwhile introducing a separate
> file here for the compat mode wrapping, so the check can validly
> be done in a single conditional.

I will switch the operand in the next version.

Regards,

-- 
Julien Grall

[RFC 08/14] xen/xsm: flask: Rename variable "bool" in "b"

[Julien Grall]

On ARM, the compilation is failing with the following error:
In file included from flask_op.c:21:0:
./include/conditional.h:24:43: error: two or more data types in declaration specifiers
./include/conditional.h:25:42: error: two or more data types in declaration specifiers

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/xsm/flask/include/conditional.h |    4 ++--
 xen/xsm/flask/ss/services.c         |   14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/xsm/flask/include/conditional.h b/xen/xsm/flask/include/conditional.h
index 043cfd8..9055340 100644
--- a/xen/xsm/flask/include/conditional.h
+++ b/xen/xsm/flask/include/conditional.h
@@ -21,7 +21,7 @@ int security_set_bools(int len, int *values);
 
 int security_find_bool(const char *name);
 
-char *security_get_bool_name(unsigned int bool);
-int security_get_bool_value(unsigned int bool);
+char *security_get_bool_name(unsigned int b);
+int security_get_bool_value(unsigned int b);
 
 #endif
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index aebbec7..59234ff 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1958,7 +1958,7 @@ out:
     return rc;
 }
 
-int security_get_bool_value(unsigned int bool)
+int security_get_bool_value(unsigned int b)
 {
     int rc = 0;
     unsigned int len;
@@ -1966,19 +1966,19 @@ int security_get_bool_value(unsigned int bool)
     POLICY_RDLOCK;
 
     len = policydb.p_bools.nprim;
-    if ( bool >= len )
+    if ( b >= len )
     {
         rc = -ENOENT;
         goto out;
     }
 
-    rc = policydb.bool_val_to_struct[bool]->state;
+    rc = policydb.bool_val_to_struct[b]->state;
 out:
     POLICY_RDUNLOCK;
     return rc;
 }
 
-char *security_get_bool_name(unsigned int bool)
+char *security_get_bool_name(unsigned int b)
 {
     unsigned int len;
     char *rv = NULL;
@@ -1986,16 +1986,16 @@ char *security_get_bool_name(unsigned int bool)
     POLICY_RDLOCK;
 
     len = policydb.p_bools.nprim;
-    if ( bool >= len )
+    if ( b >= len )
     {
         goto out;
     }
 
-    len = strlen(policydb.p_bool_val_to_name[bool]) + 1;
+    len = strlen(policydb.p_bool_val_to_name[b]) + 1;
     rv = xmalloc_array(char, len);
     if ( !rv )
         goto out;
-    memcpy(rv, policydb.p_bool_val_to_name[bool], len);
+    memcpy(rv, policydb.p_bool_val_to_name[b], len);
 out:
     POLICY_RDUNLOCK;
     return rv;
-- 
1.7.10.4

Re: [RFC 08/14] xen/xsm: flask: Rename variable "bool" in "b"

[Andrew Cooper]

On 12/03/14 16:16, Julien Grall wrote:
> On ARM, the compilation is failing with the following error:
> In file included from flask_op.c:21:0:
> ./include/conditional.h:24:43: error: two or more data types in declaration specifiers
> ./include/conditional.h:25:42: error: two or more data types in declaration specifiers
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

I am curious as to why this works on x86.  The name 'bool' is specified
as an alias for '_Bool' by C99/stdboot.h

~Andrew

> ---
>  xen/xsm/flask/include/conditional.h |    4 ++--
>  xen/xsm/flask/ss/services.c         |   14 +++++++-------
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/xen/xsm/flask/include/conditional.h b/xen/xsm/flask/include/conditional.h
> index 043cfd8..9055340 100644
> --- a/xen/xsm/flask/include/conditional.h
> +++ b/xen/xsm/flask/include/conditional.h
> @@ -21,7 +21,7 @@ int security_set_bools(int len, int *values);
>  
>  int security_find_bool(const char *name);
>  
> -char *security_get_bool_name(unsigned int bool);
> -int security_get_bool_value(unsigned int bool);
> +char *security_get_bool_name(unsigned int b);
> +int security_get_bool_value(unsigned int b);
>  
>  #endif
> diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
> index aebbec7..59234ff 100644
> --- a/xen/xsm/flask/ss/services.c
> +++ b/xen/xsm/flask/ss/services.c
> @@ -1958,7 +1958,7 @@ out:
>      return rc;
>  }
>  
> -int security_get_bool_value(unsigned int bool)
> +int security_get_bool_value(unsigned int b)
>  {
>      int rc = 0;
>      unsigned int len;
> @@ -1966,19 +1966,19 @@ int security_get_bool_value(unsigned int bool)
>      POLICY_RDLOCK;
>  
>      len = policydb.p_bools.nprim;
> -    if ( bool >= len )
> +    if ( b >= len )
>      {
>          rc = -ENOENT;
>          goto out;
>      }
>  
> -    rc = policydb.bool_val_to_struct[bool]->state;
> +    rc = policydb.bool_val_to_struct[b]->state;
>  out:
>      POLICY_RDUNLOCK;
>      return rc;
>  }
>  
> -char *security_get_bool_name(unsigned int bool)
> +char *security_get_bool_name(unsigned int b)
>  {
>      unsigned int len;
>      char *rv = NULL;
> @@ -1986,16 +1986,16 @@ char *security_get_bool_name(unsigned int bool)
>      POLICY_RDLOCK;
>  
>      len = policydb.p_bools.nprim;
> -    if ( bool >= len )
> +    if ( b >= len )
>      {
>          goto out;
>      }
>  
> -    len = strlen(policydb.p_bool_val_to_name[bool]) + 1;
> +    len = strlen(policydb.p_bool_val_to_name[b]) + 1;
>      rv = xmalloc_array(char, len);
>      if ( !rv )
>          goto out;
> -    memcpy(rv, policydb.p_bool_val_to_name[bool], len);
> +    memcpy(rv, policydb.p_bool_val_to_name[b], len);
>  out:
>      POLICY_RDUNLOCK;
>      return rv;

Re: [RFC 08/14] xen/xsm: flask: Rename variable "bool" in "b"

[Julien Grall]

Hi Andrew,

On 03/12/2014 04:26 PM, Andrew Cooper wrote:
> On 12/03/14 16:16, Julien Grall wrote:
>> On ARM, the compilation is failing with the following error:
>> In file included from flask_op.c:21:0:
>> ./include/conditional.h:24:43: error: two or more data types in declaration specifiers
>> ./include/conditional.h:25:42: error: two or more data types in declaration specifiers
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> I am curious as to why this works on x86.  The name 'bool' is specified
> as an alias for '_Bool' by C99/stdboot.h

It's because ARM include indirectly stdbool.h via xen/device_tree.h.
I've tried to add "#include <xen/stdbool.h>" in conditional.h and I get
the same compilation error.

Regards,

-- 
Julien Grall

Re: [RFC 08/14] xen/xsm: flask: Rename variable "bool" in "b"

[Jan Beulich]

>>> On 13.03.14 at 14:17, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Andrew,
> 
> On 03/12/2014 04:26 PM, Andrew Cooper wrote:
>> On 12/03/14 16:16, Julien Grall wrote:
>>> On ARM, the compilation is failing with the following error:
>>> In file included from flask_op.c:21:0:
>>> ./include/conditional.h:24:43: error: two or more data types in declaration 
> specifiers
>>> ./include/conditional.h:25:42: error: two or more data types in declaration 
> specifiers
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> 
>> I am curious as to why this works on x86.  The name 'bool' is specified
>> as an alias for '_Bool' by C99/stdboot.h
> 
> It's because ARM include indirectly stdbool.h via xen/device_tree.h.
> I've tried to add "#include <xen/stdbool.h>" in conditional.h and I get
> the same compilation error.

And (at least so far) this isn't intended to be included by arbitrary
code, it exists solely to support code shared with the tools (e.g.
common/libelf/). Which isn't to say that I mind the change - it's
valid cleanup after all.

Jan

Re: [RFC 08/14] xen/xsm: flask: Rename variable "bool" in "b"

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> On ARM, the compilation is failing with the following error:
> In file included from flask_op.c:21:0:
> ./include/conditional.h:24:43: error: two or more data types in declaration specifiers
> ./include/conditional.h:25:42: error: two or more data types in declaration specifiers
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/xsm/flask/include/conditional.h |    4 ++--
>   xen/xsm/flask/ss/services.c         |   14 +++++++-------
>   2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/xen/xsm/flask/include/conditional.h b/xen/xsm/flask/include/conditional.h
> index 043cfd8..9055340 100644
> --- a/xen/xsm/flask/include/conditional.h
> +++ b/xen/xsm/flask/include/conditional.h
> @@ -21,7 +21,7 @@ int security_set_bools(int len, int *values);
>
>   int security_find_bool(const char *name);
>
> -char *security_get_bool_name(unsigned int bool);
> -int security_get_bool_value(unsigned int bool);
> +char *security_get_bool_name(unsigned int b);
> +int security_get_bool_value(unsigned int b);
>
>   #endif
> diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
> index aebbec7..59234ff 100644
> --- a/xen/xsm/flask/ss/services.c
> +++ b/xen/xsm/flask/ss/services.c
> @@ -1958,7 +1958,7 @@ out:
>       return rc;
>   }
>
> -int security_get_bool_value(unsigned int bool)
> +int security_get_bool_value(unsigned int b)
>   {
>       int rc = 0;
>       unsigned int len;
> @@ -1966,19 +1966,19 @@ int security_get_bool_value(unsigned int bool)
>       POLICY_RDLOCK;
>
>       len = policydb.p_bools.nprim;
> -    if ( bool >= len )
> +    if ( b >= len )
>       {
>           rc = -ENOENT;
>           goto out;
>       }
>
> -    rc = policydb.bool_val_to_struct[bool]->state;
> +    rc = policydb.bool_val_to_struct[b]->state;
>   out:
>       POLICY_RDUNLOCK;
>       return rc;
>   }
>
> -char *security_get_bool_name(unsigned int bool)
> +char *security_get_bool_name(unsigned int b)
>   {
>       unsigned int len;
>       char *rv = NULL;
> @@ -1986,16 +1986,16 @@ char *security_get_bool_name(unsigned int bool)
>       POLICY_RDLOCK;
>
>       len = policydb.p_bools.nprim;
> -    if ( bool >= len )
> +    if ( b >= len )
>       {
>           goto out;
>       }
>
> -    len = strlen(policydb.p_bool_val_to_name[bool]) + 1;
> +    len = strlen(policydb.p_bool_val_to_name[b]) + 1;
>       rv = xmalloc_array(char, len);
>       if ( !rv )
>           goto out;
> -    memcpy(rv, policydb.p_bool_val_to_name[bool], len);
> +    memcpy(rv, policydb.p_bool_val_to_name[b], len);
>   out:
>       POLICY_RDUNLOCK;
>       return rv;
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 08/14] xen/xsm: flask: Rename variable "bool" in "b"

[Ian Campbell]

On Thu, 2014-03-13 at 10:27 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > On ARM, the compilation is failing with the following error:
> > In file included from flask_op.c:21:0:
> > ./include/conditional.h:24:43: error: two or more data types in declaration specifiers
> > ./include/conditional.h:25:42: error: two or more data types in declaration specifiers
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Julien Grall]

MSI is not yet support on ARM and will break the compilation when XSM_ENABLE=y.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/xsm/flask/hooks.c |   72 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 65343f3..56c7645 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -19,7 +19,9 @@
 #include <xen/errno.h>
 #include <xen/guest_access.h>
 #include <xen/xenoprof.h>
+#ifdef HAS_PCI
 #include <asm/msi.h>
+#endif
 #include <public/xen.h>
 #include <public/physdev.h>
 #include <public/platform.h>
@@ -100,7 +102,6 @@ static int domain_has_xen(struct domain *d, u32 perms)
 
 static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
 {
-    struct irq_desc *desc = irq_to_desc(irq);
     if ( irq >= nr_irqs || irq < 0 )
         return -EINVAL;
     if ( irq < nr_static_irqs ) {
@@ -110,15 +111,21 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
         }
         return security_irq_sid(irq, sid);
     }
-    if ( desc->msi_desc && desc->msi_desc->dev ) {
-        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;
+#ifdef HAS_PCI
+    {
+        struct irq_desc *desc = irq_to_desc(irq);
+        if ( desc->msi_desc && desc->msi_desc->dev ) {
+            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);
         }
-        return security_device_sid(sbdf, sid);
     }
+#endif
+
     if (ad) {
         AVC_AUDIT_DATA_INIT(ad, IRQ);
         ad->irq = irq;
@@ -825,21 +832,34 @@ static int flask_map_domain_pirq (struct domain *d)
     return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
 }
 
+static int flask_map_domain_msi (struct domain *d, int irq, void *data,
+                                 u32 *sid, struct avc_audit_data *ad)
+{
+#ifdef HAS_PCI
+    struct msi_info *msi = data;
+
+    u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
+    AVC_AUDIT_DATA_INIT(ad, DEV);
+    ad->device = machine_bdf;
+
+    return security_device_sid(machine_bdf, sid);
+#else
+    return -EINVAL;
+#endif
+}
+
 static int flask_map_domain_irq (struct domain *d, int irq, void *data)
 {
     u32 sid, dsid;
     int rc = -EPERM;
-    struct msi_info *msi = data;
     struct avc_audit_data ad;
 
-    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);
+    if ( irq >= nr_static_irqs && data ) {
+        rc = flask_map_domain_msi(d, irq, data, &sid, &ad);
     } else {
         rc = get_irq_sid(irq, &sid, &ad);
     }
+
     if ( rc )
         return rc;
 
@@ -858,18 +878,30 @@ static int flask_unmap_domain_pirq (struct domain *d)
     return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
 }
 
+static int flask_unmap_domain_msi (struct domain *d, int irq, void *data,
+                                   u32 *sid, struct avc_audit_data *ad)
+{
+#ifdef HAS_PCI
+    struct msi_info *msi = data;
+    u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
+
+    AVC_AUDIT_DATA_INIT(ad, DEV);
+    ad->device = machine_bdf;
+
+    return security_device_sid(machine_bdf, sid);
+#else
+    return -EINVAL;
+#endif
+}
+
 static int flask_unmap_domain_irq (struct domain *d, int irq, void *data)
 {
     u32 sid;
     int rc = -EPERM;
-    struct msi_info *msi = data;
     struct avc_audit_data ad;
 
-    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);
+    if ( irq >= nr_static_irqs && data ) {
+        rc = flask_unmap_domain_msi(d, irq, data, &sid, &ad);
     } else {
         rc = get_irq_sid(irq, &sid, &ad);
     }
-- 
1.7.10.4

Re: [RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> MSI is not yet support on ARM and will break the compilation when XSM_ENABLE=y.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

I am assuming that IRQs on ARM are generally allocated in a static manner for
a given board, so that a security policy based on their numeric values would
be effective. If IRQs above nr_static_irqs are dynamic as on x86, it may be
necessary to resolve them in some way. Since this resolution would naturally
be split into a separate patch, in either case this patch is

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/xsm/flask/hooks.c |   72 +++++++++++++++++++++++++++++++++++--------------
>   1 file changed, 52 insertions(+), 20 deletions(-)
>
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 65343f3..56c7645 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -19,7 +19,9 @@
>   #include <xen/errno.h>
>   #include <xen/guest_access.h>
>   #include <xen/xenoprof.h>
> +#ifdef HAS_PCI
>   #include <asm/msi.h>
> +#endif
>   #include <public/xen.h>
>   #include <public/physdev.h>
>   #include <public/platform.h>
> @@ -100,7 +102,6 @@ static int domain_has_xen(struct domain *d, u32 perms)
>
>   static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
>   {
> -    struct irq_desc *desc = irq_to_desc(irq);
>       if ( irq >= nr_irqs || irq < 0 )
>           return -EINVAL;
>       if ( irq < nr_static_irqs ) {
> @@ -110,15 +111,21 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
>           }
>           return security_irq_sid(irq, sid);
>       }
> -    if ( desc->msi_desc && desc->msi_desc->dev ) {
> -        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;
> +#ifdef HAS_PCI
> +    {
> +        struct irq_desc *desc = irq_to_desc(irq);
> +        if ( desc->msi_desc && desc->msi_desc->dev ) {
> +            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);
>           }
> -        return security_device_sid(sbdf, sid);
>       }
> +#endif
> +
>       if (ad) {
>           AVC_AUDIT_DATA_INIT(ad, IRQ);
>           ad->irq = irq;
> @@ -825,21 +832,34 @@ static int flask_map_domain_pirq (struct domain *d)
>       return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD);
>   }
>
> +static int flask_map_domain_msi (struct domain *d, int irq, void *data,
> +                                 u32 *sid, struct avc_audit_data *ad)
> +{
> +#ifdef HAS_PCI
> +    struct msi_info *msi = data;
> +
> +    u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
> +    AVC_AUDIT_DATA_INIT(ad, DEV);
> +    ad->device = machine_bdf;
> +
> +    return security_device_sid(machine_bdf, sid);
> +#else
> +    return -EINVAL;
> +#endif
> +}
> +
>   static int flask_map_domain_irq (struct domain *d, int irq, void *data)
>   {
>       u32 sid, dsid;
>       int rc = -EPERM;
> -    struct msi_info *msi = data;
>       struct avc_audit_data ad;
>
> -    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);
> +    if ( irq >= nr_static_irqs && data ) {
> +        rc = flask_map_domain_msi(d, irq, data, &sid, &ad);
>       } else {
>           rc = get_irq_sid(irq, &sid, &ad);
>       }
> +
>       if ( rc )
>           return rc;
>
> @@ -858,18 +878,30 @@ static int flask_unmap_domain_pirq (struct domain *d)
>       return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE);
>   }
>
> +static int flask_unmap_domain_msi (struct domain *d, int irq, void *data,
> +                                   u32 *sid, struct avc_audit_data *ad)
> +{
> +#ifdef HAS_PCI
> +    struct msi_info *msi = data;
> +    u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
> +
> +    AVC_AUDIT_DATA_INIT(ad, DEV);
> +    ad->device = machine_bdf;
> +
> +    return security_device_sid(machine_bdf, sid);
> +#else
> +    return -EINVAL;
> +#endif
> +}
> +
>   static int flask_unmap_domain_irq (struct domain *d, int irq, void *data)
>   {
>       u32 sid;
>       int rc = -EPERM;
> -    struct msi_info *msi = data;
>       struct avc_audit_data ad;
>
> -    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);
> +    if ( irq >= nr_static_irqs && data ) {
> +        rc = flask_unmap_domain_msi(d, irq, data, &sid, &ad);
>       } else {
>           rc = get_irq_sid(irq, &sid, &ad);
>       }
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Julien Grall]

Hello Daniel,

Thanks for your review.

On 03/13/2014 02:34 PM, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
>> MSI is not yet support on ARM and will break the compilation when
>> XSM_ENABLE=y.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> I am assuming that IRQs on ARM are generally allocated in a static
> manner for
> a given board, so that a security policy based on their numeric values
> would
> be effective. If IRQs above nr_static_irqs are dynamic as on x86, it may be
> necessary to resolve them in some way. Since this resolution would
> naturally
> be split into a separate patch, in either case this patch is

Right, nr_static_irqs is set to 1024 on ARM. For now every IRQs, are not
higher than this number.

Regards,

-- 
Julien Grall

Re: [RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Ian Campbell]

On Thu, 2014-03-13 at 14:40 +0000, Julien Grall wrote:
> Hello Daniel,
> 
> Thanks for your review.
> 
> On 03/13/2014 02:34 PM, Daniel De Graaf wrote:
> > On 03/12/2014 12:16 PM, Julien Grall wrote:
> >> MSI is not yet support on ARM and will break the compilation when
> >> XSM_ENABLE=y.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> > 
> > I am assuming that IRQs on ARM are generally allocated in a static
> > manner for
> > a given board, so that a security policy based on their numeric values
> > would
> > be effective. If IRQs above nr_static_irqs are dynamic as on x86, it may be
> > necessary to resolve them in some way. Since this resolution would
> > naturally
> > be split into a separate patch, in either case this patch is
> 
> Right, nr_static_irqs is set to 1024 on ARM. For now every IRQs, are not
> higher than this number.

This will change before long though. Would HAVE_MSI_IRQS be a better
name for this particular arch-configurable? After all PCI without MSI is
plausible isn't it? (well, maybe not in the modern world)

Ian

Re: [RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Julien Grall]

Hi Ian,

On 03/14/2014 05:25 PM, Ian Campbell wrote:
> On Thu, 2014-03-13 at 14:40 +0000, Julien Grall wrote:
>>
>> Right, nr_static_irqs is set to 1024 on ARM. For now every IRQs, are not
>> higher than this number.
> 
> This will change before long though. Would HAVE_MSI_IRQS be a better
> name for this particular arch-configurable? After all PCI without MSI is
> plausible isn't it? (well, maybe not in the modern world)

Most of PCI passthrough code is relying that MSI is also implemented.
Futhermore, I think the plan for ARM is to support both PCI and MSI.
That's why I chose HAS_PCI.

Regards,

-- 
Julien Grall

Re: [RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Ian Campbell]

On Fri, 2014-03-14 at 18:15 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/14/2014 05:25 PM, Ian Campbell wrote:
> > On Thu, 2014-03-13 at 14:40 +0000, Julien Grall wrote:
> >>
> >> Right, nr_static_irqs is set to 1024 on ARM. For now every IRQs, are not
> >> higher than this number.
> > 
> > This will change before long though. Would HAVE_MSI_IRQS be a better
> > name for this particular arch-configurable? After all PCI without MSI is
> > plausible isn't it? (well, maybe not in the modern world)
> 
> Most of PCI passthrough code is relying that MSI is also implemented.
> Futhermore, I think the plan for ARM is to support both PCI and MSI.

Right, but we might get PCI support first and separately I think?

> That's why I chose HAS_PCI.

I suppose we can revisit it easily enough if it causes problems.

Ian.

Re: [RFC 09/14] xen/xsm: flask: MSI is PCI specific

[Julien Grall]

Hi Ian,

On 03/17/2014 10:13 AM, Ian Campbell wrote:
> On Fri, 2014-03-14 at 18:15 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 03/14/2014 05:25 PM, Ian Campbell wrote:
>>> On Thu, 2014-03-13 at 14:40 +0000, Julien Grall wrote:
>>>>
>>>> Right, nr_static_irqs is set to 1024 on ARM. For now every IRQs, are not
>>>> higher than this number.
>>>
>>> This will change before long though. Would HAVE_MSI_IRQS be a better
>>> name for this particular arch-configurable? After all PCI without MSI is
>>> plausible isn't it? (well, maybe not in the modern world)
>>
>> Most of PCI passthrough code is relying that MSI is also implemented.
>> Futhermore, I think the plan for ARM is to support both PCI and MSI.
> 
> Right, but we might get PCI support first and separately I think?

Good question, I though it's better to come with PCI and MSI support at
the time.

>> That's why I chose HAS_PCI.
> 
> I suppose we can revisit it easily enough if it causes problems.

If we implement first PCI, we might need to modify a couple of other
place in Xen. I think we can delay HAS_MSI_IRQS and see if we really
need it.

Regards,

-- 
Julien Grall

[RFC 10/14] xen/xsm: flask: flask_copying_string is taking a XEN_GUEST_HANDLE as first param

[Julien Grall]

Rather than x86, on ARM XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are
not compatible. This will result to a compilation failure on ARM when XSM
will be enabled.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/xsm/flask/flask_op.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 3eb3a01..7233da9 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -76,7 +76,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,
+static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf,
                                size_t size, size_t max_size)
 {
     char *tmp;
-- 
1.7.10.4

Re: [RFC 10/14] xen/xsm: flask: flask_copying_string is taking a XEN_GUEST_HANDLE as first param

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> Rather than x86, on ARM XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are
> not compatible. This will result to a compilation failure on ARM when XSM
> will be enabled.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/xsm/flask/flask_op.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
> index 3eb3a01..7233da9 100644
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -76,7 +76,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,
> +static int flask_copyin_string(XEN_GUEST_HANDLE(char) u_buf, char **buf,
>                                  size_t size, size_t max_size)
>   {
>       char *tmp;
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 10/14] xen/xsm: flask: flask_copying_string is taking a XEN_GUEST_HANDLE as first param

[Ian Campbell]

On Thu, 2014-03-13 at 10:34 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > Rather than x86, on ARM XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are
> > not compatible. This will result to a compilation failure on ARM when XSM
> > will be enabled.
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[RFC 11/14] xen/xsm: flask: Add missing header in hooks.c

[Julien Grall]

nr_static_irqs and nr_irqs is defined in asm/irq.h (on both x86 and ARM).

Include directly the header in hooks.c to avoid compilation failure on ARM.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/xsm/flask/hooks.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 56c7645..ba719de 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -9,6 +9,7 @@
  */
 
 #include <xen/init.h>
+#include <xen/irq.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <xen/paging.h>
-- 
1.7.10.4

Re: [RFC 11/14] xen/xsm: flask: Add missing header in hooks.c

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> nr_static_irqs and nr_irqs is defined in asm/irq.h (on both x86 and ARM).
>
> Include directly the header in hooks.c to avoid compilation failure on ARM.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/xsm/flask/hooks.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 56c7645..ba719de 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -9,6 +9,7 @@
>    */
>
>   #include <xen/init.h>
> +#include <xen/irq.h>
>   #include <xen/lib.h>
>   #include <xen/sched.h>
>   #include <xen/paging.h>
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 11/14] xen/xsm: flask: Add missing header in hooks.c

[Ian Campbell]

On Thu, 2014-03-13 at 10:34 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > nr_static_irqs and nr_irqs is defined in asm/irq.h (on both x86 and ARM).
> >
> > Include directly the header in hooks.c to avoid compilation failure on ARM.
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[RFC 12/14] xen/xsm: Don't use multiboot by default to initialize XSM

[Julien Grall]

Multiboot protocol is only used by x86. When XSM support for ARM will be
introduced, another way will be used to retrieve the policy blob.

Introduce CONFIG_MULTIBOOT to compile XSM specific multiboot code and
rename xsm_init into xsm_multiboot_init to avoid confusion.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 xen/arch/x86/setup.c         |    2 +-
 xen/include/asm-x86/config.h |    2 ++
 xen/include/xsm/xsm.h        |   23 +++++++++++++++--------
 xen/xsm/xsm_core.c           |    9 ++++++---
 xen/xsm/xsm_policy.c         |   10 +++++++---
 5 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4dbf2b7..e9c2c51 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1251,7 +1251,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     bitmap_fill(module_map, mbi->mods_count);
     __clear_bit(0, module_map); /* Dom0 kernel is always first */
 
-    xsm_init(module_map, mbi, bootstrap_map);
+    xsm_multiboot_init(module_map, mbi, bootstrap_map);
 
     microcode_grab_module(module_map, mbi, bootstrap_map);
 
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 99d0e2c..24f95a3 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -52,6 +52,8 @@
 #define CONFIG_KEXEC 1
 #define CONFIG_WATCHDOG 1
 
+#define CONFIG_MULTIBOOT 1
+
 #define HZ 100
 
 #define OPT_CONSOLE_STR "vga"
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 15acb3b..4863e41 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -643,11 +643,15 @@ static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, str
 
 #endif /* XSM_NO_WRAPPERS */
 
-extern int xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
-                    void *(*bootstrap_map)(const module_t *));
-extern int xsm_policy_init(unsigned long *module_map,
-                           const multiboot_info_t *mbi,
-                           void *(*bootstrap_map)(const module_t *));
+#ifdef CONFIG_MULTIBOOT
+extern int xsm_multiboot_init(unsigned long *module_map,
+                              const multiboot_info_t *mbi,
+                              void *(*bootstrap_map)(const module_t *));
+extern int xsm_multiboot_policy_init(unsigned long *module_map,
+                                     const multiboot_info_t *mbi,
+                                     void *(*bootstrap_map)(const module_t *));
+#endif
+
 extern int register_xsm(struct xsm_operations *ops);
 extern int unregister_xsm(struct xsm_operations *ops);
 
@@ -658,12 +662,15 @@ extern void xsm_fixup_ops(struct xsm_operations *ops);
 
 #include <xsm/dummy.h>
 
-static inline int xsm_init (unsigned long *module_map,
-                            const multiboot_info_t *mbi,
-                            void *(*bootstrap_map)(const module_t *))
+#ifdef CONFIG_MULTIBOOT
+static inline int xsm_multiboot_init (unsigned long *module_map,
+                                      const multiboot_info_t *mbi,
+                                      void *(*bootstrap_map)(const module_t *))
 {
     return 0;
 }
+#endif
+
 #endif /* XSM_ENABLE */
 
 #endif /* __XSM_H */
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index b16c071..11a9ca7 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -43,8 +43,10 @@ static void __init do_xsm_initcalls(void)
     }
 }
 
-int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
-                    void *(*bootstrap_map)(const module_t *))
+#ifdef CONFIG_MULTIBOOT
+int __init xsm_multiboot_init(unsigned long *module_map,
+                              const multiboot_info_t *mbi,
+                              void *(*bootstrap_map)(const module_t *))
 {
     int ret = 0;
 
@@ -52,7 +54,7 @@ int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
 
     if ( XSM_MAGIC )
     {
-        ret = xsm_policy_init(module_map, mbi, bootstrap_map);
+        ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
         if ( ret )
         {
             bootstrap_map(NULL);
@@ -75,6 +77,7 @@ int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
 
     return 0;
 }
+#endif
 
 int register_xsm(struct xsm_operations *ops)
 {
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 0186f30..3d5f66a 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -19,15 +19,18 @@
  */
 
 #include <xsm/xsm.h>
+#ifdef CONFIG_MULTIBOOT
 #include <xen/multiboot.h>
+#endif
 #include <xen/bitops.h>
 
 char *__initdata policy_buffer = NULL;
 u32 __initdata policy_size = 0;
 
-int __init xsm_policy_init(unsigned long *module_map,
-                           const multiboot_info_t *mbi,
-                           void *(*bootstrap_map)(const module_t *))
+#ifdef CONFIG_MULTIBOOT
+int __init xsm_multiboot_policy_init(unsigned long *module_map,
+                                     const multiboot_info_t *mbi,
+                                     void *(*bootstrap_map)(const module_t *))
 {
     int i;
     module_t *mod = (module_t *)__va(mbi->mods_addr);
@@ -65,3 +68,4 @@ int __init xsm_policy_init(unsigned long *module_map,
 
     return rc;
 }
+#endif
-- 
1.7.10.4

Re: [RFC 12/14] xen/xsm: Don't use multiboot by default to initialize XSM

[Jan Beulich]

>>> On 12.03.14 at 17:16, Julien Grall <julien.grall@linaro.org> wrote:
> Multiboot protocol is only used by x86. When XSM support for ARM will be
> introduced, another way will be used to retrieve the policy blob.
> 
> Introduce CONFIG_MULTIBOOT to compile XSM specific multiboot code and
> rename xsm_init into xsm_multiboot_init to avoid confusion.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Keir Fraser <keir@xen.org>

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

> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  xen/arch/x86/setup.c         |    2 +-
>  xen/include/asm-x86/config.h |    2 ++
>  xen/include/xsm/xsm.h        |   23 +++++++++++++++--------
>  xen/xsm/xsm_core.c           |    9 ++++++---
>  xen/xsm/xsm_policy.c         |   10 +++++++---
>  5 files changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 4dbf2b7..e9c2c51 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1251,7 +1251,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      bitmap_fill(module_map, mbi->mods_count);
>      __clear_bit(0, module_map); /* Dom0 kernel is always first */
>  
> -    xsm_init(module_map, mbi, bootstrap_map);
> +    xsm_multiboot_init(module_map, mbi, bootstrap_map);
>  
>      microcode_grab_module(module_map, mbi, bootstrap_map);
>  
> diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
> index 99d0e2c..24f95a3 100644
> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -52,6 +52,8 @@
>  #define CONFIG_KEXEC 1
>  #define CONFIG_WATCHDOG 1
>  
> +#define CONFIG_MULTIBOOT 1
> +
>  #define HZ 100
>  
>  #define OPT_CONSOLE_STR "vga"
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 15acb3b..4863e41 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -643,11 +643,15 @@ static inline int xsm_map_gmfn_foreign (xsm_default_t 
> def, struct domain *d, str
>  
>  #endif /* XSM_NO_WRAPPERS */
>  
> -extern int xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
> -                    void *(*bootstrap_map)(const module_t *));
> -extern int xsm_policy_init(unsigned long *module_map,
> -                           const multiboot_info_t *mbi,
> -                           void *(*bootstrap_map)(const module_t *));
> +#ifdef CONFIG_MULTIBOOT
> +extern int xsm_multiboot_init(unsigned long *module_map,
> +                              const multiboot_info_t *mbi,
> +                              void *(*bootstrap_map)(const module_t *));
> +extern int xsm_multiboot_policy_init(unsigned long *module_map,
> +                                     const multiboot_info_t *mbi,
> +                                     void *(*bootstrap_map)(const module_t 
> *));
> +#endif
> +
>  extern int register_xsm(struct xsm_operations *ops);
>  extern int unregister_xsm(struct xsm_operations *ops);
>  
> @@ -658,12 +662,15 @@ extern void xsm_fixup_ops(struct xsm_operations *ops);
>  
>  #include <xsm/dummy.h>
>  
> -static inline int xsm_init (unsigned long *module_map,
> -                            const multiboot_info_t *mbi,
> -                            void *(*bootstrap_map)(const module_t *))
> +#ifdef CONFIG_MULTIBOOT
> +static inline int xsm_multiboot_init (unsigned long *module_map,
> +                                      const multiboot_info_t *mbi,
> +                                      void *(*bootstrap_map)(const module_t 
> *))
>  {
>      return 0;
>  }
> +#endif
> +
>  #endif /* XSM_ENABLE */
>  
>  #endif /* __XSM_H */
> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
> index b16c071..11a9ca7 100644
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -43,8 +43,10 @@ static void __init do_xsm_initcalls(void)
>      }
>  }
>  
> -int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
> -                    void *(*bootstrap_map)(const module_t *))
> +#ifdef CONFIG_MULTIBOOT
> +int __init xsm_multiboot_init(unsigned long *module_map,
> +                              const multiboot_info_t *mbi,
> +                              void *(*bootstrap_map)(const module_t *))
>  {
>      int ret = 0;
>  
> @@ -52,7 +54,7 @@ int __init xsm_init(unsigned long *module_map, const 
> multiboot_info_t *mbi,
>  
>      if ( XSM_MAGIC )
>      {
> -        ret = xsm_policy_init(module_map, mbi, bootstrap_map);
> +        ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
>          if ( ret )
>          {
>              bootstrap_map(NULL);
> @@ -75,6 +77,7 @@ int __init xsm_init(unsigned long *module_map, const 
> multiboot_info_t *mbi,
>  
>      return 0;
>  }
> +#endif
>  
>  int register_xsm(struct xsm_operations *ops)
>  {
> diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
> index 0186f30..3d5f66a 100644
> --- a/xen/xsm/xsm_policy.c
> +++ b/xen/xsm/xsm_policy.c
> @@ -19,15 +19,18 @@
>   */
>  
>  #include <xsm/xsm.h>
> +#ifdef CONFIG_MULTIBOOT
>  #include <xen/multiboot.h>
> +#endif
>  #include <xen/bitops.h>
>  
>  char *__initdata policy_buffer = NULL;
>  u32 __initdata policy_size = 0;
>  
> -int __init xsm_policy_init(unsigned long *module_map,
> -                           const multiboot_info_t *mbi,
> -                           void *(*bootstrap_map)(const module_t *))
> +#ifdef CONFIG_MULTIBOOT
> +int __init xsm_multiboot_policy_init(unsigned long *module_map,
> +                                     const multiboot_info_t *mbi,
> +                                     void *(*bootstrap_map)(const module_t 
> *))
>  {
>      int i;
>      module_t *mod = (module_t *)__va(mbi->mods_addr);
> @@ -65,3 +68,4 @@ int __init xsm_policy_init(unsigned long *module_map,
>  
>      return rc;
>  }
> +#endif
> -- 
> 1.7.10.4

Re: [RFC 12/14] xen/xsm: Don't use multiboot by default to initialize XSM

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> Multiboot protocol is only used by x86. When XSM support for ARM will be
> introduced, another way will be used to retrieve the policy blob.
>
> Introduce CONFIG_MULTIBOOT to compile XSM specific multiboot code and
> rename xsm_init into xsm_multiboot_init to avoid confusion.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   xen/arch/x86/setup.c         |    2 +-
>   xen/include/asm-x86/config.h |    2 ++
>   xen/include/xsm/xsm.h        |   23 +++++++++++++++--------
>   xen/xsm/xsm_core.c           |    9 ++++++---
>   xen/xsm/xsm_policy.c         |   10 +++++++---
>   5 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 4dbf2b7..e9c2c51 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1251,7 +1251,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>       bitmap_fill(module_map, mbi->mods_count);
>       __clear_bit(0, module_map); /* Dom0 kernel is always first */
>
> -    xsm_init(module_map, mbi, bootstrap_map);
> +    xsm_multiboot_init(module_map, mbi, bootstrap_map);
>
>       microcode_grab_module(module_map, mbi, bootstrap_map);
>
> diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
> index 99d0e2c..24f95a3 100644
> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -52,6 +52,8 @@
>   #define CONFIG_KEXEC 1
>   #define CONFIG_WATCHDOG 1
>
> +#define CONFIG_MULTIBOOT 1
> +
>   #define HZ 100
>
>   #define OPT_CONSOLE_STR "vga"
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 15acb3b..4863e41 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -643,11 +643,15 @@ static inline int xsm_map_gmfn_foreign (xsm_default_t def, struct domain *d, str
>
>   #endif /* XSM_NO_WRAPPERS */
>
> -extern int xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
> -                    void *(*bootstrap_map)(const module_t *));
> -extern int xsm_policy_init(unsigned long *module_map,
> -                           const multiboot_info_t *mbi,
> -                           void *(*bootstrap_map)(const module_t *));
> +#ifdef CONFIG_MULTIBOOT
> +extern int xsm_multiboot_init(unsigned long *module_map,
> +                              const multiboot_info_t *mbi,
> +                              void *(*bootstrap_map)(const module_t *));
> +extern int xsm_multiboot_policy_init(unsigned long *module_map,
> +                                     const multiboot_info_t *mbi,
> +                                     void *(*bootstrap_map)(const module_t *));
> +#endif
> +
>   extern int register_xsm(struct xsm_operations *ops);
>   extern int unregister_xsm(struct xsm_operations *ops);
>
> @@ -658,12 +662,15 @@ extern void xsm_fixup_ops(struct xsm_operations *ops);
>
>   #include <xsm/dummy.h>
>
> -static inline int xsm_init (unsigned long *module_map,
> -                            const multiboot_info_t *mbi,
> -                            void *(*bootstrap_map)(const module_t *))
> +#ifdef CONFIG_MULTIBOOT
> +static inline int xsm_multiboot_init (unsigned long *module_map,
> +                                      const multiboot_info_t *mbi,
> +                                      void *(*bootstrap_map)(const module_t *))
>   {
>       return 0;
>   }
> +#endif
> +
>   #endif /* XSM_ENABLE */
>
>   #endif /* __XSM_H */
> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
> index b16c071..11a9ca7 100644
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -43,8 +43,10 @@ static void __init do_xsm_initcalls(void)
>       }
>   }
>
> -int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
> -                    void *(*bootstrap_map)(const module_t *))
> +#ifdef CONFIG_MULTIBOOT
> +int __init xsm_multiboot_init(unsigned long *module_map,
> +                              const multiboot_info_t *mbi,
> +                              void *(*bootstrap_map)(const module_t *))
>   {
>       int ret = 0;
>
> @@ -52,7 +54,7 @@ int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
>
>       if ( XSM_MAGIC )
>       {
> -        ret = xsm_policy_init(module_map, mbi, bootstrap_map);
> +        ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
>           if ( ret )
>           {
>               bootstrap_map(NULL);
> @@ -75,6 +77,7 @@ int __init xsm_init(unsigned long *module_map, const multiboot_info_t *mbi,
>
>       return 0;
>   }
> +#endif
>
>   int register_xsm(struct xsm_operations *ops)
>   {
> diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
> index 0186f30..3d5f66a 100644
> --- a/xen/xsm/xsm_policy.c
> +++ b/xen/xsm/xsm_policy.c
> @@ -19,15 +19,18 @@
>    */
>
>   #include <xsm/xsm.h>
> +#ifdef CONFIG_MULTIBOOT
>   #include <xen/multiboot.h>
> +#endif
>   #include <xen/bitops.h>
>
>   char *__initdata policy_buffer = NULL;
>   u32 __initdata policy_size = 0;
>
> -int __init xsm_policy_init(unsigned long *module_map,
> -                           const multiboot_info_t *mbi,
> -                           void *(*bootstrap_map)(const module_t *))
> +#ifdef CONFIG_MULTIBOOT
> +int __init xsm_multiboot_policy_init(unsigned long *module_map,
> +                                     const multiboot_info_t *mbi,
> +                                     void *(*bootstrap_map)(const module_t *))
>   {
>       int i;
>       module_t *mod = (module_t *)__va(mbi->mods_addr);
> @@ -65,3 +68,4 @@ int __init xsm_policy_init(unsigned long *module_map,
>
>       return rc;
>   }
> +#endif
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 12/14] xen/xsm: Don't use multiboot by default to initialize XSM

[Ian Campbell]

On Thu, 2014-03-13 at 10:36 -0400, Daniel De Graaf wrote:
> On 03/12/2014 12:16 PM, Julien Grall wrote:
> > Multiboot protocol is only used by x86. When XSM support for ARM will be
> > introduced, another way will be used to retrieve the policy blob.
> >
> > Introduce CONFIG_MULTIBOOT to compile XSM specific multiboot code and
> > rename xsm_init into xsm_multiboot_init to avoid confusion.
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Keir Fraser <keir@xen.org>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

FWIW: Acked-by: Ian Campbell <ian.campbell@citrix.com>

[RFC 13/14] xen/xsm: Add support for device tree

[Julien Grall]

This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
policy when Xen is booting.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 docs/misc/arm/device-tree/booting.txt |    1 +
 xen/common/device_tree.c              |    2 ++
 xen/include/xen/device_tree.h         |    3 ++-
 xen/include/xsm/xsm.h                 |   12 +++++++++++
 xen/xsm/xsm_core.c                    |   37 +++++++++++++++++++++++++++++++++
 xen/xsm/xsm_policy.c                  |   37 +++++++++++++++++++++++++++++++++
 6 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 07fde27..85988fb 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -16,6 +16,7 @@ Each node contains the following properties:
 
 	- "linux-zimage" -- the dom0 kernel
 	- "linux-initrd" -- the dom0 ramdisk
+	- "xsm-blob"	 -- XSM policy blob
 
 - reg
 
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 55716a8..91146fb 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -354,6 +354,8 @@ static void __init process_multiboot_node(const void *fdt, int node,
         nr = MOD_KERNEL;
     else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
         nr = MOD_INITRD;
+    else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-blob") == 0 )
+        nr = MOD_XSM;
     else
         early_panic("%s not a known xen multiboot type\n", name);
 
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 9a8c3de..76faf11 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -24,7 +24,8 @@
 #define MOD_FDT    1
 #define MOD_KERNEL 2
 #define MOD_INITRD 3
-#define NR_MODULES 4
+#define MOD_XSM    4
+#define NR_MODULES 5
 
 #define MOD_DISCARD_FIRST MOD_FDT
 
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4863e41..2cd3a3b 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -652,6 +652,11 @@ extern int xsm_multiboot_policy_init(unsigned long *module_map,
                                      void *(*bootstrap_map)(const module_t *));
 #endif
 
+#ifdef HAS_DEVICE_TREE
+extern int xsm_dt_init(void);
+extern int xsm_dt_policy_init(void);
+#endif
+
 extern int register_xsm(struct xsm_operations *ops);
 extern int unregister_xsm(struct xsm_operations *ops);
 
@@ -671,6 +676,13 @@ static inline int xsm_multiboot_init (unsigned long *module_map,
 }
 #endif
 
+#ifdef HAS_DEVICE_TREE
+static inline int xsm_dt_init(void)
+{
+    return 0;
+}
+#endif
+
 #endif /* XSM_ENABLE */
 
 #endif /* __XSM_H */
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 11a9ca7..755a5dd 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -79,6 +79,43 @@ int __init xsm_multiboot_init(unsigned long *module_map,
 }
 #endif
 
+#ifdef HAS_DEVICE_TREE
+int __init xsm_dt_init(void)
+{
+    int ret = 0;
+
+    printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
+
+    if ( XSM_MAGIC )
+    {
+        ret = xsm_dt_policy_init();
+        if ( ret )
+        {
+            printk("%s: Error initializing policy (rc = %d).\n",
+                   __FUNCTION__, ret);
+            return -EINVAL;
+        }
+    }
+
+    if ( verify(&dummy_xsm_ops) )
+    {
+        printk("%s could not verify dummy_xsm_ops structure.\n",
+               __FUNCTION__);
+        ret = -EIO;
+        goto err;
+    }
+
+    xsm_ops = &dummy_xsm_ops;
+    do_xsm_initcalls();
+
+err:
+    if ( policy_buffer )
+        xfree(policy_buffer);
+
+    return ret;
+}
+#endif
+
 int register_xsm(struct xsm_operations *ops)
 {
     if ( verify(ops) )
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 3d5f66a..a0dee09 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -23,6 +23,10 @@
 #include <xen/multiboot.h>
 #endif
 #include <xen/bitops.h>
+#ifdef HAS_DEVICE_TREE
+# include <asm/setup.h>
+# include <xen/device_tree.h>
+#endif
 
 char *__initdata policy_buffer = NULL;
 u32 __initdata policy_size = 0;
@@ -69,3 +73,36 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
     return rc;
 }
 #endif
+
+#ifdef HAS_DEVICE_TREE
+int __init xsm_dt_policy_init(void)
+{
+    paddr_t paddr = early_info.modules.module[MOD_XSM].start;
+    paddr_t len = early_info.modules.module[MOD_XSM].size;
+    xsm_magic_t magic;
+
+    if ( !len )
+        return 0;
+
+    copy_from_paddr(&magic, paddr, sizeof(magic));
+
+    if ( magic != XSM_MAGIC )
+    {
+        printk(XENLOG_ERR "xsm: Invalid magic for XSM blob got 0x%x "
+               "expected 0x%x\n", magic, XSM_MAGIC);
+        return -EINVAL;
+    }
+
+    printk("xsm: Policy len = 0x%"PRIpaddr" start at 0x%"PRIpaddr"\n",
+           len, paddr);
+
+    policy_buffer = xmalloc_bytes(len);
+    if ( !policy_buffer )
+        return -ENOMEM;
+
+    copy_from_paddr(policy_buffer, paddr, len);
+    policy_size = len;
+
+    return 0;
+}
+#endif
-- 
1.7.10.4

Re: [RFC 13/14] xen/xsm: Add support for device tree

[Daniel De Graaf]

On 03/12/2014 12:16 PM, Julien Grall wrote:
> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
> policy when Xen is booting.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

> ---
>   docs/misc/arm/device-tree/booting.txt |    1 +
>   xen/common/device_tree.c              |    2 ++
>   xen/include/xen/device_tree.h         |    3 ++-
>   xen/include/xsm/xsm.h                 |   12 +++++++++++
>   xen/xsm/xsm_core.c                    |   37 +++++++++++++++++++++++++++++++++
>   xen/xsm/xsm_policy.c                  |   37 +++++++++++++++++++++++++++++++++
>   6 files changed, 91 insertions(+), 1 deletion(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index 07fde27..85988fb 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -16,6 +16,7 @@ Each node contains the following properties:
>
>   	- "linux-zimage" -- the dom0 kernel
>   	- "linux-initrd" -- the dom0 ramdisk
> +	- "xsm-blob"	 -- XSM policy blob
>
>   - reg
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 55716a8..91146fb 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -354,6 +354,8 @@ static void __init process_multiboot_node(const void *fdt, int node,
>           nr = MOD_KERNEL;
>       else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
>           nr = MOD_INITRD;
> +    else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-blob") == 0 )
> +        nr = MOD_XSM;
>       else
>           early_panic("%s not a known xen multiboot type\n", name);
>
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 9a8c3de..76faf11 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -24,7 +24,8 @@
>   #define MOD_FDT    1
>   #define MOD_KERNEL 2
>   #define MOD_INITRD 3
> -#define NR_MODULES 4
> +#define MOD_XSM    4
> +#define NR_MODULES 5
>
>   #define MOD_DISCARD_FIRST MOD_FDT
>
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 4863e41..2cd3a3b 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -652,6 +652,11 @@ extern int xsm_multiboot_policy_init(unsigned long *module_map,
>                                        void *(*bootstrap_map)(const module_t *));
>   #endif
>
> +#ifdef HAS_DEVICE_TREE
> +extern int xsm_dt_init(void);
> +extern int xsm_dt_policy_init(void);
> +#endif
> +
>   extern int register_xsm(struct xsm_operations *ops);
>   extern int unregister_xsm(struct xsm_operations *ops);
>
> @@ -671,6 +676,13 @@ static inline int xsm_multiboot_init (unsigned long *module_map,
>   }
>   #endif
>
> +#ifdef HAS_DEVICE_TREE
> +static inline int xsm_dt_init(void)
> +{
> +    return 0;
> +}
> +#endif
> +
>   #endif /* XSM_ENABLE */
>
>   #endif /* __XSM_H */
> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
> index 11a9ca7..755a5dd 100644
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -79,6 +79,43 @@ int __init xsm_multiboot_init(unsigned long *module_map,
>   }
>   #endif
>
> +#ifdef HAS_DEVICE_TREE
> +int __init xsm_dt_init(void)
> +{
> +    int ret = 0;
> +
> +    printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
> +
> +    if ( XSM_MAGIC )
> +    {
> +        ret = xsm_dt_policy_init();
> +        if ( ret )
> +        {
> +            printk("%s: Error initializing policy (rc = %d).\n",
> +                   __FUNCTION__, ret);
> +            return -EINVAL;
> +        }
> +    }
> +
> +    if ( verify(&dummy_xsm_ops) )
> +    {
> +        printk("%s could not verify dummy_xsm_ops structure.\n",
> +               __FUNCTION__);
> +        ret = -EIO;
> +        goto err;
> +    }
> +
> +    xsm_ops = &dummy_xsm_ops;
> +    do_xsm_initcalls();
> +
> +err:
> +    if ( policy_buffer )
> +        xfree(policy_buffer);
> +
> +    return ret;
> +}
> +#endif
> +
>   int register_xsm(struct xsm_operations *ops)
>   {
>       if ( verify(ops) )
> diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
> index 3d5f66a..a0dee09 100644
> --- a/xen/xsm/xsm_policy.c
> +++ b/xen/xsm/xsm_policy.c
> @@ -23,6 +23,10 @@
>   #include <xen/multiboot.h>
>   #endif
>   #include <xen/bitops.h>
> +#ifdef HAS_DEVICE_TREE
> +# include <asm/setup.h>
> +# include <xen/device_tree.h>
> +#endif
>
>   char *__initdata policy_buffer = NULL;
>   u32 __initdata policy_size = 0;
> @@ -69,3 +73,36 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
>       return rc;
>   }
>   #endif
> +
> +#ifdef HAS_DEVICE_TREE
> +int __init xsm_dt_policy_init(void)
> +{
> +    paddr_t paddr = early_info.modules.module[MOD_XSM].start;
> +    paddr_t len = early_info.modules.module[MOD_XSM].size;
> +    xsm_magic_t magic;
> +
> +    if ( !len )
> +        return 0;
> +
> +    copy_from_paddr(&magic, paddr, sizeof(magic));
> +
> +    if ( magic != XSM_MAGIC )
> +    {
> +        printk(XENLOG_ERR "xsm: Invalid magic for XSM blob got 0x%x "
> +               "expected 0x%x\n", magic, XSM_MAGIC);
> +        return -EINVAL;
> +    }
> +
> +    printk("xsm: Policy len = 0x%"PRIpaddr" start at 0x%"PRIpaddr"\n",
> +           len, paddr);
> +
> +    policy_buffer = xmalloc_bytes(len);
> +    if ( !policy_buffer )
> +        return -ENOMEM;
> +
> +    copy_from_paddr(policy_buffer, paddr, len);
> +    policy_size = len;
> +
> +    return 0;
> +}
> +#endif
>


-- 
Daniel De Graaf
National Security Agency

Re: [RFC 13/14] xen/xsm: Add support for device tree

[Ian Campbell]

On Wed, 2014-03-12 at 16:16 +0000, Julien Grall wrote:
> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
> policy when Xen is booting.

While xsm_dt_policy_init does seem DT specific the xem_dt_init is very
similar to the xsm_multiboot_init. Can they not be a common function
with #ifdef HAVE_MULTIBOOT / #ifdef HAVE_DEVICETREE around the relevant
specific bits (essentially just the call to the relevant policy_init)?
Or at least refactor the tail of xsm_init into xsm_core_init which both
cases can call into.

Re: [RFC 13/14] xen/xsm: Add support for device tree

[Julien Grall]

Hi Ian,

On 03/14/2014 05:34 PM, Ian Campbell wrote:
> On Wed, 2014-03-12 at 16:16 +0000, Julien Grall wrote:
>> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
>> policy when Xen is booting.
> 
> While xsm_dt_policy_init does seem DT specific the xem_dt_init is very
> similar to the xsm_multiboot_init. Can they not be a common function
> with #ifdef HAVE_MULTIBOOT / #ifdef HAVE_DEVICETREE around the relevant
> specific bits (essentially just the call to the relevant policy_init)?
> Or at least refactor the tail of xsm_init into xsm_core_init which both
> cases can call into.

The parameters of the function is not the same and it seems stupid to
also ifdef the list of arguments :).

I can at least create a new function with

    if ( verify(&dummy_xsm_ops) )
    {
        printk("%s could not verify dummy_xsm_ops structure.\n",
               __FUNCTION__);
        ret = -EIO;
        goto err;
    }

    xsm_ops = &dummy_xsm_ops;
    do_xsm_initcalls();

Regards,

-- 
Julien Grall

Re: [RFC 13/14] xen/xsm: Add support for device tree

[Ian Campbell]

On Fri, 2014-03-14 at 18:24 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/14/2014 05:34 PM, Ian Campbell wrote:
> > On Wed, 2014-03-12 at 16:16 +0000, Julien Grall wrote:
> >> This patch adds a new module "xen,xsm-blob" to allow the user to load the XSM
> >> policy when Xen is booting.
> > 
> > While xsm_dt_policy_init does seem DT specific the xem_dt_init is very
> > similar to the xsm_multiboot_init. Can they not be a common function
> > with #ifdef HAVE_MULTIBOOT / #ifdef HAVE_DEVICETREE around the relevant
> > specific bits (essentially just the call to the relevant policy_init)?
> > Or at least refactor the tail of xsm_init into xsm_core_init which both
> > cases can call into.
> 
> The parameters of the function is not the same and it seems stupid to
> also ifdef the list of arguments :).
> 
> I can at least create a new function with
> 
>     if ( verify(&dummy_xsm_ops) )
>     {
>         printk("%s could not verify dummy_xsm_ops structure.\n",
>                __FUNCTION__);
>         ret = -EIO;
>         goto err;
>     }
> 
>     xsm_ops = &dummy_xsm_ops;
>     do_xsm_initcalls();

My main concern was new code getting added to the generic tail of the
init function, so putting that into a function sounds like a good idea
to me. 

Ian.

[RFC 14/14] xen/arm: Add support for XSM

[Julien Grall]

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/setup.c |    2 ++
 xen/arch/arm/traps.c |    1 +
 2 files changed, 3 insertions(+)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 31d853d..3bfa704 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -43,6 +43,7 @@
 #include <asm/gic.h>
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
+#include <xsm/xsm.h>
 
 struct cpuinfo_arm __read_mostly boot_cpu_data;
 
@@ -718,6 +719,7 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     gic_route_ppis();
     gic_route_spis();
+    xsm_dt_init();
 
     init_maintenance_interrupt();
     init_timer_interrupt();
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 21c7b26..f967d2f 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1006,6 +1006,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
     HYPERCALL(sched_op, 2),
     HYPERCALL(console_io, 3),
     HYPERCALL(xen_version, 2),
+    HYPERCALL(xsm_op, 1),
     HYPERCALL(event_channel_op, 2),
     HYPERCALL(physdev_op, 2),
     HYPERCALL(sysctl, 2),
-- 
1.7.10.4

Re: [RFC 14/14] xen/arm: Add support for XSM

[Ian Campbell]

On Wed, 2014-03-12 at 16:16 +0000, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>