ioreq server regression

ioreq server regression

[Boris Ostrovsky]

Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
default ioreq server") breaks restore for HVM guests.

I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.

-boris


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

Re: ioreq server regression

[Jan Beulich]

>>> On 12.12.16 at 17:01, <boris.ostrovsky@oracle.com> wrote:
> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
> default ioreq server") breaks restore for HVM guests.
> 
> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.

And that's qemu-trad or qemu-upstream?

Jan


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

Re: ioreq server regression

[Paul Durrant]

> -----Original Message-----
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: 12 December 2016 16:02
> To: Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich
> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
> Juergen Gross <jgross@suse.com>; xen-devel <xen-devel@lists.xen.org>
> Cc: zhangchen.fnst@cn.fujitsu.com
> Subject: ioreq server regression
> 
> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
> default ioreq server") breaks restore for HVM guests.
> 
> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
> 

Damn, yes that will be the case when migrating with legacy QEMU... we're going to need another flag that's set on the domain by the restore code so that the default server also gets created on restore.

  Paul

> -boris

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

Re: ioreq server regression

[Andrew Cooper]

On 12/12/16 16:22, Paul Durrant wrote:
>> -----Original Message-----
>> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
>> Sent: 12 December 2016 16:02
>> To: Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich
>> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
>> Juergen Gross <jgross@suse.com>; xen-devel <xen-devel@lists.xen.org>
>> Cc: zhangchen.fnst@cn.fujitsu.com
>> Subject: ioreq server regression
>>
>> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
>> default ioreq server") breaks restore for HVM guests.
>>
>> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
>>
> Damn, yes that will be the case when migrating with legacy QEMU... we're going to need another flag that's set on the domain by the restore code so that the default server also gets created on restore.

I don't understand why the existing code doesn't suffice.  A domain
being restored is conceptually no different from one which is still
being constructed.

The creation_finished flag should still be clear at this point during
restore.  Is it perhaps something else doing a pause/unpause on the
domain before qemu starts up?  The creation_finished flag should only be
set at the first point the refcount goes to zero, not the first unpause
hypercall.

~Andrew

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

Re: ioreq server regression

[Boris Ostrovsky]

On 12/12/2016 11:21 AM, Jan Beulich wrote:
>>>> On 12.12.16 at 17:01, <boris.ostrovsky@oracle.com> wrote:
>> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
>> default ioreq server") breaks restore for HVM guests.
>>
>> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
> And that's qemu-trad or qemu-upstream?

Traditional. Which explains now why my 32-bit guest (which uses upstream
qemu) worked fine.

-boris

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

Re: ioreq server regression

[Paul Durrant]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 12 December 2016 16:27
> To: Paul Durrant <Paul.Durrant@citrix.com>; Boris Ostrovsky
> <boris.ostrovsky@oracle.com>; Jan Beulich <jbeulich@suse.com>; Juergen
> Gross <jgross@suse.com>; xen-devel <xen-devel@lists.xen.org>
> Cc: zhangchen.fnst@cn.fujitsu.com
> Subject: Re: ioreq server regression
> 
> On 12/12/16 16:22, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> >> Sent: 12 December 2016 16:02
> >> To: Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich
> >> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
> >> Juergen Gross <jgross@suse.com>; xen-devel <xen-devel@lists.xen.org>
> >> Cc: zhangchen.fnst@cn.fujitsu.com
> >> Subject: ioreq server regression
> >>
> >> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
> >> default ioreq server") breaks restore for HVM guests.
> >>
> >> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
> >>
> > Damn, yes that will be the case when migrating with legacy QEMU... we're
> going to need another flag that's set on the domain by the restore code so
> that the default server also gets created on restore.
> 
> I don't understand why the existing code doesn't suffice.  A domain
> being restored is conceptually no different from one which is still
> being constructed.
> 
> The creation_finished flag should still be clear at this point during
> restore.

That's the problem... The creation flag is clear so reading the HVM params does not instantiate the default ioreq server in the target domain.

  Paul

>  Is it perhaps something else doing a pause/unpause on the
> domain before qemu starts up?  The creation_finished flag should only be
> set at the first point the refcount goes to zero, not the first unpause
> hypercall.
> 
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: ioreq server regression

[Paul Durrant]

> -----Original Message-----
> From: Paul Durrant
> Sent: 12 December 2016 16:29
> To: Andrew Cooper <Andrew.Cooper3@citrix.com>; Boris Ostrovsky
> <boris.ostrovsky@oracle.com>; Jan Beulich <jbeulich@suse.com>; Juergen
> Gross <jgross@suse.com>; xen-devel <xen-devel@lists.xen.org>
> Cc: zhangchen.fnst@cn.fujitsu.com
> Subject: RE: ioreq server regression
> 
> > -----Original Message-----
> > From: Andrew Cooper
> > Sent: 12 December 2016 16:27
> > To: Paul Durrant <Paul.Durrant@citrix.com>; Boris Ostrovsky
> > <boris.ostrovsky@oracle.com>; Jan Beulich <jbeulich@suse.com>; Juergen
> > Gross <jgross@suse.com>; xen-devel <xen-devel@lists.xen.org>
> > Cc: zhangchen.fnst@cn.fujitsu.com
> > Subject: Re: ioreq server regression
> >
> > On 12/12/16 16:22, Paul Durrant wrote:
> > >> -----Original Message-----
> > >> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> > >> Sent: 12 December 2016 16:02
> > >> To: Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich
> > >> <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>;
> > >> Juergen Gross <jgross@suse.com>; xen-devel <xen-
> devel@lists.xen.org>
> > >> Cc: zhangchen.fnst@cn.fujitsu.com
> > >> Subject: ioreq server regression
> > >>
> > >> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
> > >> default ioreq server") breaks restore for HVM guests.
> > >>
> > >> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
> > >>
> > > Damn, yes that will be the case when migrating with legacy QEMU...
> we're
> > going to need another flag that's set on the domain by the restore code so
> > that the default server also gets created on restore.
> >
> > I don't understand why the existing code doesn't suffice.  A domain
> > being restored is conceptually no different from one which is still
> > being constructed.
> >
> > The creation_finished flag should still be clear at this point during
> > restore.
> 
> That's the problem... The creation flag is clear so reading the HVM params
> does not instantiate the default ioreq server in the target domain.
> 

Sorry, ignore me... I my logic backwards.

I guess the creation flag must get set too early on restore.

  Paul

>   Paul
> 
> >  Is it perhaps something else doing a pause/unpause on the
> > domain before qemu starts up?  The creation_finished flag should only be
> > set at the first point the refcount goes to zero, not the first unpause
> > hypercall.
> >
> > ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: ioreq server regression

[Andrew Cooper]

On 12/12/16 16:01, Boris Ostrovsky wrote:
> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
> default ioreq server") breaks restore for HVM guests.
>
> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
>
> -boris
>

Does this help?  The current creation_finished logic is wrong if a 2nd
toolstack component performs a pause/unpause pair on the domain.

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 3abaca9..6043f89 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,14 @@ int domain_unpause_by_systemcontroller(struct
domain *d)
         prev = cmpxchg(&d->controller_pause_count, old, new);
     } while ( prev != old );
 
+    /*
+     * 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.
+     */
+    if ( prev == 0 )
+        d->creation_finished = true;
+
     domain_unpause(d);
 
     return 0;


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

Re: ioreq server regression

[Boris Ostrovsky]

On 12/12/2016 11:36 AM, Andrew Cooper wrote:
> On 12/12/16 16:01, Boris Ostrovsky wrote:
>> Looks like commit e7dabe59c ("x86/hvm: don't unconditionally create a
>> default ioreq server") breaks restore for HVM guests.
>>
>> I see "qemu: hardware error: Invalid ioreq type 0x53" in qemu log.
>>
>> -boris
>>
> Does this help?  The current creation_finished logic is wrong if a 2nd
> toolstack component performs a pause/unpause pair on the domain.

Yes, this seems to work.

-boris

>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 3abaca9..6043f89 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,14 @@ int domain_unpause_by_systemcontroller(struct
> domain *d)
>          prev = cmpxchg(&d->controller_pause_count, old, new);
>      } while ( prev != old );
>  
> +    /*
> +     * 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.
> +     */
> +    if ( prev == 0 )
> +        d->creation_finished = true;
> +
>      domain_unpause(d);
>  
>      return 0;
>



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