Re: [PATCH] x86/hvm: Fix HVM guest regression introduced by c58ba78c84

[Don Slutz <dslutz@verizon.com>]

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, <andrew.cooper3@citrix.com> 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 <dslutz@verizon.com>
To: Don Slutz <dslutz@verizon.com>, Paul Durrant
<paul.durrant@citrix.com>, qemu-devel@nongnu.org, Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
CC: Peter Maydell <peter.maydell@linaro.org>, Olaf Hering
<olaf@aepfle.de>, Alexey Kardashevskiy <aik@ozlabs.ru>, Stefan Weil
<sw@weilnetz.de>, Michael Tokarev <mjt@tls.msk.ru>, Alexander Graf
<agraf@suse.de>, Gerd Hoffmann <kraxel@redhat.com>, Stefan Hajnoczi
<stefanha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>



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 <config>
> 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 <config>
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
>