DPDK patches and discussions
 help / color / mirror / Atom feed
From: Santosh Shukla <sshukla@mvista.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region
Date: Wed, 3 Feb 2016 15:20:09 +0530	[thread overview]
Message-ID: <CAAyOgsaWSGTrmZCE0FmhasfSv6TXbk+fkn7c2g6W1Y+dLQA_sg@mail.gmail.com> (raw)
In-Reply-To: <CAAyOgsbrMTbTmNRC3Rs-Jcq6JdFquup1a_GGZYPSuTdm1c_jbQ@mail.gmail.com>

On Tue, Feb 2, 2016 at 9:48 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>>> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
>>>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>>>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>>>> >> Current use-case is virtio: It is used as io_bar which is first
>>>> >> bar[1]. But implementation is generic, can be used to do rd/wr for
>>>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>>>> >> w/o mapping that bar, So apis will be useful for such cases in future.
>>>> >>
>>>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>>>> >> then no need to do rd/wr, user can directly access the pci_memory. But
>>>> >> use-case of this api entirely different: unmapped memory by
>>>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>>>> >>
>>>> >> Is above explanation convincing? Pl. let me know.
>>>> >
>>>> > TBH, not really. So, as you stated, it should be generic APIs to
>>>> > read/write bar space, but limiting it to VFIO only and claiming
>>>> > that read/write bar space is not support by other drivers (such
>>>> > as UIO) while in fact it can (in some ways) doesn't seem right
>>>> > to me.
>>>> >
>>>> > Anyway, it's just some thoughts from me. David, comments?
>>>>
>>>> >From the very start, same opinion.
>>>> We should have a unique api to access those, and eal should hide
>>>> details like kernel drivers (uio, vfio, whatever) to the pmd.
>>>>
>>>> Now the thing is, how to do this in an elegant and efficient way.
>>>
>>> I was thinking that we may just make it be IO port specific read/
>>> write functions:
>>>
>>
>> Ok,
>>
>>>         rte_eal_pci_ioport_read(dev, bar, buf, size)
>>>         {
>>>
>>>                 return if not an IO bar;
>>>
>>>                 if (has io)
>>>                         return inb/w/l();
>>>
>>
>> In that case, It may be r / if (has io) / if (drv->kdrv == UIO)
>>
>>>                 if (vfio)
>>>                         return vfio_ioport_read();
>>>
>>>                 else, claim aloud that io port read is not allowed
>>>         }
>>>
>>> Let us not handle memory bar resource here: in such case, you should
>>> go with rte_eal_pci_map_device() and do it with memory mapped io.
>>>
>>> Does that make any sense?
>>>
>> I am not entirely sure.
>> Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ?
>>
>
> Just came-up something below what Yuanhan has proposed, Does this look okay?
>
> int rte_eal_pci_ioport_read(const struct rte_pci_device *device,
>                                               void *buf, size_t len,
> off_t offset,
>                                               int bar_idx)
> {
>       if (bar_idx != 0) {
>               RTE_LOG(ERR, EAL, "not a ioport bar\n");
>               return -1;
>        }
>
>      switch (device->kdrv) {
>      case RTE_KDRV_VFIO:
>                return pci_vfio_ioport_read(device, buf, len, offset, bar_idx);
>      case RTE_KDRV_IGB_UIO:
>      case RTE_KDRV_UIO_GENERIC:
>      case RTE_KDRV_NIC_UIO:
>          {
>               switch (size)
>                       case 1: return inb(buf /*ioport address*/);
>                       case 2: return inw(buf /* ioport address*/);
>                       case 4: return inl(buf /* ioport address*/);
>                       default:
>                              RTE_LOG(ERR, EAL, "invalid size\n");
>           }
>
>        default:
>                  RTE_LOG(ERR, EAL, "read bar not supported by driver\n");
>                  return -1;
>       }
> }
>

Ping?

Also can someone please review rest of series. This patchset going
through multiple revision, Each revision get one / two comment, It
would help if I get review comment for each patch.

>>
>>>         --yliu

  reply	other threads:[~2016-02-03  9:50 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <0000-cover-letter.patch>
2016-01-29 18:21 ` Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 2/8] linuxapp/vfio: ignore mapping for ioport region Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 3/8] eal/linux: never check iopl for arm Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 4/8] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 5/8] virtio: move io header and api from virtio_pci.h Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 6/8] virtio: add vfio api to rd/wr ioport space Santosh Shukla
2016-02-01 12:48     ` Yuanhan Liu
2016-02-02  4:30       ` Santosh Shukla
2016-02-02  5:19         ` Yuanhan Liu
2016-02-02  6:02           ` Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 7/8] virtio: extend pci rw api for vfio Santosh Shukla
2016-01-29 18:21   ` [dpdk-dev] [PATCH v6 8/8] virtio: do not parse if interface is vfio Santosh Shukla
2016-02-01 13:48   ` [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region Yuanhan Liu
2016-02-02  4:14     ` Santosh Shukla
2016-02-02  5:43       ` Yuanhan Liu
2016-02-02  5:50         ` David Marchand
2016-02-02  8:49           ` Yuanhan Liu
2016-02-02 15:51             ` Santosh Shukla
2016-02-02 16:18               ` Santosh Shukla
2016-02-03  9:50                 ` Santosh Shukla [this message]
2016-02-03 11:50                   ` Yuanhan Liu
2016-02-05 17:56                     ` David Marchand
2016-02-03 11:43                 ` Yuanhan Liu
2016-02-02  7:00         ` Santosh Shukla
2016-02-02  7:01           ` Santosh Shukla
2016-02-08 10:03 ` [dpdk-dev] [PATCH v8 3/4] eal/linux: vfio: ignore mapping for ioport region Santosh Shukla
2016-02-08 10:03   ` [dpdk-dev] [PATCH v8 4/4] eal/linux: vfio: add pci ioport support Santosh Shukla
2016-02-08 14:13     ` Burakov, Anatoly
2016-02-09  9:04       ` David Marchand
2016-02-18  5:25         ` Santosh Shukla
2016-02-18 14:00           ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAAyOgsaWSGTrmZCE0FmhasfSv6TXbk+fkn7c2g6W1Y+dLQA_sg@mail.gmail.com \
    --to=sshukla@mvista.com \
    --cc=dev@dpdk.org \
    --cc=yuanhan.liu@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).