Re: [Xen-devel] [PATCH 2/2] xen: Fix determining when domain creation is complete

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 12 December 2016 18:30
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Boris
> Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Subject: [PATCH 2/2] xen: Fix determining when domain creation is complete
> 
> d->creation_finished is used in several places alter behaviour depending on
> whether the domain is being created, or is already running.
> 
> However, there is a latent bug if a toolstack component makes a pair of
> pause/unpause calls, where creation will be considered finished
> prematurely.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>

I tested this on initial boot and restore.

Tested-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> 
> This is probably a backport candidate, as creation_finished was introduced in
> 4.8
> ---
>  xen/common/domain.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 3abaca9..05130e2 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1004,13 +1004,6 @@ int domain_unpause_by_systemcontroller(struct
> domain *d)
>  {
>      int old, new, prev = d->controller_pause_count;
> 
> -    /*
> -     * We record this information here for populate_physmap to figure out
> -     * that the domain has finished being created. In fact, we're only
> -     * allowed to set the MEMF_no_tlbflush flag during VM creation.
> -     */
> -    d->creation_finished = true;
> -
>      do
>      {
>          old = prev;
> @@ -1022,6 +1015,20 @@ int domain_unpause_by_systemcontroller(struct
> domain *d)
>          prev = cmpxchg(&d->controller_pause_count, old, new);
>      } while ( prev != old );
> 
> +    /*
> +     * d->controller_pause_count is initialised to 1, and the toolstack is
> +     * responsible for making one unpause hypercall when it wishes the guest
> +     * to start running.
> +     *
> +     * All other toolstack operations should make a pair of pause/unpause
> +     * calls and rely on the reference counting here.
> +     *
> +     * Creation is considered finished when the controller reference count
> +     * first drops to 0.
> +     */
> +    if ( new == 0 )
> +        d->creation_finished = true;
> +
>      domain_unpause(d);
> 
>      return 0;
> --
> 2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel