Re: [dpdk-dev] virtio PMD is not working with master version

[Santosh Shukla <sshukla@mvista.com>]

On Fri, Feb 26, 2016 at 2:14 PM, Xie, Huawei <huawei.xie@intel.com> wrote:
> On 2/26/2016 4:29 PM, David Marchand wrote:
>> On Fri, Feb 26, 2016 at 3:23 AM, Yuanhan Liu
>> <yuanhan.liu@linux.intel.com> wrote:
>>> Mauricio, thanks for the testing and report.
>>>
>>> On Thu, Feb 25, 2016 at 02:30:18PM +0100, David Marchand wrote:
>>>> >From the logs, I would say I broke uio_pci_generic since we are in
>>>> "uio" case, but uio portio sysfs does not exist.
>>>> virtio pmd fell back to ioports discovery before my change.
>>> Maybe we can do same?
> We shouldn't, :). I am now rebasing the patch to fix the issue that
> virtio driver takes the virtio device blindly.
> With the patch:
>  if driver is VFIO/UIO, and errors happens, returns without falling back
> to IO port.

Nice, This will be useful for non-x86 arch case, IO port is NA for
non-x86 arch so falling back to IO port would always fail. so
defaulting to IO port case is incorrect. IMO, not arch agnostic.

>  if no any kernel driver is managing the device, try IO port; otherwise
> returns 1 to tell the layer we don't take over this device.
>


>> I suppose, but see below.
>>
>>> ---
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
>>> index 4346973..579731c 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>>> @@ -685,12 +685,11 @@ int
>>>  rte_eal_pci_ioport_map(struct rte_pci_device *dev, int bar,
>>>                        struct rte_pci_ioport *p)
>>>  {
>>> -       int ret;
>>> +       int ret = -1;
>>>
>>>         switch (dev->kdrv) {
>>>  #ifdef VFIO_PRESENT
>>>         case RTE_KDRV_VFIO:
>>> -               ret = -1;
>>>                 if (pci_vfio_is_enabled())
>>>                         ret = pci_vfio_ioport_map(dev, bar, p);
>>>                 break;
>>> @@ -700,14 +699,14 @@ rte_eal_pci_ioport_map(struct rte_pci_device *dev, int bar,
>>>                 ret = pci_uio_ioport_map(dev, bar, p);
>>>                 break;
>>>         default:
>>> +               break;
>>> +       }
>>> +
>>>  #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
>>> -               /* special case for x86 ... */
>>> +       /* special case for x86 ... */
>>> +       if (ret)
>>>                 ret = pci_ioport_map(dev, bar, p);
>>> -#else
>>> -               ret = -1;
>>>  #endif
>>> -               break;
>>> -       }
>> What if we are supposed to do vfio here, but for some reason init failed ?
>> Next thing, we will call ioport_read in vfio context, but init went
>> through the ioports parsing => boom ?
>>
>> Another issue is that when device is bound to a kernel driver (let's
>> say virtio-pci here), then init will succeed and pmd will kick in the
>> device registers.
>>
>> This special case should really be narrowed down to "uio" and "none"
>> driver cases.
>>
>>
>