[RFC for-4.8 v2 0/7] xen/arm: Add support for mapping mmio-sram nodes into dom0

[RFC for-4.8 v2 0/7] xen/arm: Add support for mapping mmio-sram nodes into dom0

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

This series adds support for mapping mmio-sram nodes into dom0
as normal uncached MEMORY with RWX perms.

If the no-memory-wc prop is present, we create DEVICE RW mappings.

Patch 6 is a bit of a hack to avoid remapping the mmio-sram
sub-area nodes. These nodes represent allocations within the
sram and not mappings that Xen should create.

Comments welcome!

Best regards,
Edgar

ChangeLog:

v1 -> v2
* Replace the .map method with a list of match -> map attributes
* Handle no-memory-wc by mapping as DEVICE RW
* Generalize map_regions_rw_cache instead of adding new functions

Edgar E. Iglesias (7):
  xen/arm: Add MATTR_MEM_NC for normal non-cacheable memory
  xen/arm: Rename and generalize un/map_regions_rw_cache
  xen/device-tree: Add __DT_MATCH macros without braces
  xen/device-tree: Make dt_match_node match props
  xen/arm: domain_build: Plumb for different mapping attributes
  xen/device-tree: Add an mmio-sram bus
  xen/arm: Map mmio-sram nodes as normal un-cached rwx memory

 xen/arch/arm/domain_build.c   | 112 +++++++++++++++++++++++++++++++++---------
 xen/arch/arm/p2m.c            |  29 ++++++-----
 xen/common/device_tree.c      |  46 ++++++++++++++++-
 xen/include/asm-arm/p2m.h     |  22 +++++----
 xen/include/asm-arm/page.h    |   1 +
 xen/include/xen/device_tree.h |  16 ++++--
 6 files changed, 177 insertions(+), 49 deletions(-)

-- 
2.5.0


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

[RFC for-4.8 v2 1/7] xen/arm: Add MATTR_MEM_NC for normal non-cacheable memory

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add the MATTR_MEM_NC macro describing normal non-cacheable memory.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/arch/arm/p2m.c         | 1 +
 xen/include/asm-arm/page.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 838d004..0f1e1b1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -343,6 +343,7 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr,
         e.p2m.sh = LPAE_SH_INNER;
         break;
 
+    case MATTR_MEM_NC:
     case MATTR_DEV:
         e.p2m.sh = LPAE_SH_OUTER;
         break;
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index a94e978..45d2e2c 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -73,6 +73,7 @@
  *
  */
 #define MATTR_DEV     0x1
+#define MATTR_MEM_NC  0x5
 #define MATTR_MEM     0xf
 
 /* Flags for get_page_from_gva, gvirt_to_maddr etc */
-- 
2.5.0


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

Re: [RFC for-4.8 v2 1/7] xen/arm: Add MATTR_MEM_NC for normal non-cacheable memory

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add the MATTR_MEM_NC macro describing normal non-cacheable memory.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>   xen/arch/arm/p2m.c         | 1 +
>   xen/include/asm-arm/page.h | 1 +
>   2 files changed, 2 insertions(+)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 838d004..0f1e1b1 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -343,6 +343,7 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr,
>           e.p2m.sh = LPAE_SH_INNER;
>           break;
>
> +    case MATTR_MEM_NC:

I would add a comment here to explain why non-cacheable memory should 
always be outer-shareable (see DDI 0406C.b B3-1376 to 1377).

For ideas on what to write, see mfn_to_xen_entry in include/asm-arm/page.h

>       case MATTR_DEV:
>           e.p2m.sh = LPAE_SH_OUTER;
>           break;
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index a94e978..45d2e2c 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -73,6 +73,7 @@
>    *
>    */
>   #define MATTR_DEV     0x1
> +#define MATTR_MEM_NC  0x5
>   #define MATTR_MEM     0xf
>
>   /* Flags for get_page_from_gva, gvirt_to_maddr etc */
>

Regards,

-- 
Julien Grall

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

Re: [RFC for-4.8 v2 1/7] xen/arm: Add MATTR_MEM_NC for normal non-cacheable memory

[Edgar E. Iglesias]

On Mon, Jun 06, 2016 at 06:23:47PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 03/06/16 14:29, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> >Add the MATTR_MEM_NC macro describing normal non-cacheable memory.
> >
> >Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> >---
> >  xen/arch/arm/p2m.c         | 1 +
> >  xen/include/asm-arm/page.h | 1 +
> >  2 files changed, 2 insertions(+)
> >
> >diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >index 838d004..0f1e1b1 100644
> >--- a/xen/arch/arm/p2m.c
> >+++ b/xen/arch/arm/p2m.c
> >@@ -343,6 +343,7 @@ static lpae_t mfn_to_p2m_entry(unsigned long mfn, unsigned int mattr,
> >          e.p2m.sh = LPAE_SH_INNER;
> >          break;
> >
> >+    case MATTR_MEM_NC:
> 
> I would add a comment here to explain why non-cacheable memory should always
> be outer-shareable (see DDI 0406C.b B3-1376 to 1377).
> 
> For ideas on what to write, see mfn_to_xen_entry in include/asm-arm/page.h

Thanks, I've added a comment for v3.

CHeers,
Edgar


> 
> >      case MATTR_DEV:
> >          e.p2m.sh = LPAE_SH_OUTER;
> >          break;
> >diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> >index a94e978..45d2e2c 100644
> >--- a/xen/include/asm-arm/page.h
> >+++ b/xen/include/asm-arm/page.h
> >@@ -73,6 +73,7 @@
> >   *
> >   */
> >  #define MATTR_DEV     0x1
> >+#define MATTR_MEM_NC  0x5
> >  #define MATTR_MEM     0xf
> >
> >  /* Flags for get_page_from_gva, gvirt_to_maddr etc */
> >
> 
> Regards,
> 
> -- 
> Julien Grall

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

[RFC for-4.8 v2 2/7] xen/arm: Rename and generalize un/map_regions_rw_cache

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Rename and generalize un/map_regions_rw_cache into
un/map_regions. The new functions take the mapping
attributes and access permissions as arguments.

No functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/arch/arm/domain_build.c | 18 ++++++++++--------
 xen/arch/arm/p2m.c          | 28 ++++++++++++++++------------
 xen/include/asm-arm/p2m.h   | 22 +++++++++++++---------
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 00dc07a..e4fed4b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1570,10 +1570,11 @@ static void acpi_map_other_tables(struct domain *d)
     {
         addr = acpi_gbl_root_table_list.tables[i].address;
         size = acpi_gbl_root_table_list.tables[i].length;
-        res = map_regions_rw_cache(d,
-                                   paddr_to_pfn(addr & PAGE_MASK),
-                                   DIV_ROUND_UP(size, PAGE_SIZE),
-                                   paddr_to_pfn(addr & PAGE_MASK));
+        res = map_regions(d,
+                          paddr_to_pfn(addr & PAGE_MASK),
+                          DIV_ROUND_UP(size, PAGE_SIZE),
+                          paddr_to_pfn(addr & PAGE_MASK),
+                          MATTR_MEM, p2m_access_rw);
         if ( res )
         {
              panic(XENLOG_ERR "Unable to map ACPI region 0x%"PRIx64
@@ -1926,10 +1927,11 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     acpi_create_efi_mmap_table(d, &kinfo->mem, tbl_add);
 
     /* Map the EFI and ACPI tables to Dom0 */
-    rc = map_regions_rw_cache(d,
-                              paddr_to_pfn(d->arch.efi_acpi_gpa),
-                              PFN_UP(d->arch.efi_acpi_len),
-                              paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
+    rc = map_regions(d,
+                     paddr_to_pfn(d->arch.efi_acpi_gpa),
+                     PFN_UP(d->arch.efi_acpi_len),
+                     paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)),
+                     MATTR_MEM, p2m_access_rw);
     if ( rc != 0 )
     {
         printk(XENLOG_ERR "Unable to map EFI/ACPI table 0x%"PRIx64
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 0f1e1b1..a8538e5 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1217,30 +1217,34 @@ int p2m_populate_ram(struct domain *d,
                              d->arch.p2m.default_access);
 }
 
-int map_regions_rw_cache(struct domain *d,
-                         unsigned long start_gfn,
-                         unsigned long nr,
-                         unsigned long mfn)
+int map_regions(struct domain *d,
+                unsigned long start_gfn,
+                unsigned long nr,
+                unsigned long mfn,
+                unsigned int mattr,
+                p2m_access_t access)
 {
     return apply_p2m_changes(d, INSERT,
                              pfn_to_paddr(start_gfn),
                              pfn_to_paddr(start_gfn + nr),
                              pfn_to_paddr(mfn),
-                             MATTR_MEM, 0, p2m_mmio_direct,
-                             p2m_access_rw);
+                             mattr, 0, p2m_mmio_direct,
+                             access);
 }
 
-int unmap_regions_rw_cache(struct domain *d,
-                           unsigned long start_gfn,
-                           unsigned long nr,
-                           unsigned long mfn)
+int unmap_regions(struct domain *d,
+                  unsigned long start_gfn,
+                  unsigned long nr,
+                  unsigned long mfn,
+                  unsigned int mattr,
+                  p2m_access_t access)
 {
     return apply_p2m_changes(d, REMOVE,
                              pfn_to_paddr(start_gfn),
                              pfn_to_paddr(start_gfn + nr),
                              pfn_to_paddr(mfn),
-                             MATTR_MEM, 0, p2m_invalid,
-                             p2m_access_rw);
+                             mattr, 0, p2m_invalid,
+                             access);
 }
 
 int map_mmio_regions(struct domain *d,
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d240d1e..92c425e 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -144,15 +144,19 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
 /* Setup p2m RAM mapping for domain d from start-end. */
 int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
 
-int map_regions_rw_cache(struct domain *d,
-                         unsigned long start_gfn,
-                         unsigned long nr_mfns,
-                         unsigned long mfn);
-
-int unmap_regions_rw_cache(struct domain *d,
-                           unsigned long start_gfn,
-                           unsigned long nr_mfns,
-                           unsigned long mfn);
+int map_regions(struct domain *d,
+                unsigned long start_gfn,
+                unsigned long nr,
+                unsigned long mfn,
+                unsigned int mattr,
+                p2m_access_t perms);
+
+int unmap_regions(struct domain *d,
+                  unsigned long start_gfn,
+                  unsigned long nr,
+                  unsigned long mfn,
+                  unsigned int mattr,
+                  p2m_access_t perms);
 
 int map_dev_mmio_region(struct domain *d,
                         unsigned long start_gfn,
-- 
2.5.0


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

Re: [RFC for-4.8 v2 2/7] xen/arm: Rename and generalize un/map_regions_rw_cache

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Rename and generalize un/map_regions_rw_cache into
> un/map_regions.

I would name it map_regions_mattr (or something similar) to stop people 
using this helper to map real RAM.

> The new functions take the mapping
> attributes and access permissions as arguments.

I overlooked it when I reviewed {,un}map_regions_rw_cache. p2m_access_t 
is for memaccess to restrict the permission. The helper should always 
use d->arch.p2m.default_access.

Can you fix it in a separate patch and request for backport to Xen 4.7?

Regards,

-- 
Julien Grall

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

Re: [RFC for-4.8 v2 2/7] xen/arm: Rename and generalize un/map_regions_rw_cache

[Edgar E. Iglesias]

On Mon, Jun 06, 2016 at 06:55:30PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 03/06/16 14:29, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> >Rename and generalize un/map_regions_rw_cache into
> >un/map_regions.
> 
> I would name it map_regions_mattr (or something similar) to stop people
> using this helper to map real RAM.

Sounds good, I've done it for v3.

> 
> >The new functions take the mapping
> >attributes and access permissions as arguments.
> 
> I overlooked it when I reviewed {,un}map_regions_rw_cache. p2m_access_t is
> for memaccess to restrict the permission. The helper should always use
> d->arch.p2m.default_access.
> 
> Can you fix it in a separate patch and request for backport to Xen 4.7?

Yes, I'll do that.

Cheers,
Edgar


> 
> Regards,
> 
> -- 
> Julien Grall

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

[RFC for-4.8 v2 3/7] xen/device-tree: Add __DT_MATCH macros without braces

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add __DT_MATCH macros without braces to allow the creation
of match descriptors with multiple combined match options.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/include/xen/device_tree.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index e3fe77c..b348913 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -33,10 +33,15 @@ struct dt_device_match {
     const void *data;
 };
 
-#define DT_MATCH_PATH(p)                { .path = p }
-#define DT_MATCH_TYPE(typ)              { .type = typ }
-#define DT_MATCH_COMPATIBLE(compat)     { .compatible = compat }
-#define DT_MATCH_NOT_AVAILABLE()        { .not_available = 1 }
+#define __DT_MATCH_PATH(p)              .path = p
+#define __DT_MATCH_TYPE(typ)            .type = typ
+#define __DT_MATCH_COMPATIBLE(compat)   .compatible = compat
+#define __DT_MATCH_NOT_AVAILABLE()      .not_available = 1
+
+#define DT_MATCH_PATH(p)                { __DT_MATCH_PATH(p) }
+#define DT_MATCH_TYPE(typ)              { __DT_MATCH_TYPE(typ) }
+#define DT_MATCH_COMPATIBLE(compat)     { __DT_MATCH_COMPATIBLE(compat) }
+#define DT_MATCH_NOT_AVAILABLE()        { __DT_MATCH_NOT_AVAILABLE() }
 
 typedef u32 dt_phandle;
 
-- 
2.5.0


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

Re: [RFC for-4.8 v2 3/7] xen/device-tree: Add __DT_MATCH macros without braces

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add __DT_MATCH macros without braces to allow the creation
> of match descriptors with multiple combined match options.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,

-- 
Julien Grall

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

[RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Make dt_match_node match for existing properties.
We only search for the existance of the properties, not
for specific values.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/common/device_tree.c      | 9 ++++++++-
 xen/include/xen/device_tree.h | 3 +++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 06a2837..fbee354 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -323,8 +323,9 @@ dt_match_node(const struct dt_device_match *matches,
         return NULL;
 
     while ( matches->path || matches->type ||
-            matches->compatible || matches->not_available )
+            matches->compatible || matches->not_available || matches->props)
     {
+        const char **props = matches->props;
         bool_t match = 1;
 
         if ( matches->path )
@@ -339,6 +340,12 @@ dt_match_node(const struct dt_device_match *matches,
         if ( matches->not_available )
             match &= !dt_device_is_available(node);
 
+        while ( match && props && *props )
+        {
+            match &= dt_find_property(node, *props, NULL) != NULL;
+            props++;
+        }
+
         if ( match )
             return matches;
         matches++;
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index b348913..f13d186 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -30,6 +30,7 @@ struct dt_device_match {
     const char *type;
     const char *compatible;
     const bool_t not_available;
+    const char **props;
     const void *data;
 };
 
@@ -37,11 +38,13 @@ struct dt_device_match {
 #define __DT_MATCH_TYPE(typ)            .type = typ
 #define __DT_MATCH_COMPATIBLE(compat)   .compatible = compat
 #define __DT_MATCH_NOT_AVAILABLE()      .not_available = 1
+#define __DT_MATCH_PROPS(p...)          .props = (const char *[]) { p, NULL }
 
 #define DT_MATCH_PATH(p)                { __DT_MATCH_PATH(p) }
 #define DT_MATCH_TYPE(typ)              { __DT_MATCH_TYPE(typ) }
 #define DT_MATCH_COMPATIBLE(compat)     { __DT_MATCH_COMPATIBLE(compat) }
 #define DT_MATCH_NOT_AVAILABLE()        { __DT_MATCH_NOT_AVAILABLE() }
+#define DT_MATCH_PROPS(p...)            { __DT_MATCH_PROPS(p) }
 
 typedef u32 dt_phandle;
 
-- 
2.5.0


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

Re: [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Make dt_match_node match for existing properties.
> We only search for the existance of the properties, not

s/existance/existence/

> for specific values.

[..]

> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index b348913..f13d186 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -30,6 +30,7 @@ struct dt_device_match {
>       const char *type;
>       const char *compatible;
>       const bool_t not_available;
> +    const char **props;

I would add a comment above the field to explain the behavior.

>       const void *data;
>   };
>
> @@ -37,11 +38,13 @@ struct dt_device_match {
>   #define __DT_MATCH_TYPE(typ)            .type = typ
>   #define __DT_MATCH_COMPATIBLE(compat)   .compatible = compat
>   #define __DT_MATCH_NOT_AVAILABLE()      .not_available = 1
> +#define __DT_MATCH_PROPS(p...)          .props = (const char *[]) { p, NULL }

Why the cast?

>
>   #define DT_MATCH_PATH(p)                { __DT_MATCH_PATH(p) }
>   #define DT_MATCH_TYPE(typ)              { __DT_MATCH_TYPE(typ) }
>   #define DT_MATCH_COMPATIBLE(compat)     { __DT_MATCH_COMPATIBLE(compat) }
>   #define DT_MATCH_NOT_AVAILABLE()        { __DT_MATCH_NOT_AVAILABLE() }
> +#define DT_MATCH_PROPS(p...)            { __DT_MATCH_PROPS(p) }
>
>   typedef u32 dt_phandle;

Regards,

-- 
Julien Grall

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

Re: [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Edgar E. Iglesias]

On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 03/06/16 14:29, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> >Make dt_match_node match for existing properties.
> >We only search for the existance of the properties, not
> 
> s/existance/existence/

Fixed

> 
> >for specific values.
> 
> [..]
> 
> >diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> >index b348913..f13d186 100644
> >--- a/xen/include/xen/device_tree.h
> >+++ b/xen/include/xen/device_tree.h
> >@@ -30,6 +30,7 @@ struct dt_device_match {
> >      const char *type;
> >      const char *compatible;
> >      const bool_t not_available;
> >+    const char **props;
> 
> I would add a comment above the field to explain the behavior.

Sounds good. I've added the following:
    /*
     * NULL terminated array of property names to search for.
     * We only search for the properties existence.
     */
    const char **props;


> 
> >      const void *data;
> >  };
> >
> >@@ -37,11 +38,13 @@ struct dt_device_match {
> >  #define __DT_MATCH_TYPE(typ)            .type = typ
> >  #define __DT_MATCH_COMPATIBLE(compat)   .compatible = compat
> >  #define __DT_MATCH_NOT_AVAILABLE()      .not_available = 1
> >+#define __DT_MATCH_PROPS(p...)          .props = (const char *[]) { p, NULL }
> 
> Why the cast?

AFAIK, it's needed to instantiate the dynamically sized array of pointers.
Another option is to make __DT_MATCH_PROPS take the char ** pointer.
The descriptor declaration would instead of looking like this:
    {
        __DT_MATCH_COMPATIBLE("mmio-sram"),
        __DT_MATCH_PROPS("no-memory-wc"),
        .data = &mattr_device_rw,
    },

Look something like this:

const char *props_no_mem_wc[] = { "no-memory-wc", NULL };

....

    {
        __DT_MATCH_COMPATIBLE("mmio-sram"),
        __DT_MATCH_PROPS(props_no_mem_wc),
        .data = &mattr_device_rw,
    },


Or do you have better suggestions?

Best regards,
Edgar


> 
> >
> >  #define DT_MATCH_PATH(p)                { __DT_MATCH_PATH(p) }
> >  #define DT_MATCH_TYPE(typ)              { __DT_MATCH_TYPE(typ) }
> >  #define DT_MATCH_COMPATIBLE(compat)     { __DT_MATCH_COMPATIBLE(compat) }
> >  #define DT_MATCH_NOT_AVAILABLE()        { __DT_MATCH_NOT_AVAILABLE() }
> >+#define DT_MATCH_PROPS(p...)            { __DT_MATCH_PROPS(p) }
> >
> >  typedef u32 dt_phandle;
> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Julien Grall]

Hi Edgar,

On 07/06/2016 21:43, Edgar E. Iglesias wrote:
> On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote:
>> On 03/06/16 14:29, Edgar E. Iglesias wrote:

[...]

> AFAIK, it's needed to instantiate the dynamically sized array of pointers.
> Another option is to make __DT_MATCH_PROPS take the char ** pointer.
> The descriptor declaration would instead of looking like this:
>     {
>         __DT_MATCH_COMPATIBLE("mmio-sram"),
>         __DT_MATCH_PROPS("no-memory-wc"),
>         .data = &mattr_device_rw,
>     },
>
> Look something like this:
>
> const char *props_no_mem_wc[] = { "no-memory-wc", NULL };
>
> ....
>
>     {
>         __DT_MATCH_COMPATIBLE("mmio-sram"),
>         __DT_MATCH_PROPS(props_no_mem_wc),
>         .data = &mattr_device_rw,
>     },
>
>
> Or do you have better suggestions?

How about defining props with the type "const char *props[]"?

Regards,

-- 
Julien Grall

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

Re: [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Edgar E. Iglesias]

On Wed, Jun 08, 2016 at 09:44:51AM +0100, Julien Grall wrote:
> Hi Edgar,

Hi Julien,


> 
> On 07/06/2016 21:43, Edgar E. Iglesias wrote:
> >On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote:
> >>On 03/06/16 14:29, Edgar E. Iglesias wrote:
> 
> [...]
> 
> >AFAIK, it's needed to instantiate the dynamically sized array of pointers.
> >Another option is to make __DT_MATCH_PROPS take the char ** pointer.
> >The descriptor declaration would instead of looking like this:
> >    {
> >        __DT_MATCH_COMPATIBLE("mmio-sram"),
> >        __DT_MATCH_PROPS("no-memory-wc"),
> >        .data = &mattr_device_rw,
> >    },
> >
> >Look something like this:
> >
> >const char *props_no_mem_wc[] = { "no-memory-wc", NULL };
> >
> >....
> >
> >    {
> >        __DT_MATCH_COMPATIBLE("mmio-sram"),
> >        __DT_MATCH_PROPS(props_no_mem_wc),
> >        .data = &mattr_device_rw,
> >    },
> >
> >
> >Or do you have better suggestions?
> 
> How about defining props with the type "const char *props[]"?
> 

That doesn't work for arrays of match descriptors (i.e you can't have arrays of variable sized objects)...

Cheers,
Edgar

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

Re: [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Julien Grall]

Hi Edgar,

On 09/06/16 17:04, Edgar E. Iglesias wrote:
> On Wed, Jun 08, 2016 at 09:44:51AM +0100, Julien Grall wrote:
>>
>> On 07/06/2016 21:43, Edgar E. Iglesias wrote:
>>> On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote:
>>>> On 03/06/16 14:29, Edgar E. Iglesias wrote:
>>
>> [...]
>>
>>> AFAIK, it's needed to instantiate the dynamically sized array of pointers.
>>> Another option is to make __DT_MATCH_PROPS take the char ** pointer.
>>> The descriptor declaration would instead of looking like this:
>>>     {
>>>         __DT_MATCH_COMPATIBLE("mmio-sram"),
>>>         __DT_MATCH_PROPS("no-memory-wc"),
>>>         .data = &mattr_device_rw,
>>>     },
>>>
>>> Look something like this:
>>>
>>> const char *props_no_mem_wc[] = { "no-memory-wc", NULL };
>>>
>>> ....
>>>
>>>     {
>>>         __DT_MATCH_COMPATIBLE("mmio-sram"),
>>>         __DT_MATCH_PROPS(props_no_mem_wc),
>>>         .data = &mattr_device_rw,
>>>     },
>>>
>>>
>>> Or do you have better suggestions?
>>
>> How about defining props with the type "const char *props[]"?
>>
>
> That doesn't work for arrays of match descriptors (i.e you can't have arrays of variable sized objects)...

Hmmmm... I would rather try to avoid the cast, but the other solution 
you suggested does not look appealing (i.e declare separately the 
properties).

However do you have a use case where checking multiple properties would 
be useful? If not, I would just handle one property for now.

Regards,

-- 
Julien Grall

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

Re: [RFC for-4.8 v2 4/7] xen/device-tree: Make dt_match_node match props

[Edgar E. Iglesias]

On Thu, Jun 09, 2016 at 05:23:33PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 09/06/16 17:04, Edgar E. Iglesias wrote:
> >On Wed, Jun 08, 2016 at 09:44:51AM +0100, Julien Grall wrote:
> >>
> >>On 07/06/2016 21:43, Edgar E. Iglesias wrote:
> >>>On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote:
> >>>>On 03/06/16 14:29, Edgar E. Iglesias wrote:
> >>
> >>[...]
> >>
> >>>AFAIK, it's needed to instantiate the dynamically sized array of pointers.
> >>>Another option is to make __DT_MATCH_PROPS take the char ** pointer.
> >>>The descriptor declaration would instead of looking like this:
> >>>    {
> >>>        __DT_MATCH_COMPATIBLE("mmio-sram"),
> >>>        __DT_MATCH_PROPS("no-memory-wc"),
> >>>        .data = &mattr_device_rw,
> >>>    },
> >>>
> >>>Look something like this:
> >>>
> >>>const char *props_no_mem_wc[] = { "no-memory-wc", NULL };
> >>>
> >>>....
> >>>
> >>>    {
> >>>        __DT_MATCH_COMPATIBLE("mmio-sram"),
> >>>        __DT_MATCH_PROPS(props_no_mem_wc),
> >>>        .data = &mattr_device_rw,
> >>>    },
> >>>
> >>>
> >>>Or do you have better suggestions?
> >>
> >>How about defining props with the type "const char *props[]"?
> >>
> >
> >That doesn't work for arrays of match descriptors (i.e you can't have arrays of variable sized objects)...
> 
> Hmmmm... I would rather try to avoid the cast, but the other solution you
> suggested does not look appealing (i.e declare separately the properties).
> 
> However do you have a use case where checking multiple properties would be
> useful? If not, I would just handle one property for now.

No I don't. We can do one property for now. I'll change that for v3.

Thanks,
Edgar

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

[RFC for-4.8 v2 5/7] xen/arm: domain_build: Plumb for different mapping attributes

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add plumbing for passing around mapping attributes. This
is in preparation to allow us to differentiate the attributes
for specific device nodes.

We still use the same DEVICE mappings for all nodes so this
patch has no functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/arch/arm/domain_build.c | 57 +++++++++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e4fed4b..064feb3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -42,6 +42,18 @@ static void __init parse_dom0_mem(const char *s)
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
+struct map_attr
+{
+    unsigned int memattr;
+    p2m_access_t access;
+};
+
+struct map_range_data
+{
+    struct domain *d;
+    const struct map_attr *attr;
+};
+
 //#define DEBUG_DT
 
 #ifdef DEBUG_DT
@@ -1024,7 +1036,8 @@ static int map_range_to_domain(const struct dt_device_node *dev,
                                u64 addr, u64 len,
                                void *data)
 {
-    struct domain *d = data;
+    struct map_range_data *mr_data = data;
+    struct domain *d = mr_data->d;
     bool_t need_mapping = !dt_device_for_passthrough(dev);
     int res;
 
@@ -1041,10 +1054,13 @@ static int map_range_to_domain(const struct dt_device_node *dev,
 
     if ( need_mapping )
     {
-        res = map_mmio_regions(d,
-                               paddr_to_pfn(addr),
-                               DIV_ROUND_UP(len, PAGE_SIZE),
-                               paddr_to_pfn(addr));
+        res = map_regions(d,
+                          paddr_to_pfn(addr),
+                          DIV_ROUND_UP(len, PAGE_SIZE),
+                          paddr_to_pfn(addr),
+                          mr_data->attr->memattr,
+                          mr_data->attr->access);
+
         if ( res < 0 )
         {
             printk(XENLOG_ERR "Unable to map 0x%"PRIx64
@@ -1055,7 +1071,8 @@ static int map_range_to_domain(const struct dt_device_node *dev,
         }
     }
 
-    DPRINT("  - MMIO: %010"PRIx64" - %010"PRIx64"\n", addr, addr + len);
+    DPRINT("  - MMIO: %010"PRIx64" - %010"PRIx64" MemAttr=%x\n",
+           addr, addr + len, mr_data->attr->memattr);
 
     return 0;
 }
@@ -1066,8 +1083,10 @@ static int map_range_to_domain(const struct dt_device_node *dev,
  * the child resources available to domain 0.
  */
 static int map_device_children(struct domain *d,
-                               const struct dt_device_node *dev)
+                               const struct dt_device_node *dev,
+                               const struct map_attr *attr)
 {
+    struct map_range_data mr_data = { .d = d, .attr = attr };
     int ret;
 
     if ( dt_device_type_is_equal(dev, "pci") )
@@ -1078,7 +1097,7 @@ static int map_device_children(struct domain *d,
         if ( ret < 0 )
             return ret;
 
-        ret = dt_for_each_range(dev, &map_range_to_domain, d);
+        ret = dt_for_each_range(dev, &map_range_to_domain, &mr_data);
         if ( ret < 0 )
             return ret;
     }
@@ -1094,7 +1113,8 @@ static int map_device_children(struct domain *d,
  *  - Assign the device to the guest if it's protected by an IOMMU
  *  - Map the IRQs and iomem regions to DOM0
  */
-static int handle_device(struct domain *d, struct dt_device_node *dev)
+static int handle_device(struct domain *d, struct dt_device_node *dev,
+                         const struct map_attr *attr)
 {
     unsigned int nirq;
     unsigned int naddr;
@@ -1160,6 +1180,7 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
     /* Give permission and map MMIOs */
     for ( i = 0; i < naddr; i++ )
     {
+        struct map_range_data mr_data = { .d = d, attr = attr };
         res = dt_device_get_address(dev, i, &addr, &size);
         if ( res )
         {
@@ -1168,12 +1189,12 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
             return res;
         }
 
-        res = map_range_to_domain(dev, addr, size, d);
+        res = map_range_to_domain(dev, addr, size, &mr_data);
         if ( res )
             return res;
     }
 
-    res = map_device_children(d, dev);
+    res = map_device_children(d, dev, attr);
     if ( res )
         return res;
 
@@ -1181,7 +1202,8 @@ static int handle_device(struct domain *d, struct dt_device_node *dev)
 }
 
 static int handle_node(struct domain *d, struct kernel_info *kinfo,
-                       struct dt_device_node *node)
+                       struct dt_device_node *node,
+                       const struct map_attr *attr)
 {
     static const struct dt_device_match skip_matches[] __initconst =
     {
@@ -1268,7 +1290,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
                "WARNING: Path %s is reserved, skip the node as we may re-use the path.\n",
                path);
 
-    res = handle_device(d, node);
+    res = handle_device(d, node, attr);
     if ( res)
         return res;
 
@@ -1290,7 +1312,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
 
     for ( child = node->child; child != NULL; child = child->sibling )
     {
-        res = handle_node(d, kinfo, child);
+        res = handle_node(d, kinfo, child, attr);
         if ( res )
             return res;
     }
@@ -1322,6 +1344,11 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
 
 static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 {
+    const struct map_attr default_attr =
+    {
+        .memattr = MATTR_DEV,
+        .access = d->arch.p2m.default_access,
+    };
     const void *fdt;
     int new_size;
     int ret;
@@ -1341,7 +1368,7 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 
     fdt_finish_reservemap(kinfo->fdt);
 
-    ret = handle_node(d, kinfo, dt_host);
+    ret = handle_node(d, kinfo, dt_host, &default_attr);
     if ( ret )
         goto err;
 
-- 
2.5.0


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

Re: [RFC for-4.8 v2 5/7] xen/arm: domain_build: Plumb for different mapping attributes

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add plumbing for passing around mapping attributes. This
> is in preparation to allow us to differentiate the attributes
> for specific device nodes.
>
> We still use the same DEVICE mappings for all nodes so this
> patch has no functional change.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>   xen/arch/arm/domain_build.c | 57 +++++++++++++++++++++++++++++++++------------
>   1 file changed, 42 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e4fed4b..064feb3 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -42,6 +42,18 @@ static void __init parse_dom0_mem(const char *s)
>   }
>   custom_param("dom0_mem", parse_dom0_mem);
>
> +struct map_attr
> +{
> +    unsigned int memattr;
> +    p2m_access_t access;

Based on my comment on patch #2, access should not be used for other 
purpose than memaccess. So ...

> +};
> +
> +struct map_range_data
> +{
> +    struct domain *d;
> +    const struct map_attr *attr;

.. this could be replaced by "unsigned int memattr" directly.

> +};
> +
>   //#define DEBUG_DT
>
>   #ifdef DEBUG_DT

[...]

> @@ -1322,6 +1344,11 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>
>   static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>   {
> +    const struct map_attr default_attr =
> +    {
> +        .memattr = MATTR_DEV,
> +        .access = d->arch.p2m.default_access,
> +    };

Please add a comment to explain that by default, region are always 
mapped the most restrictive way with device attribute.

>       const void *fdt;
>       int new_size;
>       int ret;
> @@ -1341,7 +1368,7 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>
>       fdt_finish_reservemap(kinfo->fdt);
>
> -    ret = handle_node(d, kinfo, dt_host);
> +    ret = handle_node(d, kinfo, dt_host, &default_attr);
>       if ( ret )
>           goto err;
>
>

Regards,

-- 
Julien Grall

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

[RFC for-4.8 v2 6/7] xen/device-tree: Add an mmio-sram bus

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Add an mmio-sram bus that prevents sram sub areas from
being re-mapped. These sub-areas describe allocations and
not mappings.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/common/device_tree.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index fbee354..698d4da 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -616,10 +616,47 @@ static int dt_bus_pci_translate(__be32 *addr, u64 offset, int na)
 }
 
 /*
+ * MMIO-SRAM bus specific translator
+ */
+
+static bool_t dt_bus_mmio_sram_match(const struct dt_device_node *np)
+{
+    static const struct dt_device_match match[] __initconst =
+    {
+        DT_MATCH_COMPATIBLE("mmio-sram"),
+        { /* sentinel */ },
+    };
+
+    return dt_match_node(match, np) != NULL;
+}
+
+static void dt_bus_mmio_sram_count_cells(const struct dt_device_node *np,
+					 int *addrc, int *sizec)
+{
+    /*
+     * Child areas describe allocations, not mappings. Don't map anything.
+     */
+    if (addrc)
+        *addrc = 0;
+    if (sizec)
+        *sizec = 0;
+}
+
+/*
  * Array of bus specific translators
  */
 static const struct dt_bus dt_busses[] =
 {
+    /* MMIO-SRAM */
+    {
+        .name = "mmio-sram",
+        .addresses = "reg",
+        .match = dt_bus_mmio_sram_match,
+        .count_cells = dt_bus_mmio_sram_count_cells,
+        .map = dt_bus_default_map,
+        .translate = dt_bus_default_translate,
+        .get_flags = dt_bus_default_get_flags,
+    },
     /* PCI */
     {
         .name = "pci",
-- 
2.5.0


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

Re: [RFC for-4.8 v2 6/7] xen/device-tree: Add an mmio-sram bus

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Add an mmio-sram bus that prevents sram sub areas from
> being re-mapped. These sub-areas describe allocations and
> not mappings.

mmio-sram is not a bus and the region below should point to a valid 
physical address.

So why do you want that?

Regards,

-- 
Julien Grall

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

Re: [RFC for-4.8 v2 6/7] xen/device-tree: Add an mmio-sram bus

[Edgar E. Iglesias]

On Mon, Jun 06, 2016 at 07:05:20PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 03/06/16 14:29, Edgar E. Iglesias wrote:
> >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> >Add an mmio-sram bus that prevents sram sub areas from
> >being re-mapped. These sub-areas describe allocations and
> >not mappings.
> 
> mmio-sram is not a bus and the region below should point to a valid physical
> address.
> 
> So why do you want that?

Hi,

Yes, I briefly mentioned this in the cover letter. It's a hack
to avoid the mapping of sub allocations. The regions under mmio-sram
point to valid addresses but they point into the same space that
the outer mmio-sram node maps in. They also don't need to be page
aligned. AFAICT, Xen should ignore these sub-regions allthough I
think things work anyway (at least with my dts).

Maybe I should just drop this until we have a real problem or
come up with a nicer way of dealing with this.

Cheers,
Edgar





> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [RFC for-4.8 v2 6/7] xen/device-tree: Add an mmio-sram bus

[Julien Grall]

Hi Edgar,

On 08/06/2016 01:21, Edgar E. Iglesias wrote:
> On Mon, Jun 06, 2016 at 07:05:20PM +0100, Julien Grall wrote:
>> Hi Edgar,
>>
>> On 03/06/16 14:29, Edgar E. Iglesias wrote:
>>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>>
>>> Add an mmio-sram bus that prevents sram sub areas from
>>> being re-mapped. These sub-areas describe allocations and
>>> not mappings.
>>
>> mmio-sram is not a bus and the region below should point to a valid physical
>> address.
>>
>> So why do you want that?
>
> Hi,
>
> Yes, I briefly mentioned this in the cover letter. It's a hack
> to avoid the mapping of sub allocations. The regions under mmio-sram
> point to valid addresses but they point into the same space that
> the outer mmio-sram node maps in. They also don't need to be page
> aligned. AFAICT, Xen should ignore these sub-regions allthough I
> think things work anyway (at least with my dts).

Sorry I haven't fully read the cover letter. I agree that going through 
those sub-regions are pointless because it already mapped by the parent.

However, it is harmless as they should be part of the parent MMIO. It 
also makes the code simpler.

>
> Maybe I should just drop this until we have a real problem or
> come up with a nicer way of dealing with this.

Sounds good to me.

Regards,

-- 
Julien Grall

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

[RFC for-4.8 v2 7/7] xen/arm: Map mmio-sram nodes as normal un-cached rwx memory

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Map mmio-sram nodes as normal un-cached MEMORY with RWX perms.
If the node has set the no-memory-wc property, we map it as
DEVICE RW.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 xen/arch/arm/domain_build.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 064feb3..2a2316b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -54,6 +54,32 @@ struct map_range_data
     const struct map_attr *attr;
 };
 
+static const struct map_attr mattr_device_rw __initconst =
+{
+    .memattr = MATTR_DEV,
+    .access = p2m_access_rw,
+};
+
+static const struct map_attr mattr_memory_nc_rwx __initconst =
+{
+    .memattr = MATTR_MEM_NC,
+    .access = p2m_access_rwx,
+};
+
+static const struct dt_device_match dev_map_attrs[] __initconst =
+{
+    {
+        __DT_MATCH_COMPATIBLE("mmio-sram"),
+        __DT_MATCH_PROPS("no-memory-wc"),
+        .data = &mattr_device_rw,
+    },
+    {
+        __DT_MATCH_COMPATIBLE("mmio-sram"),
+        .data = &mattr_memory_nc_rwx,
+    },
+    { /* sentinel */ },
+};
+
 //#define DEBUG_DT
 
 #ifdef DEBUG_DT
@@ -1201,6 +1227,16 @@ static int handle_device(struct domain *d, struct dt_device_node *dev,
     return 0;
 }
 
+static const struct map_attr *lookup_map_attr(struct dt_device_node *node,
+                                            const struct map_attr *parent_attr)
+{
+    const struct dt_device_match *r;
+
+    /* Search and if nothing matches, use the parent's attributes.  */
+    r = dt_match_node(dev_map_attrs, node);
+    return r ? r->data : parent_attr;
+}
+
 static int handle_node(struct domain *d, struct kernel_info *kinfo,
                        struct dt_device_node *node,
                        const struct map_attr *attr)
@@ -1290,6 +1326,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
                "WARNING: Path %s is reserved, skip the node as we may re-use the path.\n",
                path);
 
+    attr = lookup_map_attr(node, attr);
     res = handle_device(d, node, attr);
     if ( res)
         return res;
-- 
2.5.0


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

Re: [RFC for-4.8 v2 7/7] xen/arm: Map mmio-sram nodes as normal un-cached rwx memory

[Julien Grall]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Map mmio-sram nodes as normal un-cached MEMORY with RWX perms.
> If the node has set the no-memory-wc property, we map it as
> DEVICE RW.

Please add a path/link to the bindings in the commit message.

>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>   xen/arch/arm/domain_build.c | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 064feb3..2a2316b 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -54,6 +54,32 @@ struct map_range_data
>       const struct map_attr *attr;
>   };
>
> +static const struct map_attr mattr_device_rw __initconst =
> +{
> +    .memattr = MATTR_DEV,
> +    .access = p2m_access_rw,
> +};
> +
> +static const struct map_attr mattr_memory_nc_rwx __initconst =
> +{
> +    .memattr = MATTR_MEM_NC,
> +    .access = p2m_access_rwx,
> +};
> +
> +static const struct dt_device_match dev_map_attrs[] __initconst =
> +{
> +    {
> +        __DT_MATCH_COMPATIBLE("mmio-sram"),
> +        __DT_MATCH_PROPS("no-memory-wc"),
> +        .data = &mattr_device_rw,
> +    },
> +    {
> +        __DT_MATCH_COMPATIBLE("mmio-sram"),
> +        .data = &mattr_memory_nc_rwx,
> +    },
> +    { /* sentinel */ },
> +};
> +
>   //#define DEBUG_DT
>
>   #ifdef DEBUG_DT
> @@ -1201,6 +1227,16 @@ static int handle_device(struct domain *d, struct dt_device_node *dev,
>       return 0;
>   }
>
> +static const struct map_attr *lookup_map_attr(struct dt_device_node *node,
> +                                            const struct map_attr *parent_attr)

The indentation looks wrong here.

> +{
> +    const struct dt_device_match *r;
> +
> +    /* Search and if nothing matches, use the parent's attributes.  */
> +    r = dt_match_node(dev_map_attrs, node);

Newline here please.

> +    return r ? r->data : parent_attr;

I would explain this behavior in the commit message and give some 
rationale why it is fine to do that.

> +}
> +
>   static int handle_node(struct domain *d, struct kernel_info *kinfo,
>                          struct dt_device_node *node,
>                          const struct map_attr *attr)
> @@ -1290,6 +1326,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>                  "WARNING: Path %s is reserved, skip the node as we may re-use the path.\n",
>                  path);
>
> +    attr = lookup_map_attr(node, attr);
>       res = handle_device(d, node, attr);
>       if ( res)
>           return res;
>

Regards,

-- 
Julien Grall

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