Mailing List Archive: [PATCH 1/2] x86/mem_sharing: copy cpuid during vm forking

[PATCH 1/2] x86/mem_sharing: copy cpuid during vm forking

Jan 4, 2021, 9:41 AM

Post #1 of 4 (5 views)

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 c428fd16ce..4a02c7d6f2 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1764,6 +1764,7 @@ static int fork(struct domain *cd, struct domain *d)

domain_pause(d);
cd->max_pages = d->max_pages;
+ memcpy(cd->arch.cpuid, d->arch.cpuid, sizeof(*d->arch.cpuid));
cd->parent = d;
}

--
2.25.1

Re: [PATCH 1/2] x86/mem_sharing: copy cpuid during vm forking [ In reply to ]

jbeulich at suse

Jan 5, 2021, 12:40 AM

Post #2 of 4 (5 views)

Permalink

On 04.01.2021 18:41, Tamas K Lengyel wrote:
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1764,6 +1764,7 @@ static int fork(struct domain *cd, struct domain *d)
>
> domain_pause(d);
> cd->max_pages = d->max_pages;
> + memcpy(cd->arch.cpuid, d->arch.cpuid, sizeof(*d->arch.cpuid));

Can such copying please be done using assignment rather than
memcpy(), for the added type safety? (I wouldn't mind doing
this while committing, so long as you don't mind.)

Jan

Re: [PATCH 1/2] x86/mem_sharing: copy cpuid during vm forking [ In reply to ]

andrew.cooper3 at citrix

Jan 5, 2021, 3:04 AM

Post #3 of 4 (5 views)

Permalink

On 04/01/2021 17:41, Tamas K Lengyel wrote:
> 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 c428fd16ce..4a02c7d6f2 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -1764,6 +1764,7 @@ static int fork(struct domain *cd, struct domain *d)
>
> domain_pause(d);
> cd->max_pages = d->max_pages;
> + memcpy(cd->arch.cpuid, d->arch.cpuid, sizeof(*d->arch.cpuid));
> cd->parent = d;
> }
>

You need to extend this to d->arch.msr and v->arch.msrs or someone is
going to have a very subtle bug to debug in the future.

~Andrew

RE: [PATCH 1/2] x86/mem_sharing: copy cpuid during vm forking [ In reply to ]

tamas.lengyel at intel

Jan 5, 2021, 7:50 AM

Post #4 of 4 (5 views)

Permalink

> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: Tuesday, January 5, 2021 6:05 AM
> To: Lengyel, Tamas <tamas.lengyel@intel.com>; xen-
> devel@lists.xenproject.org
> Cc: Tamas K Lengyel <tamas@tklengyel.com>; Jan Beulich
> <jbeulich@suse.com>; George Dunlap <george.dunlap@citrix.com>; Roger
> Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH 1/2] x86/mem_sharing: copy cpuid during vm forking
>
> On 04/01/2021 17:41, Tamas K Lengyel wrote:
> > 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 c428fd16ce..4a02c7d6f2
> 100644
> > --- a/xen/arch/x86/mm/mem_sharing.c
> > +++ b/xen/arch/x86/mm/mem_sharing.c
> > @@ -1764,6 +1764,7 @@ static int fork(struct domain *cd, struct domain
> > *d)
> >
> > domain_pause(d);
> > cd->max_pages = d->max_pages;
> > + memcpy(cd->arch.cpuid, d->arch.cpuid,
> > + sizeof(*d->arch.cpuid));
> > cd->parent = d;
> > }
> >
>
> You need to extend this to d->arch.msr and v->arch.msrs or someone is
> going to have a very subtle bug to debug in the future.

I need more information why v->arch.msrs would need to be copied manually. If it's saved/reloaded by hvm_save/hvm_load then we are already covered. If not, then why would we need to do that for forks but not for domain save/restore?

Tamas