From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH] ioreq-server: handle IOREQ_TYPE_PCI_CONFIG in assist function Date: Wed, 28 Jan 2015 19:27:41 -0500 Message-ID: <54C97E7D.8070103@terremark.com> References: <1422385589-17316-1-git-send-email-wei.liu2@citrix.com> <9AAE0902D5BC7E449B7C8E4E778ABCD0257D8FA3@AMSPEX01CL01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD0257D8FA3@AMSPEX01CL01.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Paul Durrant , Wei Liu , "xen-devel@lists.xen.org" Cc: Andrew Cooper , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 01/28/15 04:22, Paul Durrant wrote: >> -----Original Message----- >> From: Wei Liu [mailto:wei.liu2@citrix.com] >> Sent: 27 January 2015 19:06 >> To: xen-devel@lists.xen.org >> Cc: Wei Liu; Jan Beulich; Paul Durrant; Andrew Cooper >> Subject: [PATCH] ioreq-server: handle IOREQ_TYPE_PCI_CONFIG in assist >> function >> >> QEMU stubdom will read PCI config space when enumerating PCI devices. >> Xen should return ~0 when there is no suitable ioreq server to dispatch >> the request. >> >> Without this patch, QEMU stubdom will fail to start because hvmloader >> fails following assertion: >> >> 118 ASSERT((devfn != PCI_ISA_DEVFN) || >> 119 ((vendor_id == 0x8086) && (device_id == 0x7000))); >> >> because vendor_id and device_id are 0. >> >> This fixes a regression for QEMU stubdom. It should be backported to 4.5 >> as well. >> >> Signed-off-by: Wei Liu >> Cc: Jan Beulich >> Cc: Paul Durrant >> Cc: Andrew Cooper > > No, you should not need this. If you look at the code closely you'll see that > hvm_complete_assist_req() is only called if hvm_select_ioreq_server() returns NULL, > and the switch from IOREQ_TYPE_PIO to IOREQ_TYPE_PCI_CONFIG is only made if a > non-default ioreq server matches the current cf8. Hence we could > actually ASSERT(p->type !- IOREQ_TYPE_PCI_CONFIG) at the top > of hvm_complete_assist_req(). > I will agree that the ASSERT looks good to me with s/!-/!=/. I have no idea how Wei Liu makes it to this code via QEMU stubdom (last I knew QEMU stubdom needs QEMU traditional not QEMU upstream. And the only way to get IOREQ_TYPE_PCI_CONFIG is with QEMU upstream). I have found out how hvmloader ends up using hvm_complete_assist_req() when not expected. Not sure how this is related to Wei Liu's issue. -Don Slutz > Paul > >> --- >> xen/arch/x86/hvm/hvm.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index c7984d1..c826ac5 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -2577,6 +2577,7 @@ static bool_t hvm_complete_assist_req(ioreq_t *p) >> { >> case IOREQ_TYPE_COPY: >> case IOREQ_TYPE_PIO: >> + case IOREQ_TYPE_PCI_CONFIG: >> if ( p->dir == IOREQ_READ ) >> { >> if ( !p->data_is_ptr ) >> -- >> 1.9.1 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >