[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a72553d1-3d79-4314-aa41-11a09dc60089@amd.com>
Date: Thu, 22 Jan 2026 09:40:01 -0500
From: Jason Andryuk <jason.andryuk@....com>
To: Roger Pau Monné <roger.pau@...rix.com>
CC: <xen-devel@...ts.xenproject.org>, <linux-kernel@...r.kernel.org>, "James
Dingwall" <james@...gwall.me.uk>, Juergen Gross <jgross@...e.com>, "Stefano
Stabellini" <sstabellini@...nel.org>, Oleksandr Tyshchenko
<oleksandr_tyshchenko@...m.com>
Subject: Re: [PATCH] Partial revert "x86/xen: fix balloon target
initialization for PVH dom0"
On 2026-01-21 12:49, Roger Pau Monné wrote:
> On Wed, Jan 21, 2026 at 12:21:33PM -0500, Jason Andryuk wrote:
>> On 2026-01-21 06:17, Roger Pau Monné wrote:
>>> On Tue, Jan 20, 2026 at 03:10:06PM -0500, Jason Andryuk wrote:
>>>> On 2026-01-20 09:06, Roger Pau Monne wrote:
>>>>> This partially reverts commit 87af633689ce16ddb166c80f32b120e50b1295de so
>>>>> the current memory target for PV guests is still fetched from
>>>>> start_info->nr_pages, which matches exactly what the toolstack sets the
>>>>> initial memory target to.
>>>>>
>>>>> Using get_num_physpages() is possible on PV also, but needs adjusting to
>>>>> take into account the ISA hole and the PFN at 0 not considered usable
>>>>> memory depite being populated, and hence would need extra adjustments.
>>>>> Instead of carrying those extra adjustments switch back to the previous
>>>>> code. That leaves Linux with a difference in how current memory target is
>>>>> obtained for HVM vs PV, but that's better than adding extra logic just for
>>>>> PV.
>>>>>
>>>>> Also, for HVM the target is not (and has never been) accurately calculated,
>>>>> as in that case part of what starts as guest memory is reused by hvmloader
>>>>> and possibly other firmware to store ACPI tables and similar firmware
>>>>> information, thus the memory is no longer being reported as RAM in the
>>>>> memory map.
>>>>>
>>>>> Reported-by: James Dingwall <james@...gwall.me.uk>
>>>>> Signed-off-by: Roger Pau Monné <roger.pau@...rix.com>
>>>>
>>>> Reviewed-by: Jason Andryuk <jason.andryuk@....com>
>>>
>>> Thanks.
>>>
>>> I've been considering what we discussed and as a separate follow up we
>>> might want to attempt to switch to using `XENMEM_current_reservation`
>>> for dom0? It might make the accounting in PVH dom0 better, as that's
>>> what the toolstack uses to set the xenstore target when initializing
>>> dom0 values.
>>
>> Yes, I thought that could be a follow on. I've attached what I have tested,
>> but it is based on a branch pre-dating xen_released_pages.
>> xenmem_current_reservation with PVH dom0 seemed good without the
>> xen_released_pages adjustment, but I don't know what that would be for a PVH
>> dom0.
>>
>> Regards,
>> Jason
>
>> From 8b628ad0ebe52c30e31298e868f2f5187f2f52da Mon Sep 17 00:00:00 2001
>> From: Jason Andryuk <jason.andryuk@....com>
>> Date: Fri, 7 Nov 2025 16:44:53 -0500
>> Subject: [PATCH] xen/balloon: Initialize dom0 with XENMEM_current_reservation
>>
>> The balloon driver bases its action off the memory/target and
>> memory/static-max xenstore keys. These are set by the toolstack and
>> match the domain's hypervisor allocated pages - domain_tot_pages().
>>
>> However, PVH and HVM domains query get_num_physpages() for the initial
>> balloon driver current and target pages. get_num_physpages() is different
>> from dom_totain_pages(), and has been observed way off in a PVH dom0.
>> Here a PVH dom0 is assigned 917503 pages (3.5GB), but
>> get_num_physpages() is 7312455:
>> xen:balloon: pages curr 7312455 target 7312455
>> xen:balloon: current_reservation 917503
>>
>> While Xen truncated the PVH dom0 memory map's RAM, Linux undoes that
>> operation and restores RAM regions in pvh_reserve_extra_memory().
>>
>> Use XENMEM_current_reveration to initialize the balloon driver current
>> and target pages. This is the hypervisor value, and matches what the
>> toolstack writes. This avoids any ballooning from the inital
>> allocation. While domUs are affected, the implications of the change
>> are unclear - only make the change for dom0.
>>
>> Signed-off-by: Jason Andryuk <jason.andryuk@....com>
>> ---
>> drivers/xen/balloon.c | 9 ++++++---
>> drivers/xen/mem-reservation.c | 8 ++++++++
>> include/xen/interface/memory.h | 5 +++++
>> include/xen/mem-reservation.h | 2 ++
>> 4 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
>> index 528395133b4f..fa6cbe6ce2ca 100644
>> --- a/drivers/xen/balloon.c
>> +++ b/drivers/xen/balloon.c
>> @@ -713,10 +713,13 @@ static int __init balloon_init(void)
>>
>> #ifdef CONFIG_XEN_PV
>> balloon_stats.current_pages = xen_pv_domain()
>> - ? min(xen_start_info->nr_pages - xen_released_pages, max_pfn)
>> - : get_num_physpages();
>> + ? min(xen_start_info->nr_pages - xen_released_pages, max_pfn) :
>> + xen_initial_domain() ? xenmem_current_reservation() :
>> + get_num_physpages();
>> #else
>> - balloon_stats.current_pages = get_num_physpages();
>> + balloon_stats.current_pages =
>> + xen_initial_domain() ? xenmem_current_reservation() :
>> + get_num_physpages();
>> #endif
>> balloon_stats.target_pages = balloon_stats.current_pages;
>> balloon_stats.balloon_low = 0;
>> diff --git a/drivers/xen/mem-reservation.c b/drivers/xen/mem-reservation.c
>> index 24648836e0d4..40c5c40d34fe 100644
>> --- a/drivers/xen/mem-reservation.c
>> +++ b/drivers/xen/mem-reservation.c
>> @@ -113,3 +113,11 @@ int xenmem_reservation_decrease(int count, xen_pfn_t *frames)
>> return HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation);
>> }
>> EXPORT_SYMBOL_GPL(xenmem_reservation_decrease);
>> +
>> +long xenmem_current_reservation(void)
>> +{
>> + struct xen_memory_domain domain = { .domid = DOMID_SELF };
>> +
>> + return HYPERVISOR_memory_op(XENMEM_current_reservation, &domain);
>> +}
>> +EXPORT_SYMBOL_GPL(xenmem_current_reservation);
>> diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h
>> index 1a371a825c55..72619a75fed2 100644
>> --- a/include/xen/interface/memory.h
>> +++ b/include/xen/interface/memory.h
>> @@ -104,6 +104,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_memory_exchange);
>> */
>> #define XENMEM_maximum_ram_page 2
>>
>> +struct xen_memory_domain {
>> + /* [IN] Domain information is being queried for. */
>> + domid_t domid;
>> +};
>
> Other callers that would use xen_memory_domain just pass a pointer to
> a domid_t, I think you could simplify the patch to the diff below,
> which sits on top of the patch here:
Ah, yes, xen_memory_domain is just wrapping a domid.
>
> I haven't tested it yet to see whether that's OK to do on PV, I would
> think PV and PVH would be the same here, since the setting of the
> xenstore target value is based in the return of
> XENMEM_current_reservation for both.
On a system with 32GB and dom0=pvh dom0_mem=7G:
[ 0.295201] xen:balloon: current_pages: 1835007 get_num_physpages
8220126 xen_released_pages 6385120
[ 0.295201] ------------[ cut here ]------------
[ 0.295201] Released pages underflow current target
8220126 - 6385120 = 1835006
And also for PV:
[ 1.406923] xen:balloon: current_pages: 1835008 get_num_physpages
8220127 xen_released_pages 6385120
[ 1.406928] ------------[ cut here ]------------
[ 1.406931] Released pages underflow current target
So we don't want to subtract xen_released_pages for dom0. Is
xen_released_pages expected to be non-zero for a domU?
IIRC, for a domU, xl writes the xenstore nodes as the ~build time memory
value, which doesn't include video ram. Later QEMU populates the
videoram, which increases current reservation. Then the two values
don't match when the domU initializes the balloon values.
Regards,
Jason
Powered by blists - more mailing lists