Re: [Xen-devel] [PATCH] ioreq-server: handle IOREQ_TYPE_PCI_CONFIG in assist function

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jan 28, 2015 at 07:27:41PM -0500, Don Slutz wrote:
> On 01/28/15 04:22, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> >> Sent: 27 January 2015 19:06
> >> To: xen-devel@xxxxxxxxxxxxx
> >> 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 <wei.liu2@xxxxxxxxxx>
> >> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> >> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
> >> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > 
> > 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.
> 

This patch is now useless because the problem I discovered is a race
between stubdom and guest. It will take some time to fix.

Wei.

>    -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@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> > 

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