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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Edgar,

On 03/06/16 14:29, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
>   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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel