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

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Thu, Jan 19, 2017 at 10:46:03AM -0800, Stefano Stabellini wrote:
> On Thu, 19 Jan 2017, Julien Grall wrote:
> > Hi Edgar,
> > 
> > On 10/01/2017 11:37, Edgar E. Iglesias wrote:
> > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> > > 
> > > 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@xilinx.com>
> > > ---
> > >  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?
> 
> I would keep the plumbings too (9eed361a), but I am fine either way.


Thanks, I've changed the patch to keep the plumbing from 9eed361a.

Cheers,
Edgar

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