On 1/21/20 5:49 PM, Tamas K Lengyel wrote:
> VM forking is the process of creating a domain with an empty memory space and
> a
> parent domain specified from which to populate the memory when necessary. For
> the new domain to be functional the VM state is copied over as part of the
> fork
> operation (HVM params, hap allocation, etc).
>
> Signed-off-by: Tamas K Lengyel <tamas.leng...@intel.com>
Overall this looks really good. Just a few questions.
> ---
> xen/arch/x86/domain.c | 9 ++
> xen/arch/x86/hvm/hvm.c | 2 +-
> xen/arch/x86/mm/mem_sharing.c | 220 ++++++++++++++++++++++++++++++
> xen/arch/x86/mm/p2m.c | 11 +-
> xen/include/asm-x86/mem_sharing.h | 20 ++-
> xen/include/public/memory.h | 5 +
> xen/include/xen/sched.h | 2 +
> 7 files changed, 265 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 28fefa1f81..953abcc1fc 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -2198,6 +2198,15 @@ int domain_relinquish_resources(struct domain *d)
> ret = relinquish_shared_pages(d);
> if ( ret )
> return ret;
> +
> + /*
> + * If the domain is forked, decrement the parent's pause count.
> + */
> + if ( d->parent )
> + {
> + domain_unpause(d->parent);
> + d->parent = NULL;
Did this want to be `if ( d->parent_paused )`?
> +static int bring_up_vcpus(struct domain *cd, struct cpupool *cpupool)
> +{
> + int ret;
> + unsigned int i;
> +
> + if ( (ret = cpupool_move_domain(cd, cpupool)) )
> + return ret;
> +
> + for ( i = 0; i < cd->max_vcpus; i++ )
> + {
> + if ( cd->vcpu[i] )
> + continue;
> +
> + if ( !vcpu_create(cd, i) )
> + return -EINVAL;
You're not copying the contents of the vcpu registers or anything here
-- is that something you're leaving to your dom0 agent?
> +static int mem_sharing_fork(struct domain *d, struct domain *cd)
> +{
> + int rc = -EINVAL;
> +
> + if ( !cd->controller_pause_count )
> + return rc;
> +
> + /*
> + * We only want to pause the parent once, not each time this
> + * operation is restarted due to preemption.
> + */
> + if ( !cd->parent_paused )
> + {
> + domain_pause(d);
> + cd->parent_paused = true;
> + }
> +
> + cd->max_pages = d->max_pages;
> + cd->max_vcpus = d->max_vcpus;
> +
> + /* this is preemptible so it's the first to get done */
> + if ( (rc = fork_hap_allocation(d, cd)) )
> + goto done;
> +
> + if ( (rc = bring_up_vcpus(cd, d->cpupool)) )
> + goto done;
> +
> + if ( (rc = hvm_copy_context_and_params(d, cd)) )
> + goto done;
> +
> + fork_tsc(d, cd);
> +
> + cd->parent = d;
What happens if the parent dies?
It seems like we might want to do get_domain(parent) here, and
put_domain(parent) in domain_destroy.
I'll probably need to come back to this, at which point I may have more
questions.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
