On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla <sshukla at mvista.com> wrote:
> On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu at 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 at
>>> 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;
}
}
>
>> --yliu
