From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH] x86/hvm: Fix HVM guest regression introduced by c58ba78c84 Date: Wed, 04 Feb 2015 17:01:26 -0500 Message-ID: <54D296B6.5050205@terremark.com> References: <1423053312-23666-1-git-send-email-andrew.cooper3@citrix.com> <54D2227C020000780005CC1B@mail.emea.novell.com> <54D216F5.1040907@citrix.com> <54D22616020000780005CC41@mail.emea.novell.com> <54D2187D.3040503@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54D2187D.3040503@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Jan Beulich Cc: Xen-devel List-Id: xen-devel@lists.xenproject.org On 02/04/15 08:02, Andrew Cooper wrote: > On 04/02/15 13:00, Jan Beulich wrote: >>>>> On 04.02.15 at 13:56, wrote: >>> Something between d0b2caa..c58ba78 has broken HVM guests to point at >>> which HVMloader doesn't reliably function. >> And no crash (with register state dumped) or any other hint as to >> what's going wrong? > > PV guests are all fine, and mixed PV/HVM tests have the PV side complete > successfully. > With the info provided, it looks a lot like I have found: (copied here, dropped duplication): -------- Forwarded Message -------- Subject: Re: [Qemu-devel] [PATCH v5 2/2] Xen: Use the ioreq-server API when available Date: Wed, 28 Jan 2015 19:57:42 -0500 From: Don Slutz To: Don Slutz , Paul Durrant , qemu-devel@nongnu.org, Stefano Stabellini CC: Peter Maydell , Olaf Hering , Alexey Kardashevskiy , Stefan Weil , Michael Tokarev , Alexander Graf , Gerd Hoffmann , Stefan Hajnoczi , Paolo Bonzini On 01/28/15 19:05, Don Slutz wrote: > On 01/28/15 14:32, Don Slutz wrote: >> On 12/05/14 05:50, Paul Durrant wrote: >>> The ioreq-server API added to Xen 4.5 offers better security than >>> the existing Xen/QEMU interface because the shared pages that are >>> used to pass emulation request/results back and forth are removed >>> from the guest's memory space before any requests are serviced. >>> This prevents the guest from mapping these pages (they are in a >>> well known location) and attempting to attack QEMU by synthesizing >>> its own request structures. Hence, this patch modifies configure >>> to detect whether the API is available, and adds the necessary >>> code to use the API if it is. >> >> This patch (which is now on xenbits qemu staging) is causing me >> issues. >> > > I have found the key. > > The following will reproduce my issue: > > 1) xl create -p > 2) read one of HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN, or > HVM_PARAM_BUFIOREQ_EVTCHN > 3) xl unpause new guest > > The guest will hang in hvmloader. > ... Using QEMU upstream master (or xenbits qemu staging), you do not have a default ioreq server. And so hvm_select_ioreq_server() returns NULL for hvmloader's iorequest to: CPU4 0 (+ 0) HANDLE_PIO [ port = 0x0cfe size = 2 dir = 1 ] (I added this xentrace to figure out what is happening, and I have a lot of data about it, if any one wants it.) To get a guest hang instead of calling hvm_complete_assist_req() for some of hvmloader's pci_read() calls, you can do the following: 1) xl create -p 2) read one of HVM_PARAM_IOREQ_PFN, HVM_PARAM_BUFIOREQ_PFN, or HVM_PARAM_BUFIOREQ_EVTCHN 3) xl unpause new guest The guest will hang in hvmloader. The read of HVM_PARAM_IOREQ_PFN will cause a default ioreq server to be created and directed to the QEMU upsteam that is not a default ioreq server. This read also creates the extra event channels that I see. I see that hvmop_create_ioreq_server() prevents you from creating an is_default ioreq_server, so QEMU is not able to do. Not sure where we go from here. -Don Slutz Using the debug key e, you can see extra unbound event channels. I had proposed the 1 line fix: diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index bad410e..7ac4b45 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -993,7 +993,7 @@ static int hvm_create_ioreq_server(struct domain *d, domid_t domid, spin_lock(&d->arch.hvm_domain.ioreq_server.lock); rc = -EEXIST; - if ( is_default && d->arch.hvm_domain.default_ioreq_server != NULL ) + if ( is_default && !list_empty(&d->arch.hvm_domain.ioreq_server.list) ) goto fail2; rc = hvm_ioreq_server_init(s, d, domid, is_default, handle_bufioreq, But no idea if this is right or way off base. -Don Slutz > ~Andrew > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >