Re: [Xen-devel] [PATCH v1 1/1] xen/arm: Relax hw domain mapping attributes to p2m_mmio_direct_c

[Julien Grall <julien.grall@xxxxxxx>]

Hi Edgar,

On 10/01/2017 11:37, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
>
> Relax the hardware domains mapping attributes to p2m_mmio_direct_c.
> This will allow the hardware domain to fully control the
> attribtues via its S1 mappings.
s/attribtues/attributes/

I would like some rationale in the commit message to explain why it is 
fine to do this relaxation (e.g the hardware domain is a trusted domain).
A such relaxation would probably be necessary for the ACPI case too (see 
map_dev_mmio_region).
Also, this is a revert of patch 1e75ed8 and 9eed361 + relaxation, I 
would either mention it in the commit message. Or send separate patch to 
revert both of them. Either way will be fine by me.
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 63 ++++++++++-----------------------------------
>  1 file changed, 13 insertions(+), 50 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 07b868d..a3521c7 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -48,20 +48,6 @@ struct map_range_data
>      p2m_type_t p2mt;
>  };
>
> -static const struct dt_device_match dev_map_attrs[] __initconst =
> -{
> -    {
> -        __DT_MATCH_COMPATIBLE("mmio-sram"),
> -        __DT_MATCH_PROP("no-memory-wc"),
> -        .data = (void *) (uintptr_t) p2m_mmio_direct_dev,
> -    },
> -    {
> -        __DT_MATCH_COMPATIBLE("mmio-sram"),
> -        .data = (void *) (uintptr_t) p2m_mmio_direct_nc,
> -    },
> -    { /* sentinel */ },
> -};
> -
>  //#define DEBUG_11_ALLOCATION
>  #ifdef DEBUG_11_ALLOCATION
>  # define D11PRINT(fmt, args...) printk(XENLOG_DEBUG fmt, ##args)
> @@ -1005,8 +991,7 @@ static int map_range_to_domain(const struct dt_device_node 
> *dev,
>                                 u64 addr, u64 len,
>                                 void *data)
>  {
> -    struct map_range_data *mr_data = data;
I would actually prefer to keep the plumbing and only remove 
dev_map_attrs. Stefano do you have any opinions?
If we are going to remove the plumbing, you would need to remove 
map_range_data which has been introduced by the plumbing patch.
Cheers,

--
Julien Grall

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