Re: [PATCH v4 2/2] xen: add helpers to allocate unpopulated memory

From: Jürgen Groß
Date: Fri Aug 14 2020 - 09:57:34 EST


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