From mboxrd@z Thu Jan  1 00:00:00 1970
From: Don Slutz <don.slutz@gmail.com>
Subject: Migration bug added by commit
	2df1aa01bef7366798248ac6d03cfb42048b003d
Date: Sat, 27 Jun 2015 17:02:13 -0400
Message-ID: <558F0F55.7030908@Gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: xen-devel@lists.xen.org, Paul Durrant <paul.durrant@citrix.com>, Jan Beulich <jbeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

commit 2df1aa01bef7366798248ac6d03cfb42048b003d
Author: Paul Durrant <paul.durrant@citrix.com>
Date:   Tue Jun 23 18:07:49 2015 +0200

     x86/hvm: remove hvm_io_pending() check in hvmemul_do_io()

...
-            rc = X86EMUL_RETRY;
-            if ( !hvm_send_assist_req(s, &p) )
+            rc = hvm_send_assist_req(s, &p);
+            if ( rc != X86EMUL_RETRY )
...
      if ( unlikely(!vcpu_start_shutdown_deferral(curr)) )
-        return 0; /* implicitly bins the i/o operation */
+        return X86EMUL_OKAY;

So now X86EMUL_OKAY is returned from hvmemul_do_io() during shutdown.

 From Jan Beulich about this:


  Re: [Xen-devel] [PATCH 3/5] hvmemul_do_io: If the send to the ioreq
  server failed do not retry.
  <https://www.mail-archive.com/search?l=xen-devel@lists.xen.org&q=subject:%22Re%5C%3A+%5C%5BXen%5C-devel%5C%5D+%5C%5BPATCH+3%5C%2F5%5C%5D+hvmemul_do_io%5C%3A+If+the+send+to+the+ioreq+server+failed+do+not+retry.%22&o=newest>


Jan Beulich 
<https://www.mail-archive.com/search?l=xen-devel@lists.xen.org&q=from:%22Jan+Beulich%22> 
Fri, 30 Jan 2015 02:24:55 -0800 
<https://www.mail-archive.com/search?l=xen-devel@lists.xen.org&q=date:20150130> 


>>> On 30.01.15 at 01:52, <dsl...@verizon.com> wrote:
> I.E. do just what no backing DM does.


_If_ this is correct, the if() modified here should be folded with the
one a few lines up. But looking at the description of the commit that
introduced this (bac0999325 "x86 hvm: Do not incorrectly retire an
instruction emulation...", almost immediately modified by f20f3c8ece
"x86 hvm: On failed hvm_send_assist_req(), io emulation...") I doubt
this is really what we want, or at the very least your change
description should explain what was wrong with the original commit.

Jan



going up the call stack:

in handle_pio when hvmemul_do_pio_buffer() returns X86EMUL_OKAY, it 
returns 1.

    svm_vmexit_handler 2578 if ( handle_pio(port, bytes, dir) )
or
    vmx_vmexit_handler 3178 if ( handle_pio(port, bytes, dir) )

both update the IP in this case which is wrong during shutdown.

So I think:

             rc = hvm_send_assist_req(s, &p);
             if ( rc != X86EMUL_RETRY )
                  vio->io_state = HVMIO_none;

needs to change to:

                 rc = hvm_send_assist_req(s, &p);
                 if ( rc != X86EMUL_RETRY )
                 {
                     vio->io_state = HVMIO_none;
                     if ( rc == X86EMUL_OKAY )
                         rc = X86EMUL_RETRY;
                 }


    -Don Slutz