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

[Don Slutz <dslutz@verizon.com>]

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 <wei.liu2@citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Paul Durrant <paul.durrant@citrix.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> 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
>