Re: [PATCH 1/2] xen: fix setting of max_pfn in shared_info

[Juergen Gross <jgross@xxxxxxxx>]

On 16.06.21 11:52, Jan Beulich wrote:
> On 16.06.2021 09:30, Juergen Gross wrote:
> > Xen PV guests are specifying the highest used PFN via the max_pfn
> > field in shared_info. This value is used by the Xen tools when saving
> > or migrating the guest.
> >
> > Unfortunately this field is misnamed, as in reality it is specifying
> > the number of pages (including any memory holes) of the guest, so it
> > is the highest used PFN + 1. Renaming isn't possible, as this is a
> > public Xen hypervisor interface which needs to be kept stable.
> >
> > The kernel will set the value correctly initially at boot time, but
> > when adding more pages (e.g. due to memory hotplug or ballooning) a
> > real PFN number is stored in max_pfn. This is done when expanding the
> > p2m array, and the PFN stored there is even possibly wrong, as it
> > should be the last possible PFN of the just added P2M frame, and not
> > one which led to the P2M expansion.
> >
> > Fix that by setting shared_info->max_pfn to the last possible PFN + 1.
> >
> > Fixes: 98dd166ea3a3c3 ("x86/xen/p2m: hint at the last populated P2M entry")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> The code change is fine, so
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> But I think even before the rename you would want to clarify the comment
> next to the variable's definition, to make clear what it really holds.
It already says: "Number of valid entries in the p2m table(s) ..."
What do you think is unclear about that? Or do you mean another
variable?


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature