[PATCH v2 0/2] xen/x86: alternative fix for XSA-369

[PATCH v2 0/2] xen/x86: alternative fix for XSA-369

[Roger Pau Monne]

Hello,

This is a proposal for an alternative fix for XSA-369 that instead of
special casing XEN_UNPOPULATED_ALLOC to size the p2m relies on making
XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on the generic MEMORY_HOTPLUG
option rather than XEN_BALLOON_MEMORY_HOTPLUG.

I think this is safer, as we don't want to be special casing any option
that pulls in generic MEMORY_HOTPLUG without XEN_BALLOON_MEMORY_HOTPLUG.
Without this we would also need to at least special case ZONE_DEVICE
which also relies on MEMORY_HOTPLUG, and is what pulls the generic
MEMORY_HOTPLUG option even when XEN_BALLOON_MEMORY_HOTPLUG is disabled
with XEN_UNPOPULATED_ALLOC.

Thanks, Roger.

Roger Pau Monne (2):
  xen/x86: make XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on
    MEMORY_HOTPLUG
  Revert "xen: fix p2m size in dom0 for disabled memory hotplug case"

 arch/x86/include/asm/xen/page.h | 12 ------------
 arch/x86/xen/p2m.c              |  7 ++-----
 arch/x86/xen/setup.c            | 16 ++++++++++++++--
 drivers/xen/Kconfig             |  4 ++--
 4 files changed, 18 insertions(+), 21 deletions(-)

-- 
2.30.1

[PATCH v2 1/2] xen/x86: make XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on MEMORY_HOTPLUG

[Roger Pau Monne]

The Xen memory hotplug limit should depend on the memory hotplug
generic option, rather than the Xen balloon configuration. It's
possible to have a kernel with generic memory hotplug enabled, but
without Xen balloon enabled, at which point memory hotplug won't work
correctly due to the size limitation of the p2m.

Rename the option to XEN_MEMORY_HOTPLUG_LIMIT since it's no longer
tied to ballooning.

Fixes: 9e2369c06c8a18 ("xen: add helpers to allocate unpopulated memory")
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: xen-devel@lists.xenproject.org
---
 arch/x86/xen/p2m.c  | 4 ++--
 drivers/xen/Kconfig | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 17d80f751fcb..a33902d05e45 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -98,8 +98,8 @@ EXPORT_SYMBOL_GPL(xen_p2m_size);
 unsigned long xen_max_p2m_pfn __read_mostly;
 EXPORT_SYMBOL_GPL(xen_max_p2m_pfn);
 
-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG_LIMIT
-#define P2M_LIMIT CONFIG_XEN_BALLOON_MEMORY_HOTPLUG_LIMIT
+#ifdef CONFIG_XEN_MEMORY_HOTPLUG_LIMIT
+#define P2M_LIMIT CONFIG_XEN_MEMORY_HOTPLUG_LIMIT
 #else
 #define P2M_LIMIT 0
 #endif
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 41645fe6ad48..ea0efd290c37 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -50,11 +50,11 @@ config XEN_BALLOON_MEMORY_HOTPLUG
 
 	  SUBSYSTEM=="memory", ACTION=="add", RUN+="/bin/sh -c '[ -f /sys$devpath/state ] && echo online > /sys$devpath/state'"
 
-config XEN_BALLOON_MEMORY_HOTPLUG_LIMIT
+config XEN_MEMORY_HOTPLUG_LIMIT
 	int "Hotplugged memory limit (in GiB) for a PV guest"
 	default 512
 	depends on XEN_HAVE_PVMMU
-	depends on XEN_BALLOON_MEMORY_HOTPLUG
+	depends on MEMORY_HOTPLUG
 	help
 	  Maxmium amount of memory (in GiB) that a PV guest can be
 	  expanded to when using memory hotplug.
-- 
2.30.1

[PATCH v2 2/2] Revert "xen: fix p2m size in dom0 for disabled memory hotplug case"

[Roger Pau Monne]

This partially reverts commit 882213990d32fd224340a4533f6318dd152be4b2.

There's no need to special case XEN_UNPOPULATED_ALLOC anymore in order
to correctly size the p2m. The generic memory hotplug option has
already been tied together with the Xen hotplug limit, so enabling
memory hotplug should already trigger a properly sized p2m on Xen PV.

Note that XEN_UNPOPULATED_ALLOC depends on ZONE_DEVICE which pulls in
MEMORY_HOTPLUG.

Leave the check added to __set_phys_to_machine and the adjusted
comment about EXTRA_MEM_RATIO.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: xen-devel@lists.xenproject.org
---
Changes since v1:
 - Expand commit message.
 - Do not revert code comment.
---
 arch/x86/include/asm/xen/page.h | 12 ------------
 arch/x86/xen/p2m.c              |  3 ---
 arch/x86/xen/setup.c            | 16 ++++++++++++++--
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 7068e4bb057d..1a162e559753 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -86,18 +86,6 @@ clear_foreign_p2m_mapping(struct gnttab_unmap_grant_ref *unmap_ops,
 }
 #endif
 
-/*
- * The maximum amount of extra memory compared to the base size.  The
- * main scaling factor is the size of struct page.  At extreme ratios
- * of base:extra, all the base memory can be filled with page
- * structures for the extra memory, leaving no space for anything
- * else.
- *
- * 10x seems like a reasonable balance between scaling flexibility and
- * leaving a practically usable system.
- */
-#define XEN_EXTRA_MEM_RATIO	(10)
-
 /*
  * Helper functions to write or read unsigned long values to/from
  * memory, when the access may fault.
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index a33902d05e45..ac06ca32e9ef 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -416,9 +416,6 @@ void __init xen_vmalloc_p2m_tree(void)
 	xen_p2m_last_pfn = xen_max_p2m_pfn;
 
 	p2m_limit = (phys_addr_t)P2M_LIMIT * 1024 * 1024 * 1024 / PAGE_SIZE;
-	if (!p2m_limit && IS_ENABLED(CONFIG_XEN_UNPOPULATED_ALLOC))
-		p2m_limit = xen_start_info->nr_pages * XEN_EXTRA_MEM_RATIO;
-
 	vm.flags = VM_ALLOC;
 	vm.size = ALIGN(sizeof(unsigned long) * max(xen_max_p2m_pfn, p2m_limit),
 			PMD_SIZE * PMDS_PER_MID_PAGE);
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 1a3b75652fa4..99ef476dc702 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -59,6 +59,18 @@ static struct {
 } xen_remap_buf __initdata __aligned(PAGE_SIZE);
 static unsigned long xen_remap_mfn __initdata = INVALID_P2M_ENTRY;
 
+/* 
+ * The maximum amount of extra memory compared to the base size.  The
+ * main scaling factor is the size of struct page.  At extreme ratios
+ * of base:extra, all the base memory can be filled with page
+ * structures for the extra memory, leaving no space for anything
+ * else.
+ * 
+ * 10x seems like a reasonable balance between scaling flexibility and
+ * leaving a practically usable system.
+ */
+#define EXTRA_MEM_RATIO		(10)
+
 static bool xen_512gb_limit __initdata = IS_ENABLED(CONFIG_XEN_512GB);
 
 static void __init xen_parse_512gb(void)
@@ -778,13 +790,13 @@ char * __init xen_memory_setup(void)
 		extra_pages += max_pages - max_pfn;
 
 	/*
-	 * Clamp the amount of extra memory to a XEN_EXTRA_MEM_RATIO
+	 * Clamp the amount of extra memory to a EXTRA_MEM_RATIO
 	 * factor the base size.
 	 *
 	 * Make sure we have no memory above max_pages, as this area
 	 * isn't handled by the p2m management.
 	 */
-	extra_pages = min3(XEN_EXTRA_MEM_RATIO * min(max_pfn, PFN_DOWN(MAXMEM)),
+	extra_pages = min3(EXTRA_MEM_RATIO * min(max_pfn, PFN_DOWN(MAXMEM)),
 			   extra_pages, max_pages - max_pfn);
 	i = 0;
 	addr = xen_e820_table.entries[0].addr;
-- 
2.30.1

Re: [PATCH v2 0/2] xen/x86: alternative fix for XSA-369

[Boris Ostrovsky]

On 3/24/21 8:24 AM, Roger Pau Monne wrote:
> Hello,
>
> This is a proposal for an alternative fix for XSA-369 that instead of
> special casing XEN_UNPOPULATED_ALLOC to size the p2m relies on making
> XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on the generic MEMORY_HOTPLUG
> option rather than XEN_BALLOON_MEMORY_HOTPLUG.
>
> I think this is safer, as we don't want to be special casing any option
> that pulls in generic MEMORY_HOTPLUG without XEN_BALLOON_MEMORY_HOTPLUG.
> Without this we would also need to at least special case ZONE_DEVICE
> which also relies on MEMORY_HOTPLUG, and is what pulls the generic
> MEMORY_HOTPLUG option even when XEN_BALLOON_MEMORY_HOTPLUG is disabled
> with XEN_UNPOPULATED_ALLOC.
>
> Thanks, Roger.
>
> Roger Pau Monne (2):
>   xen/x86: make XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on
>     MEMORY_HOTPLUG
>   Revert "xen: fix p2m size in dom0 for disabled memory hotplug case"
>
>  arch/x86/include/asm/xen/page.h | 12 ------------
>  arch/x86/xen/p2m.c              |  7 ++-----
>  arch/x86/xen/setup.c            | 16 ++++++++++++++--
>  drivers/xen/Kconfig             |  4 ++--
>  4 files changed, 18 insertions(+), 21 deletions(-)


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Re: [PATCH v2 0/2] xen/x86: alternative fix for XSA-369

[Boris Ostrovsky]

On 3/24/21 7:09 PM, Boris Ostrovsky wrote:
> On 3/24/21 8:24 AM, Roger Pau Monne wrote:
>> Hello,
>>
>> This is a proposal for an alternative fix for XSA-369 that instead of
>> special casing XEN_UNPOPULATED_ALLOC to size the p2m relies on making
>> XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on the generic MEMORY_HOTPLUG
>> option rather than XEN_BALLOON_MEMORY_HOTPLUG.
>>
>> I think this is safer, as we don't want to be special casing any option
>> that pulls in generic MEMORY_HOTPLUG without XEN_BALLOON_MEMORY_HOTPLUG.
>> Without this we would also need to at least special case ZONE_DEVICE
>> which also relies on MEMORY_HOTPLUG, and is what pulls the generic
>> MEMORY_HOTPLUG option even when XEN_BALLOON_MEMORY_HOTPLUG is disabled
>> with XEN_UNPOPULATED_ALLOC.
>>
>> Thanks, Roger.
>>
>> Roger Pau Monne (2):
>>   xen/x86: make XEN_BALLOON_MEMORY_HOTPLUG_LIMIT depend on
>>     MEMORY_HOTPLUG
>>   Revert "xen: fix p2m size in dom0 for disabled memory hotplug case"
>>
>>  arch/x86/include/asm/xen/page.h | 12 ------------
>>  arch/x86/xen/p2m.c              |  7 ++-----
>>  arch/x86/xen/setup.c            | 16 ++++++++++++++--
>>  drivers/xen/Kconfig             |  4 ++--
>>  4 files changed, 18 insertions(+), 21 deletions(-)
>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


Applied to for-linus-5.12b.


-boris