Re: [PATCH] x86/xen: fix init of balloon stats for PV guests with memory != maxmem

[Matthias Goergens <matthias.goergens@xxxxxxxxx>]

Hi Roger,

thanks for picking this up, and Juergen, thanks for the quick review.  Two
things I believe are still worth addressing; the Fixes: tag can of course
also be fixed up on application.

I think the Fixes: tag should point to 0949c646d646 ("Partial revert
\"x86/xen: fix balloon target initialization for PVH dom0\"").  Commit
87af633689ce changed the initial-page calculation and the extra-region
subtraction together, so those two operations were coherent: the PV initial
count then came from get_num_physpages(), which includes the extra regions.
0949c646d646 restored the PV start_info->nr_pages calculation, which
excludes the extra regions, but retained the subtraction.  Its 6.12.y
backport is also the reporter's identified regression, first seen in
6.12.75.  Applying this patch in a tree that has 87af633689ce but not
0949c646d646 (for example a 6.17-based distro tree) would double-account
the extra region.  This likely also wants Cc: stable@xxxxxxxxxxxxxxx, since
both 6.12.y and 6.18.y carry the 0949c646d646 regression.

Separately, and not something this patch introduces: PVH dom0 has the same
shape of problem on mainline since b13cd24c15d7.  A successful
XENMEM_current_reservation supplies current_pages for both PV and PVH dom0,
and that count excludes the unpopulated xen_extra_mem, so the
xen_pv_domain()-only branch leaves PVH dom0 subtracting those pages again
(-ERANGE, or a silently wrong target, when CONFIG_XEN_UNPOPULATED_ALLOC=n
leaves the regions for the balloon driver).  I am happy to pursue that as
its own thread once this one lands.

Would it be safer to pass balloon_add_regions() an explicit indication of
whether the chosen initial-page count includes the extra physmap regions?
That would cover PV, PVH dom0, and the XENMEM_current_reservation fallback
without deriving the accounting rule solely from the domain type.  On
hypercall failure PVH dom0 falls back to get_num_physpages(), which
includes the extra regions, so keying the accounting on the source of the
count keeps the fallback correct as well.

Thanks,
Matthias