Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU systems

[Julien Grall <julien@xxxxxxx>]

On 09/11/2022 06:46, Wei Chen wrote:
> Hi Julien,
Hi Wei,

> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Sent: 2022年11月7日 3:45
> > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> > Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> > <Volodymyr_Babchuk@xxxxxxxx>
> > Subject: Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU
> > systems
> >
> > Hi Wei,
> >
> > On 04/11/2022 10:07, Wei Chen wrote:
> > > FIXMAP is a special virtual address section for Xen to map some
> > > physical ram or device memory temporarily in initialization for
> > > MMU systems. FIXMAP_ADDR will return a virtual address by index
> > > for special purpose phys-to-virt mapping usage. For example,
> > > FIXMAP_ADDR(FIXMAP_CONSOLE) for early console mapping and
> > > FIXMAP_ADDR(FIXMAP_MISC) for copy_from_paddr.
> > To me, we are bending quite a bit the definition of the fixmap. There
> > are not many use of the FIXMAP within the code and I think it would
> > simply be better to abstract the use (or removing it when possible) and
> > avoid defining FIXMAP_ADDR() & co for MPU.
> I agree, if we don't mind to add some CONFIG_HAS_MPU in some generic code.
FAOD, this is not what I had in mind. Instead, it was to provide helper 
which for !HAS_MPU would call fixmap and for HAS_MPU would do the work 
to map the region in the MPU.
[...]

> > >    xen/arch/arm/Kconfig                  |  2 +-
> > >    xen/arch/arm/include/asm/config_mpu.h |  2 ++
> > >    xen/arch/arm/include/asm/fixmap.h     | 25 +++++++++++++++++++++++++
> > >    3 files changed, 28 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > index ac276307d6..1458ffa777 100644
> > > --- a/xen/arch/arm/Kconfig
> > > +++ b/xen/arch/arm/Kconfig
> > > @@ -16,7 +16,7 @@ config ARM
> > >         select HAS_DEVICE_TREE
> > >         select HAS_PASSTHROUGH
> > >         select HAS_PDX
> > > -       select HAS_PMAP
> > > +       select HAS_PMAP if !HAS_MPU
> > I can't find any change of mm.c in this series. So surely this will
> > break the build?
> Yes, in our internal testing, open PMAP for MPU will cause building
> failed, except we add some new stubs for MPU system.
Do you mean you added some stubs for PMAP? If so, I would not expect any 
caller for the pmap() to be used for the MPU. Therefore, why would they 
be necessary?
Cheers,

--
Julien Grall