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

[Julien Grall <julien.grall@arm.com>]

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