Re: [dpdk-dev] [PATCH] eal: map io resources for non x86 architectures

[Santosh Shukla <sshukla@mvista.com>]

On Thu, Dec 17, 2015 at 4:03 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2015-12-17 15:51, Santosh Shukla:
>> On Thu, Dec 17, 2015 at 3:44 PM, Thomas Monjalon
>> <thomas.monjalon@6wind.com> wrote:
>> > Hi,
>> >
>> > 2015-12-17 15:37, Santosh Shukla:
>> >> On Thu, Dec 17, 2015 at 3:32 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> >> > On Thu, Dec 17, 2015 at 3:31 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> >> >> On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
>> >> >> <yuanhan.liu@linux.intel.com> wrote:
>> >> >>> On Wed, Dec 16, 2015 at 07:21:55PM +0530, Santosh Shukla wrote:
>> >> >>>> On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
>> >> >>>> <yuanhan.liu@linux.intel.com> wrote:
>> >> >>>> > On Wed, Dec 16, 2015 at 01:31:04PM +0100, David Marchand wrote:
>> >> >>>> >> x86 requires a special set of instructions to access ioports, but other
>> >> >>>> >> architectures let you remap io resources.
>> >> >>>> >> So let eal remap io resources by accepting IORESOURCE_IO flag for
>> >> >>>> >> architectures other than x86.
>> >> >>>> >
>> >> >>>> > One question: this patch could be a replacement of the igbuio_iomap patch
>> >> >>>> > from Santosh? If so, I like it: It's more elegant.
>> >> >>>> >
>> >> >>>> >         --yliu
>> >> >>>> >
>> >> >>>>
>> >> >>>> I did tried similar in past but not in parse_sysfs (such that
>> >> >>>> mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
>> >> >>>> pci_map_resource not able to map address hence segfault at tespmd
>> >> >>>> initialization.
>> >> >>>>
>> >> >>>> i was getting these:
>> >> >>>> EAL: pci_map_resource(): cannot mmap(19, 0x7fa5c00000, 0x20, 0x0):
>> >> >>>> Invalid argument (0xffffffffffffffff)
>> >> >>>
>> >> >>> That's because ARM (at least the kernel) doesn't allow an IO map:
>> >> >>>
>> >> >>> arch/arm/kernel/bios32.c
>> >> >>> ------------------------
>> >> >>> 618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
>> >> >>> 619                         enum pci_mmap_state mmap_state, int write_combine)
>> >> >>> 620 {
>> >> >>> 621         if (mmap_state == pci_mmap_io)
>> >> >>> 622                 return -EINVAL;
>> >> >>>
>> >> >>> And with a quick glimpse of powerpc, I see no such limitation. Hence,
>> >> >>> this peice of code may work only on Powerpc platform (and maybe a few
>> >> >>> others we don't care).
>> >> >>>
>> >> >>> So, apparently, this will not work for ARM.
>> >> >>>
>> >> >>
>> >> >> Right and I did shared detailed explanation on why it wont work on
>> >> >> this link [1], infact this patch shouldn;t work for mips too.
>> >> >>
>> >> >> As I mentioned earlier I did tried similar approach and so to get
>> >> >> everything working like iomem is currently in dpdk; we need to add
>> >> >> something like pci_remap_iospace --> ioremap_page_range() but this api
>> >> >> not really pci_mmap_page_range types. user need to write more code on
>> >> >> top so to use this api efficiently, also this api looks like meant to
>> >> >> use by arch file only in kernel space.
>> >> >>
>> >> >>
>> >> > missed link;
>> >> >
>> >> > [1] http://dpdk.org/dev/patchwork/patch/9365/
>> >> >
>> >>
>> >> IMO, it is worth keeping one special device file who could work across
>> >> archs like arm/arm64/powerpc and others, who could map iopci bar to
>> >> dpdk user-space. also this approach has no kernel version dependency
>> >> too. BTW; I did mentioned in second approach in to add /dev/ioport
>> >> interface in drivers/char/mem.c which could read more than byte in one
>> >> single operation, but that has kernel dependency. However that
>> >> approach too is arch agnostic.
>> >
>> > Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
>> > really say there is no kernel dependency.
>>
>> Agree but I mentioned kernel __version__ dependency.
>
> Yes you did.
> One of the main issue with out-of-tree kernel modules is the version
> dependency. Probably that igb_uio from DPDK 2.3 will not compile with
> the kernel 5.0.
>

don't know kernel 5.0 feature list so I guess your may be right. is
uio obsoleted for 5.0 kernel?

>> > We should try to remove the need for any out-of-tree kernel module.
>> > That's why the Linux upstream approach is a better solution.
>>
>> IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
>> pci_mmap_page_range()?
>
> I don't know what is the best solution in the kernel.
> First we need to be sure that there is absolutely no solution without
> kernel changes.

I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -

# either we need to modify kernel
               - Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
            or
               - include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel.  I could send an RFC to lkml.

OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.

Thanks.

> Then we can try a pci_mmap solution or, as you suggest, an interface in
> drivers/char/mem.c
>