Re: [Xen-devel] [PATCH RFC 25/35] arm: acpi add helper functions to map memory regions

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 6 Feb 2015, Julien Grall wrote:
> On 06/02/2015 00:21, Stefano Stabellini wrote:
> > On Thu, 5 Feb 2015, Julien Grall wrote:
> > > Hi parth,
> > > 
> > > Title: this is not acpi specific.
> > > 
> > > On 04/02/2015 14:02, parth.dixit@xxxxxxxxxx wrote:
> > > > From: Parth Dixit <parth.dixit@xxxxxxxxxx>
> > > > 
> > > > For passing ACPI tables to dom0, UEFI memory needs to be mapped
> > > > by xen in dom0 address space. This patch adds helper functions for
> > > > mapping.
> > > 
> > > I believe that this is not ACPI/RAM specific. Any cached MMIO regions will
> > > have same issue.
> > > 
> > > This because Device memory is too strong and disallow unaligned access.
> > > 
> > > > Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> > > > ---
> > > >    xen/arch/arm/p2m.c        | 24 ++++++++++++++++++++++++
> > > >    xen/include/asm-arm/p2m.h | 10 ++++++++++
> > > >    2 files changed, 34 insertions(+)
> > > > 
> > > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > > > index 8809f5a..5593a91 100644
> > > > --- a/xen/arch/arm/p2m.c
> > > > +++ b/xen/arch/arm/p2m.c
> > > > @@ -943,6 +943,30 @@ int p2m_populate_ram(struct domain *d,
> > > >                                 0, MATTR_MEM, p2m_ram_rw);
> > > >    }
> > > > 
> > > > +int map_ram_regions(struct domain *d,
> > > > +                     unsigned long start_gfn,
> > > > +                     unsigned long nr,
> > > > +                     unsigned long mfn)
> > > 
> > > I don't like the name of the function. It gives the impression that we map
> > > any
> > > RAM region to the guest via this function.
> > > 
> > > Which is obviously wrong and should never be done.
> > 
> > map_mem_regions?
> 
> It's still unclear, what mem mean? is an MMIO region? Is it cached, uncached?
> Is it read-only/read-write.
> 
> We already have a function map_mmio_regions. Maybe it would make sense to
> extend it to give more control about the mapping (cached/uncached, read-only,
> read-write,....). But this function is common with x86. So I'm not sure about
> what to do.

Usually mmio refers to device memory and not just from the caching
attributes point of view. I would rather not use map_mmio_regions for
something that doesn't belong to a device.

That said, it is difficult to come up with a good name.

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