Message-ID: <20200812073840.GA975@Air-de-Roger>
Date:   Wed, 12 Aug 2020 09:38:40 +0200
From:   Roger Pau Monné <roger.pau@...rix.com>
To:     Jürgen Groß <jgross@...e.com>
CC:     <linux-kernel@...r.kernel.org>,
        Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>,
        David Airlie <airlied@...ux.ie>,
        "Daniel Vetter" <daniel@...ll.ch>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>, Wei Liu <wl@....org>,
        Yan Yankovskyi <yyankovskyi@...il.com>,
        <dri-devel@...ts.freedesktop.org>,
        <xen-devel@...ts.xenproject.org>, <linux-mm@...ck.org>,
        David Hildenbrand <david@...hat.com>,
        Michal Hocko <mhocko@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory

On Wed, Aug 12, 2020 at 09:28:45AM +0200, Jürgen Groß wrote:
> On 11.08.20 11:44, Roger Pau Monne wrote:
> > To be used in order to create foreign mappings. This is based on the
> > ZONE_DEVICE facility which is used by persistent memory devices in
> > order to create struct pages and kernel virtual mappings for the IOMEM
> > areas of such devices. Note that on kernels without support for
> > ZONE_DEVICE Xen will fallback to use ballooned pages in order to
> > create foreign mappings.
> > 
> > The newly added helpers use the same parameters as the existing
> > {alloc/free}_xenballooned_pages functions, which allows for in-place
> > replacement of the callers. Once a memory region has been added to be
> > used as scratch mapping space it will no longer be released, and pages
> > returned are kept in a linked list. This allows to have a buffer of
> > pages and prevents resorting to frequent additions and removals of
> > regions.
> > 
> > If enabled (because ZONE_DEVICE is supported) the usage of the new
> > functionality untangles Xen balloon and RAM hotplug from the usage of
> > unpopulated physical memory ranges to map foreign pages, which is the
> > correct thing to do in order to avoid mappings of foreign pages depend
> > on memory hotplug.
> > 
> > Note the driver is currently not enabled on Arm platforms because it
> > would interfere with the identity mapping required on some platforms.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@...rix.com>
> > ---
> > Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
> > Cc: David Airlie <airlied@...ux.ie>
> > Cc: Daniel Vetter <daniel@...ll.ch>
> > Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> > Cc: Juergen Gross <jgross@...e.com>
> > Cc: Stefano Stabellini <sstabellini@...nel.org>
> > Cc: Dan Carpenter <dan.carpenter@...cle.com>
> > Cc: Roger Pau Monne <roger.pau@...rix.com>
> > Cc: Wei Liu <wl@....org>
> > Cc: Yan Yankovskyi <yyankovskyi@...il.com>
> > Cc: dri-devel@...ts.freedesktop.org
> > Cc: xen-devel@...ts.xenproject.org
> > Cc: linux-mm@...ck.org
> > Cc: David Hildenbrand <david@...hat.com>
> > Cc: Michal Hocko <mhocko@...nel.org>
> > Cc: Dan Williams <dan.j.williams@...el.com>
> > ---
> > Changes since v3:
> >   - Introduce a Kconfig option that gates the addition of the
> >     unpopulated alloc driver. This allows to easily disable it on Arm
> >     platforms.
> >   - Dropped Juergen RB due to the addition of the Kconfig option.
> >   - Switched from MEMORY_DEVICE_DEVDAX to MEMORY_DEVICE_GENERIC.
> > 
> > Changes since v2:
> >   - Drop BUILD_BUG_ON regarding PVMMU page sizes.
> >   - Use a SPDX license identifier.
> >   - Call fill with only the minimum required number of pages.
> >   - Include xen.h header in xen_drm_front_gem.c.
> >   - Use less generic function names.
> >   - Exit early from the init function if not a PV guest.
> >   - Don't use all caps for region name.
> > ---
> >   drivers/gpu/drm/xen/xen_drm_front_gem.c |   9 +-
> >   drivers/xen/Kconfig                     |   4 +
> >   drivers/xen/Makefile                    |   1 +
> >   drivers/xen/balloon.c                   |   4 +-
> >   drivers/xen/grant-table.c               |   4 +-
> >   drivers/xen/privcmd.c                   |   4 +-
> >   drivers/xen/unpopulated-alloc.c         | 185 ++++++++++++++++++++++++
> >   drivers/xen/xenbus/xenbus_client.c      |   6 +-
> >   drivers/xen/xlate_mmu.c                 |   4 +-
> >   include/xen/xen.h                       |   9 ++
> >   10 files changed, 215 insertions(+), 15 deletions(-)
> >   create mode 100644 drivers/xen/unpopulated-alloc.c
> > 
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index 1d339ef92422..018020b91baa 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -327,4 +327,8 @@ config XEN_HAVE_VPMU
> >   config XEN_FRONT_PGDIR_SHBUF
> >   	tristate
> > +config XEN_UNPOPULATED_ALLOC
> > +	bool
> > +	default y if ZONE_DEVICE && !ARM && !ARM64
> 
> There is a current effort to enable Xen on RISC-V. Do we expect this
> option to be usable for this architecture?

It will depend on whether the Xen RISC-V implementation mandates an
IOMMU, for example Arm doesn't and hence uses an identity p2m for
dom0. If RISC-V does the same then I would assume this won't be
suitable as-is (not that it couldn't be made to work).

IMO it wasn't clear from the community call what was the RISC-V port
position regarding this, but IIRC the IOMMU spec for RISC-V was behind
the virtualization one, so it's likely that quite a lot of hardware
will have hardware virtualization support but no IOMMU, in which case
I think it's likely the RISC-V port will follow Arm and implement an
identity p2m.

> If yes, I'm fine with the
> exclusion of Arm, otherwise I'd opt for defaulting to yes only for
> X86.
> 
> Either way you can have my:
> 
> Reviewed-by: Juergen Gross <jgross@...e.com>

Thanks. Feel free to change the 'ZONE_DEVICE && !ARM && !ARM64' to
'ZONE_DEVICE && X86' if you think it's safer.

Roger.

Powered by blists - more mailing lists