Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On 14.08.20 15:35, Roger Pau Monné wrote: On Fri, Aug 14, 2020 at 02:54:38PM +0200, Jürgen Groß wrote: On 14.08.20 14:47, Roger Pau Monné wrote: On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote: On 14.08.20 11:56, Roger Pau Monné wrote: On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote: On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: 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. So please just select ZONE_DEVICE if this is so much better rather than maintaining two variants. We still need to other variant for Arm at least, so both need to be maintained anyway, even if we force ZONE_DEVICE on x86. Well, it still really helps reproducability if you stick to one implementation of x86. The alternative would be an explicit config option to opt into it, but just getting a different implementation based on a random kernel option is strange. Would adding something like the chunk below to the patch be OK? ---8<--- diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 018020b91baa..5f321a1319e6 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF tristate config XEN_UNPOPULATED_ALLOC - bool - default y if ZONE_DEVICE && !ARM && !ARM64 + bool "Use unpopulated memory ranges for guest mappings" + depends on X86 + select ZONE_DEVICE + default y I'd rather use "default XEN_BACKEND" here, as mappings of other guest's memory is rarely used for non-backend guests. There's also the privcmd and gnt devices which make heavy use of this, so I'm not sure only selecting by default on XEN_BACKEND is the best option. I just want to avoid that kernels built for running as Xen guest, but not as dom0, will be forced to select ZONE_DEVICE. As privcmd is dom0-only, this is no problem. Oh, didn't know that, I somehow assumed privcmd would be available to all Xen guests regardless of whether dom0 support was selected. My remark above should have been more precise in this regard: privcmd operations dealing with foreign mappings are normally limited to dom0 (there might be special cases, like linux-based stubdoms, but those will be configured carefully for that purpose, so they don't need to be considered for selecting the default IMO). In case you are worrying about gnt devices, I'd be fine to switch to default XEN_BACKEND || XEN_GNTDEV Sure. maybe even: default XEN_BACKEND || XEN_GNTDEV || XEN_DOM0 Yes. Do you want me to resend with this changed or are you happy to fixup if there are no further comments? I'd prefer a resend (maybe after Patch 1 has gained its missing Ack, and then with Patch 1 sent to me, too). Juergen
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Fri, Aug 14, 2020 at 02:54:38PM +0200, Jürgen Groß wrote: > On 14.08.20 14:47, Roger Pau Monné wrote: > > On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote: > > > On 14.08.20 11:56, Roger Pau Monné wrote: > > > > On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote: > > > > > On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: > > > > > > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > > > > > > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > > > > > > > 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. > > > > > > > > > > > > > > So please just select ZONE_DEVICE if this is so much better rather > > > > > > > than maintaining two variants. > > > > > > > > > > > > We still need to other variant for Arm at least, so both need to be > > > > > > maintained anyway, even if we force ZONE_DEVICE on x86. > > > > > > > > > > Well, it still really helps reproducability if you stick to one > > > > > implementation of x86. > > > > > > > > > > The alternative would be an explicit config option to opt into it, > > > > > but just getting a different implementation based on a random > > > > > kernel option is strange. > > > > > > > > Would adding something like the chunk below to the patch be OK? > > > > > > > > ---8<--- > > > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > > > > index 018020b91baa..5f321a1319e6 100644 > > > > --- a/drivers/xen/Kconfig > > > > +++ b/drivers/xen/Kconfig > > > > @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF > > > > tristate > > > >config XEN_UNPOPULATED_ALLOC > > > > - bool > > > > - default y if ZONE_DEVICE && !ARM && !ARM64 > > > > + bool "Use unpopulated memory ranges for guest mappings" > > > > + depends on X86 > > > > + select ZONE_DEVICE > > > > + default y > > > > > > I'd rather use "default XEN_BACKEND" here, as mappings of other guest's > > > memory is rarely used for non-backend guests. > > > > There's also the privcmd and gnt devices which make heavy use of this, > > so I'm not sure only selecting by default on XEN_BACKEND is the best > > option. > > I just want to avoid that kernels built for running as Xen guest, but > not as dom0, will be forced to select ZONE_DEVICE. > > As privcmd is dom0-only, this is no problem. Oh, didn't know that, I somehow assumed privcmd would be available to all Xen guests regardless of whether dom0 support was selected. > In case you are worrying about gnt devices, I'd be fine to switch to > > default XEN_BACKEND || XEN_GNTDEV Sure. maybe even: default XEN_BACKEND || XEN_GNTDEV || XEN_DOM0 Do you want me to resend with this changed or are you happy to fixup if there are no further comments? Thanks, Roger.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On 14.08.20 14:47, Roger Pau Monné wrote: On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote: On 14.08.20 11:56, Roger Pau Monné wrote: On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote: On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: 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. So please just select ZONE_DEVICE if this is so much better rather than maintaining two variants. We still need to other variant for Arm at least, so both need to be maintained anyway, even if we force ZONE_DEVICE on x86. Well, it still really helps reproducability if you stick to one implementation of x86. The alternative would be an explicit config option to opt into it, but just getting a different implementation based on a random kernel option is strange. Would adding something like the chunk below to the patch be OK? ---8<--- diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 018020b91baa..5f321a1319e6 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF tristate config XEN_UNPOPULATED_ALLOC - bool - default y if ZONE_DEVICE && !ARM && !ARM64 + bool "Use unpopulated memory ranges for guest mappings" + depends on X86 + select ZONE_DEVICE + default y I'd rather use "default XEN_BACKEND" here, as mappings of other guest's memory is rarely used for non-backend guests. There's also the privcmd and gnt devices which make heavy use of this, so I'm not sure only selecting by default on XEN_BACKEND is the best option. I just want to avoid that kernels built for running as Xen guest, but not as dom0, will be forced to select ZONE_DEVICE. As privcmd is dom0-only, this is no problem. In case you are worrying about gnt devices, I'd be fine to switch to default XEN_BACKEND || XEN_GNTDEV Juergen
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Fri, Aug 14, 2020 at 12:27:32PM +0200, Jürgen Groß wrote: > On 14.08.20 11:56, Roger Pau Monné wrote: > > On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote: > > > On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: > > > > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > > > > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > > > > > 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. > > > > > > > > > > So please just select ZONE_DEVICE if this is so much better rather > > > > > than maintaining two variants. > > > > > > > > We still need to other variant for Arm at least, so both need to be > > > > maintained anyway, even if we force ZONE_DEVICE on x86. > > > > > > Well, it still really helps reproducability if you stick to one > > > implementation of x86. > > > > > > The alternative would be an explicit config option to opt into it, > > > but just getting a different implementation based on a random > > > kernel option is strange. > > > > Would adding something like the chunk below to the patch be OK? > > > > ---8<--- > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > > index 018020b91baa..5f321a1319e6 100644 > > --- a/drivers/xen/Kconfig > > +++ b/drivers/xen/Kconfig > > @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF > > tristate > > config XEN_UNPOPULATED_ALLOC > > - bool > > - default y if ZONE_DEVICE && !ARM && !ARM64 > > + bool "Use unpopulated memory ranges for guest mappings" > > + depends on X86 > > + select ZONE_DEVICE > > + default y > > I'd rather use "default XEN_BACKEND" here, as mappings of other guest's > memory is rarely used for non-backend guests. There's also the privcmd and gnt devices which make heavy use of this, so I'm not sure only selecting by default on XEN_BACKEND is the best option. Thanks, Roger.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On 14.08.20 11:56, Roger Pau Monné wrote: On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote: On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: 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. So please just select ZONE_DEVICE if this is so much better rather than maintaining two variants. We still need to other variant for Arm at least, so both need to be maintained anyway, even if we force ZONE_DEVICE on x86. Well, it still really helps reproducability if you stick to one implementation of x86. The alternative would be an explicit config option to opt into it, but just getting a different implementation based on a random kernel option is strange. Would adding something like the chunk below to the patch be OK? ---8<--- diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 018020b91baa..5f321a1319e6 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF tristate config XEN_UNPOPULATED_ALLOC - bool - default y if ZONE_DEVICE && !ARM && !ARM64 + bool "Use unpopulated memory ranges for guest mappings" + depends on X86 + select ZONE_DEVICE + default y I'd rather use "default XEN_BACKEND" here, as mappings of other guest's memory is rarely used for non-backend guests. Juergen
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Fri, Aug 14, 2020 at 08:29:20AM +0100, Christoph Hellwig wrote: > On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: > > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > > > 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. > > > > > > So please just select ZONE_DEVICE if this is so much better rather > > > than maintaining two variants. > > > > We still need to other variant for Arm at least, so both need to be > > maintained anyway, even if we force ZONE_DEVICE on x86. > > Well, it still really helps reproducability if you stick to one > implementation of x86. > > The alternative would be an explicit config option to opt into it, > but just getting a different implementation based on a random > kernel option is strange. Would adding something like the chunk below to the patch be OK? ---8<--- diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 018020b91baa..5f321a1319e6 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -328,7 +328,14 @@ config XEN_FRONT_PGDIR_SHBUF tristate config XEN_UNPOPULATED_ALLOC - bool - default y if ZONE_DEVICE && !ARM && !ARM64 + bool "Use unpopulated memory ranges for guest mappings" + depends on X86 + select ZONE_DEVICE + default y + help + Use unpopulated memory ranges in order to create mappings for guest + memory regions, including grants maps and foreign pages. This avoids + having to balloon out RAM regions in order to obtain physical memory + space to create such mappings. endmenu
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monn?? wrote: > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > > 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. > > > > So please just select ZONE_DEVICE if this is so much better rather > > than maintaining two variants. > > We still need to other variant for Arm at least, so both need to be > maintained anyway, even if we force ZONE_DEVICE on x86. Well, it still really helps reproducability if you stick to one implementation of x86. The alternative would be an explicit config option to opt into it, but just getting a different implementation based on a random kernel option is strange.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
Your email client seems to set 'Reply-to:' to point to everyone on the 'Cc:' field, but not yourself, which is kind of weird. I've manually fixed it on this reply by moving everyone to the 'Cc:' field and setting you on 'To:'. On Thu, Aug 13, 2020 at 11:49:46AM +0200, Daniel Vetter wrote: > On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monné wrote: > > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > > > 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. > > > > > > So please just select ZONE_DEVICE if this is so much better rather > > > than maintaining two variants. > > > > We still need to other variant for Arm at least, so both need to be > > maintained anyway, even if we force ZONE_DEVICE on x86. > > Why does arm not have ZONE_DEVICE? It's not that Arm doesn't have ZONE_DEVICE, it's just that the approach used here won't work correctly on an Arm Xen dom0 as-is. This is due to the usage of an identity second stage translation in order to workaround the lack of an IOMMU in some Arm boards. It can be made to work on Arm, but will likely require someone from the Arm side doing that. Roger.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Thu, Aug 13, 2020 at 09:54:20AM +0200, Roger Pau Monné wrote: > On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > > 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. > > > > So please just select ZONE_DEVICE if this is so much better rather > > than maintaining two variants. > > We still need to other variant for Arm at least, so both need to be > maintained anyway, even if we force ZONE_DEVICE on x86. Why does arm not have ZONE_DEVICE? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Thu, Aug 13, 2020 at 08:33:37AM +0100, Christoph Hellwig wrote: > On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > > 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. > > So please just select ZONE_DEVICE if this is so much better rather > than maintaining two variants. We still need to other variant for Arm at least, so both need to be maintained anyway, even if we force ZONE_DEVICE on x86. Thanks, Roger.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
On Tue, Aug 11, 2020 at 11:44:47AM +0200, Roger Pau Monne wrote: > 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. So please just select ZONE_DEVICE if this is so much better rather than maintaining two variants.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
Hi Roger,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on linux/master hnaz-linux-mm/master linus/master v5.8
next-20200812]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Roger-Pau-Monne/xen-balloon-fixes-for-memory-hotplug/20200811-174616
base: https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
config: x86_64-randconfig-a001-20200812 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
30c1633386e7cfb01c0a54b31ccf4c3a3873e71b)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from drivers/xen/grant-table.c:59:
include/xen/balloon.h:8:8: error: redefinition of 'balloon_stats'
struct balloon_stats {
^
include/xen/xen.h:61:10: note: 'include/xen/balloon.h' included multiple
times, additional include site here
#include
^
drivers/xen/grant-table.c:59:10: note: 'include/xen/balloon.h' included
multiple times, additional include site here
#include
^
include/xen/balloon.h:8:8: note: unguarded header; consider using #ifdef
guards or #pragma once
struct balloon_stats {
^
In file included from drivers/xen/grant-table.c:59:
>> include/xen/balloon.h:33:20: error: redefinition of 'xen_balloon_init'
static inline void xen_balloon_init(void)
^
include/xen/balloon.h:33:20: note: previous definition is here
static inline void xen_balloon_init(void)
^
2 errors generated.
--
In file included from drivers/xen/balloon.c:70:
include/xen/balloon.h:8:8: error: redefinition of 'balloon_stats'
struct balloon_stats {
^
include/xen/xen.h:61:10: note: 'include/xen/balloon.h' included multiple
times, additional include site here
#include
^
drivers/xen/balloon.c:70:10: note: 'include/xen/balloon.h' included multiple
times, additional include site here
#include
^
include/xen/balloon.h:8:8: note: unguarded header; consider using #ifdef
guards or #pragma once
struct balloon_stats {
^
In file included from drivers/xen/balloon.c:70:
>> include/xen/balloon.h:33:20: error: redefinition of 'xen_balloon_init'
static inline void xen_balloon_init(void)
^
include/xen/balloon.h:33:20: note: previous definition is here
static inline void xen_balloon_init(void)
^
2 errors generated.
--
In file included from drivers/xen/gntdev.c:41:
include/xen/balloon.h:8:8: error: redefinition of 'balloon_stats'
struct balloon_stats {
^
include/xen/xen.h:61:10: note: 'include/xen/balloon.h' included multiple
times, additional include site here
#include
^
drivers/xen/gntdev.c:41:10: note: 'include/xen/balloon.h' included multiple
times, additional include site here
#include
^
include/xen/balloon.h:8:8: note: unguarded header; consider using #ifdef
guards or #pragma once
struct balloon_stats {
^
In file included from drivers/xen/gntdev.c:41:
>> include/xen/balloon.h:33:20: error: redefinition of 'xen_balloon_init'
static inline void xen_balloon_init(void)
^
include/xen/balloon.h:33:20: note: previous definition is here
static inline void xen_balloon_init(void)
^
In file included from drivers/xen/gntdev.c:48:
In file included from drivers/xen/gntdev-common.h:15:
include/linux/mman.h:134:9: warning: division by zero is undefined
[-Wdivision-by-zero]
_calc_vm_trans(flags, MAP_SYNC, VM_SYNC );
^~~~
include/linux/mman.h:111:21: note: expanded from macro '_calc_vm_trans'
: ((x) & (bit1)) / ((bit1) / (bit2
^ ~
1 warning and 2 errors generated.
--
In file included from drivers/xen/privcmd.c:40:
include/xen/balloon.h:8:8: error: redefinition of 'balloon_stats'
struct balloon_stats {
^
include/xen/xen.h:61:10: note: 'include/xen/balloon.h' included multiple
times, additional include site here
#include
^
drivers/xen/privcmd.c:40:10: note: 'include
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é
> > ---
> > Cc: Oleksandr Andrushchenko
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Cc: Boris Ostrovsky
> > Cc: Juergen Gross
> > Cc: Stefano Stabellini
> > Cc: Dan Carpenter
> > Cc: Roger Pau Monne
> > Cc: Wei Liu
> > Cc: Yan Yankovskyi
> > Cc: dri-de...@lists.freedesktop.org
> > Cc: xen-de...@lists.xenproject.org
> > Cc: linux...@kvack.org
> > Cc: David Hildenbrand
> > Cc: Michal Hocko
> > Cc: Dan Williams
> > ---
> > 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
Thanks. Feel free to change the 'ZONE_DEVICE && !ARM && !ARM64' to
'ZONE_DEVICE && X86' if you think it's safer.
Roger.
Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
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é
---
Cc: Oleksandr Andrushchenko
Cc: David Airlie
Cc: Daniel Vetter
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
Cc: Dan Carpenter
Cc: Roger Pau Monne
Cc: Wei Liu
Cc: Yan Yankovskyi
Cc: dri-de...@lists.freedesktop.org
Cc: xen-de...@lists.xenproject.org
Cc: linux...@kvack.org
Cc: David Hildenbrand
Cc: Michal Hocko
Cc: Dan Williams
---
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? 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
Juergen
[PATCH v4 2/2] xen: add helpers to allocate unpopulated memory
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é
---
Cc: Oleksandr Andrushchenko
Cc: David Airlie
Cc: Daniel Vetter
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
Cc: Dan Carpenter
Cc: Roger Pau Monne
Cc: Wei Liu
Cc: Yan Yankovskyi
Cc: dri-de...@lists.freedesktop.org
Cc: xen-de...@lists.xenproject.org
Cc: linux...@kvack.org
Cc: David Hildenbrand
Cc: Michal Hocko
Cc: Dan Williams
---
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/gpu/drm/xen/xen_drm_front_gem.c
b/drivers/gpu/drm/xen/xen_drm_front_gem.c
index f0b85e094111..270e1bd3e4da 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -18,6 +18,7 @@
#include
#include
+#include
#include "xen_drm_front.h"
#include "xen_drm_front_gem.h"
@@ -99,8 +100,8 @@ static struct xen_gem_object *gem_create(struct drm_device
*dev, size_t size)
* allocate ballooned pages which will be used to map
* grant references provided by the backend
*/
- ret = alloc_xenballooned_pages(xen_obj->num_pages,
- xen_obj->pages);
+ ret = xen_alloc_unpopulated_pages(xen_obj->num_pages,
+ xen_obj->pages);
if (ret < 0) {
DRM_ERROR("Cannot allocate %zu ballooned pages: %d\n",
xen_obj->num_pages, ret);
@@ -152,8 +153,8 @@ void xen_drm_front_gem_free_object_unlocked(struct
drm_gem_object *gem_obj)
} else {
if (xen_obj->pages) {
if (xen_obj->be_alloc) {
- free_xenballooned_pages(xen_obj->num_pages,
- xen_obj->pages);
+ xen_free_unpopulated_pages(xen_obj->num_pages,
+ xen_obj->pages);
gem_free_pages_array(xen_obj);
} else {
drm_gem_put_pages(&xen_obj->base,
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
+
endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index c25c9a699b48..babdc
