[PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Tamas K Lengyel]

When creating a VM fork copy the parent VM's hostp2m max_mapped_pfn value. Some
toolstack relies on the XENMEM_maximum_gpfn value to establish the maximum
addressable physical memory in the VM and for forks that have not yet been
unpaused that value is not going to reflect the correct max gpfn that's
possible to populate into the p2m. This patch fixes the issue.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
---
 xen/arch/x86/mm/mem_sharing.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 00ada05c10..98b14f7b0a 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct domain *d)
         return rc;
 
     copy_tsc(cd, d);
+    p2m_get_hostp2m(cd)->max_mapped_pfn = p2m_get_hostp2m(d)->max_mapped_pfn;
 
     return rc;
 }
-- 
2.25.1

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Jan Beulich]

On 18.03.2021 22:36, Tamas K Lengyel wrote:
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct domain *d)
>          return rc;
>  
>      copy_tsc(cd, d);
> +    p2m_get_hostp2m(cd)->max_mapped_pfn = p2m_get_hostp2m(d)->max_mapped_pfn;

Makes sense to me, yes, but then an immediate question is: What
about the somewhat similar {min,max}_remapped_gfn fields? Which
of course implies the more general question of how alternate
p2m-s (are supposed to) get treated in the first place. From my
looking at it, fork() doesn't appear to also fork those, but
also doesn't appear to refuse cloning when altp2m is in use.

Jan

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Tamas K Lengyel]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On Fri, Mar 19, 2021, 6:23 AM Jan Beulich <jbeulich@suse.com> wrote:

> On 18.03.2021 22:36, Tamas K Lengyel wrote:
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct
> domain *d)
> >          return rc;
> >
> >      copy_tsc(cd, d);
> > +    p2m_get_hostp2m(cd)->max_mapped_pfn =
> p2m_get_hostp2m(d)->max_mapped_pfn;
>
> Makes sense to me, yes, but then an immediate question is: What
> about the somewhat similar {min,max}_remapped_gfn fields? Which
> of course implies the more general question of how alternate
> p2m-s (are supposed to) get treated in the first place. From my
> looking at it, fork() doesn't appear to also fork those, but
> also doesn't appear to refuse cloning when altp2m is in use.
>

It's untested, forking and altp2m is not currently used simultaniously.
Don't know if it should be restricted as not working as I haven't tested
it. Both forking and altp2m is experimental so there be dragons. At some
point I would like to be able to use altp2m in forks but forking a domain
that has altp2m enabled will likely be a setup that's too insane to try to
get working.

Tamas

>

[-- Attachment #2: Type: text/html, Size: 1844 bytes --]

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Jan Beulich]

On 19.03.2021 12:06, Tamas K Lengyel wrote:
> On Fri, Mar 19, 2021, 6:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> 
>> On 18.03.2021 22:36, Tamas K Lengyel wrote:
>>> --- a/xen/arch/x86/mm/mem_sharing.c
>>> +++ b/xen/arch/x86/mm/mem_sharing.c
>>> @@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct
>> domain *d)
>>>          return rc;
>>>
>>>      copy_tsc(cd, d);
>>> +    p2m_get_hostp2m(cd)->max_mapped_pfn =
>> p2m_get_hostp2m(d)->max_mapped_pfn;
>>
>> Makes sense to me, yes, but then an immediate question is: What
>> about the somewhat similar {min,max}_remapped_gfn fields? Which
>> of course implies the more general question of how alternate
>> p2m-s (are supposed to) get treated in the first place. From my
>> looking at it, fork() doesn't appear to also fork those, but
>> also doesn't appear to refuse cloning when altp2m is in use.
>>
> 
> It's untested, forking and altp2m is not currently used simultaniously.
> Don't know if it should be restricted as not working as I haven't tested
> it. Both forking and altp2m is experimental so there be dragons. At some
> point I would like to be able to use altp2m in forks but forking a domain
> that has altp2m enabled will likely be a setup that's too insane to try to
> get working.

Well, I see only two (consistent) options - either the other two
fields mentioned get copied as well, or altp2m use results in
forking getting refused.

Jan

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Tamas K Lengyel]

On Fri, Mar 19, 2021 at 7:25 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 19.03.2021 12:06, Tamas K Lengyel wrote:
> > On Fri, Mar 19, 2021, 6:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> >> On 18.03.2021 22:36, Tamas K Lengyel wrote:
> >>> --- a/xen/arch/x86/mm/mem_sharing.c
> >>> +++ b/xen/arch/x86/mm/mem_sharing.c
> >>> @@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct
> >> domain *d)
> >>>          return rc;
> >>>
> >>>      copy_tsc(cd, d);
> >>> +    p2m_get_hostp2m(cd)->max_mapped_pfn =
> >> p2m_get_hostp2m(d)->max_mapped_pfn;
> >>
> >> Makes sense to me, yes, but then an immediate question is: What
> >> about the somewhat similar {min,max}_remapped_gfn fields? Which
> >> of course implies the more general question of how alternate
> >> p2m-s (are supposed to) get treated in the first place. From my
> >> looking at it, fork() doesn't appear to also fork those, but
> >> also doesn't appear to refuse cloning when altp2m is in use.
> >>
> >
> > It's untested, forking and altp2m is not currently used simultaniously.
> > Don't know if it should be restricted as not working as I haven't tested
> > it. Both forking and altp2m is experimental so there be dragons. At some
> > point I would like to be able to use altp2m in forks but forking a domain
> > that has altp2m enabled will likely be a setup that's too insane to try to
> > get working.
>
> Well, I see only two (consistent) options - either the other two
> fields mentioned get copied as well, or altp2m use results in
> forking getting refused.

Sure, but that's a separate issue from what this patch addresses so at
this point I don't plan on including that work in here.

Tamas

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Tamas K Lengyel]

On Thu, Mar 18, 2021 at 5:36 PM Tamas K Lengyel <tamas.lengyel@intel.com> wrote:
>
> When creating a VM fork copy the parent VM's hostp2m max_mapped_pfn value. Some
> toolstack relies on the XENMEM_maximum_gpfn value to establish the maximum
> addressable physical memory in the VM and for forks that have not yet been
> unpaused that value is not going to reflect the correct max gpfn that's
> possible to populate into the p2m. This patch fixes the issue.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> ---
>  xen/arch/x86/mm/mem_sharing.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> index 00ada05c10..98b14f7b0a 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct domain *d)
>          return rc;
>
>      copy_tsc(cd, d);
> +    p2m_get_hostp2m(cd)->max_mapped_pfn = p2m_get_hostp2m(d)->max_mapped_pfn;
>
>      return rc;
>  }
> --
> 2.25.1
>

CC-ing Ian as 4.15 release manager. This patch is safe to include in
4.15 as it's a minor fix in a tech preview feature that's not even
compiled by default.

Thanks,
Tamas

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Tamas K Lengyel]

> > When creating a VM fork copy the parent VM's hostp2m max_mapped_pfn value. Some
> > toolstack relies on the XENMEM_maximum_gpfn value to establish the maximum
> > addressable physical memory in the VM and for forks that have not yet been
> > unpaused that value is not going to reflect the correct max gpfn that's
> > possible to populate into the p2m. This patch fixes the issue.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> > ---
> >  xen/arch/x86/mm/mem_sharing.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
> > index 00ada05c10..98b14f7b0a 100644
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1761,6 +1761,7 @@ static int copy_settings(struct domain *cd, struct domain *d)
> >          return rc;
> >
> >      copy_tsc(cd, d);
> > +    p2m_get_hostp2m(cd)->max_mapped_pfn = p2m_get_hostp2m(d)->max_mapped_pfn;
> >
> >      return rc;
> >  }
> > --
> > 2.25.1
> >
>
> CC-ing Ian as 4.15 release manager. This patch is safe to include in
> 4.15 as it's a minor fix in a tech preview feature that's not even
> compiled by default.

Patch ping just to not forget that I would like this included in the
4.15 release.

Thanks,
Tamas

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Ian Jackson]

Tamas K Lengyel writes ("Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking"):
> On Thu, Mar 18, 2021 at 5:36 PM Tamas K Lengyel <tamas.lengyel@intel.com> wrote:
> >
> > When creating a VM fork copy the parent VM's hostp2m max_mapped_pfn value. Some
> > toolstack relies on the XENMEM_maximum_gpfn value to establish the maximum
> > addressable physical memory in the VM and for forks that have not yet been
> > unpaused that value is not going to reflect the correct max gpfn that's
> > possible to populate into the p2m. This patch fixes the issue.
...
> CC-ing Ian as 4.15 release manager. This patch is safe to include in
> 4.15 as it's a minor fix in a tech preview feature that's not even
> compiled by default.

As far as I can tell this patch is lacking a maintainer review ?

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

*Provided that it is committed today*  I'm not sure how likely that is.

Thanks,
Ian.

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Tamas K Lengyel]

On Fri, Mar 26, 2021 at 7:37 AM Ian Jackson <iwj@xenproject.org> wrote:
>
> Tamas K Lengyel writes ("Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking"):
> > On Thu, Mar 18, 2021 at 5:36 PM Tamas K Lengyel <tamas.lengyel@intel.com> wrote:
> > >
> > > When creating a VM fork copy the parent VM's hostp2m max_mapped_pfn value. Some
> > > toolstack relies on the XENMEM_maximum_gpfn value to establish the maximum
> > > addressable physical memory in the VM and for forks that have not yet been
> > > unpaused that value is not going to reflect the correct max gpfn that's
> > > possible to populate into the p2m. This patch fixes the issue.
> ...
> > CC-ing Ian as 4.15 release manager. This patch is safe to include in
> > 4.15 as it's a minor fix in a tech preview feature that's not even
> > compiled by default.
>
> As far as I can tell this patch is lacking a maintainer review ?
>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>
>
> *Provided that it is committed today*  I'm not sure how likely that is.

Thanks, as I'm the sole maintainer of the code AFAIU it just needs a
Reviewed-by from someone in the community.

Tamas

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Ian Jackson]

Tamas K Lengyel writes ("Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking"):
> On Fri, Mar 26, 2021 at 7:37 AM Ian Jackson <iwj@xenproject.org> wrote:
> > Release-Acked-by: Ian Jackson <iwj@xenproject.org>
> >
> > *Provided that it is committed today*  I'm not sure how likely that is.
> 
> Thanks, as I'm the sole maintainer of the code AFAIU it just needs a
> Reviewed-by from someone in the community.

I don't feel qualified myself, unfortunately.

But your argument in favour of having this in 4.15 seems very strong
to me so I definitely hope someone is able to do the review.

Ian.

Re: [PATCH for-4.15] x86/mem_sharing: copy parent VM's hostp2m's max_mapped_pfn during forking

[Andrew Cooper]

On 18/03/2021 21:36, Tamas K Lengyel wrote:
> When creating a VM fork copy the parent VM's hostp2m max_mapped_pfn value. Some
> toolstack relies on the XENMEM_maximum_gpfn value to establish the maximum
> addressable physical memory in the VM and for forks that have not yet been
> unpaused that value is not going to reflect the correct max gpfn that's
> possible to populate into the p2m. This patch fixes the issue.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>