[dpdk-dev,v3,06/12] eal: pci: vfio: add rd/wr func for pci bar space
Commit Message
Introducing below api for pci bar space rd/wr. Currently used for
pci iobar rd/wr.
Api's are:
- rte_eal_pci_read_bar
- rte_eal_pci_write_bar
virtio when used for vfio-mode then virtio driver will use these api
to do rd/wr operation on ioport pci bar.
Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
lib/librte_eal/common/include/rte_pci.h | 38 ++++++++++++++++++++++++++++
lib/librte_eal/linuxapp/eal/eal_pci.c | 28 ++++++++++++++++++++
lib/librte_eal/linuxapp/eal/eal_pci_init.h | 6 +++++
lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 28 ++++++++++++++++++++
4 files changed, 100 insertions(+)
Comments
On Thu, 7 Jan 2016 22:03:03 +0530
Santosh Shukla <sshukla@mvista.com> wrote:
>
> +int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
> + void *buf __rte_unused,
> + size_t len __rte_unused,
> + off_t offset __rte_unused,
> + int bar_idx __rte_unused)
> +{
> +#ifdef VFIO_PRESENT
> + const struct rte_intr_handle *intr_handle = &device->intr_handle;
> + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> +#else
> + return 0; /* UIO's not applicable */
> +#endif
> +}
It seems wrong to declare all the parameters as unused but then use them.
Maybe there is a way to have a macro for USED(x) in the #else case
On 1/7/16, 12:19 PM, "dev on behalf of Stephen Hemminger" <dev-bounces@dpdk.org on behalf of stephen@networkplumber.org> wrote:
>On Thu, 7 Jan 2016 22:03:03 +0530
>Santosh Shukla <sshukla@mvista.com> wrote:
>
>>
>> +int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
>> + void *buf __rte_unused,
>> + size_t len __rte_unused,
>> + off_t offset __rte_unused,
>> + int bar_idx __rte_unused)
>> +{
>> +#ifdef VFIO_PRESENT
>> + const struct rte_intr_handle *intr_handle = &device->intr_handle;
>> + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
>> +#else
>> + return 0; /* UIO's not applicable */
>> +#endif
>> +}
>
>It seems wrong to declare all the parameters as unused but then use them.
>Maybe there is a way to have a macro for USED(x) in the #else case
I would suggest we create a macro '#define RTE_UNUSED(x) ((void)x)’, unless we have one and I missed it groping though the code.
>
Regards,
Keith
On Thu, 7 Jan 2016 18:26:30 +0000
"Wiles, Keith" <keith.wiles@intel.com> wrote:
> On 1/7/16, 12:19 PM, "dev on behalf of Stephen Hemminger" <dev-bounces@dpdk.org on behalf of stephen@networkplumber.org> wrote:
>
> >On Thu, 7 Jan 2016 22:03:03 +0530
> >Santosh Shukla <sshukla@mvista.com> wrote:
> >
> >>
> >> +int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
> >> + void *buf __rte_unused,
> >> + size_t len __rte_unused,
> >> + off_t offset __rte_unused,
> >> + int bar_idx __rte_unused)
> >> +{
> >> +#ifdef VFIO_PRESENT
> >> + const struct rte_intr_handle *intr_handle = &device->intr_handle;
> >> + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> >> +#else
> >> + return 0; /* UIO's not applicable */
> >> +#endif
> >> +}
> >
> >It seems wrong to declare all the parameters as unused but then use them.
> >Maybe there is a way to have a macro for USED(x) in the #else case
>
> I would suggest we create a macro '#define RTE_UNUSED(x) ((void)x)’, unless we have one and I missed it groping though the code.
> >
>
>
> Regards,
> Keith
>
>
>
>
Or just move the #ifdef outside the function and have two versions
#ifdef VFIO_PRESENT
int rte_eal_pci_read_bar(const struct rte_pci_device *device,
void *buf, size_t len,
off_t offset, int bar_idx)
{
const struct rte_intr_handle *intr_handle = &device->intr_handle;
return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
}
}
#else
int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
void *buf __rte_unused,
size_t len __rte_unused,
off_t offset __rte_unused,
int bar_idx __rte_unused)
{
return 0; /* UIO's not applicable */
}
#endif
On Thu, Jan 7, 2016 at 11:49 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Thu, 7 Jan 2016 22:03:03 +0530
> Santosh Shukla <sshukla@mvista.com> wrote:
>
> >
> > +int rte_eal_pci_read_bar(const struct rte_pci_device *device
> __rte_unused,
> > + void *buf __rte_unused,
> > + size_t len __rte_unused,
> > + off_t offset __rte_unused,
> > + int bar_idx __rte_unused)
> > +{
> > +#ifdef VFIO_PRESENT
> > + const struct rte_intr_handle *intr_handle = &device->intr_handle;
> > + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> > +#else
> > + return 0; /* UIO's not applicable */
> > +#endif
> > +}
>
> It seems wrong to declare all the parameters as unused but then use them.
> Maybe there is a way to have a macro for USED(x) in the #else case
>
Yes, I followed such practice in v2 series but missed on v3, Sorry for that
and we'll take care in v4.
On Fri, Jan 8, 2016 at 12:09 AM, Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Thu, 7 Jan 2016 18:26:30 +0000
> "Wiles, Keith" <keith.wiles@intel.com> wrote:
>
> > On 1/7/16, 12:19 PM, "dev on behalf of Stephen Hemminger" <
> dev-bounces@dpdk.org on behalf of stephen@networkplumber.org> wrote:
> >
> > >On Thu, 7 Jan 2016 22:03:03 +0530
> > >Santosh Shukla <sshukla@mvista.com> wrote:
> > >
> > >>
> > >> +int rte_eal_pci_read_bar(const struct rte_pci_device *device
> __rte_unused,
> > >> + void *buf __rte_unused,
> > >> + size_t len __rte_unused,
> > >> + off_t offset __rte_unused,
> > >> + int bar_idx __rte_unused)
> > >> +{
> > >> +#ifdef VFIO_PRESENT
> > >> + const struct rte_intr_handle *intr_handle = &device->intr_handle;
> > >> + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> > >> +#else
> > >> + return 0; /* UIO's not applicable */
> > >> +#endif
> > >> +}
> > >
> > >It seems wrong to declare all the parameters as unused but then use
> them.
> > >Maybe there is a way to have a macro for USED(x) in the #else case
> >
> > I would suggest we create a macro '#define RTE_UNUSED(x) ((void)x)’,
> unless we have one and I missed it groping though the code.
> > >
> >
> >
> > Regards,
> > Keith
> >
> >
> >
> >
>
> Or just move the #ifdef outside the function and have two versions
>
> #ifdef VFIO_PRESENT
> int rte_eal_pci_read_bar(const struct rte_pci_device *device,
> void *buf, size_t len,
> off_t offset, int bar_idx)
> {
> const struct rte_intr_handle *intr_handle = &device->intr_handle;
>
> return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> }
> }
> #else
> int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
> void *buf __rte_unused,
> size_t len __rte_unused,
> off_t offset __rte_unused,
> int bar_idx __rte_unused)
> {
> return 0; /* UIO's not applicable */
> }
> #endif
>
I'll pick this one!, Thanks.
On Thu, Jan 07, 2016 at 10:19:25AM -0800, Stephen Hemminger wrote:
> On Thu, 7 Jan 2016 22:03:03 +0530
> Santosh Shukla <sshukla@mvista.com> wrote:
>
> >
> > +int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
> > + void *buf __rte_unused,
> > + size_t len __rte_unused,
> > + off_t offset __rte_unused,
> > + int bar_idx __rte_unused)
> > +{
> > +#ifdef VFIO_PRESENT
> > + const struct rte_intr_handle *intr_handle = &device->intr_handle;
> > + return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> > +#else
> > + return 0; /* UIO's not applicable */
> > +#endif
> > +}
>
> It seems wrong to declare all the parameters as unused but then use them.
> Maybe there is a way to have a macro for USED(x) in the #else case
There is RTE_SET_USED() in rte_common.h.
I'd prefer that macro used than the option of duplicating the entire function
which really pads out the code.
/Bruce
@@ -471,6 +471,44 @@ int rte_eal_pci_read_config(const struct rte_pci_device *device,
void *buf, size_t len, off_t offset);
/**
+ * Read PCI bar space.
+ *
+ * @param device
+ * A pointer to a rte_pci_device structure describing the device
+ * to use
+ * @param buf
+ * A data buffer where the bytes should be read into
+ * @param len
+ * The length of the data buffer.
+ * @param offset
+ * The offset into PCI bar space
+ * @param bar_idx
+ * The pci bar index (valid range is 0..5)
+ */
+int rte_eal_pci_read_bar(const struct rte_pci_device *device,
+ void *buf, size_t len, off_t offset, int bar_idx);
+
+/**
+ * Write PCI bar space.
+ *
+ * @param device
+ * A pointer to a rte_pci_device structure describing the device
+ * to use
+ * @param buf
+ * A data buffer containing the bytes should be written
+ * @param len
+ * The length of the data buffer.
+ * @param offset
+ * The offset into PCI config space
+ * @param bar_idx
+ * The pci bar index (valid range is 0..5)
+*/
+int rte_eal_pci_write_bar(const struct rte_pci_device *device,
+ const void *buf, size_t len, off_t offset,
+ int bar_idx);
+
+
+/**
* Write PCI config space.
*
* @param device
@@ -621,6 +621,34 @@ int rte_eal_pci_write_config(const struct rte_pci_device *device,
}
}
+int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
+ void *buf __rte_unused,
+ size_t len __rte_unused,
+ off_t offset __rte_unused,
+ int bar_idx __rte_unused)
+{
+#ifdef VFIO_PRESENT
+ const struct rte_intr_handle *intr_handle = &device->intr_handle;
+ return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
+#else
+ return 0; /* UIO's not applicable */
+#endif
+}
+
+int rte_eal_pci_write_bar(const struct rte_pci_device *device __rte_unused,
+ const void *buf __rte_unused,
+ size_t len __rte_unused,
+ off_t offset __rte_unused,
+ int bar_idx __rte_unused)
+{
+#ifdef VFIO_PRESENT
+ const struct rte_intr_handle *intr_handle = &device->intr_handle;
+ return pci_vfio_write_bar(intr_handle, buf, len, offset, bar_idx);
+#else
+ return 0; /* UIO's not applicable */
+#endif
+}
+
/* Init the PCI EAL subsystem */
int
rte_eal_pci_init(void)
@@ -68,6 +68,12 @@ int pci_vfio_read_config(const struct rte_intr_handle *intr_handle,
int pci_vfio_write_config(const struct rte_intr_handle *intr_handle,
const void *buf, size_t len, off_t offs);
+int pci_vfio_read_bar(const struct rte_intr_handle *intr_handle,
+ void *buf, size_t len, off_t offs, int bar_idx);
+
+int pci_vfio_write_bar(const struct rte_intr_handle *intr_handle,
+ const void *buf, size_t len, off_t offs, int bar_idx);
+
/* map VFIO resource prototype */
int pci_vfio_map_resource(struct rte_pci_device *dev);
int pci_vfio_get_group_fd(int iommu_group_fd);
@@ -93,6 +93,34 @@ pci_vfio_write_config(const struct rte_intr_handle *intr_handle,
VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) + offs);
}
+int
+pci_vfio_read_bar(const struct rte_intr_handle *intr_handle,
+ void *buf, size_t len, off_t offs, int bar_idx)
+{
+ if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX
+ || bar_idx > VFIO_PCI_BAR5_REGION_INDEX) {
+ RTE_LOG(ERR, EAL, "invalid bar_idx!\n");
+ return -1;
+ }
+
+ return pread64(intr_handle->vfio_dev_fd, buf, len,
+ VFIO_GET_REGION_ADDR(bar_idx) + offs);
+}
+
+int
+pci_vfio_write_bar(const struct rte_intr_handle *intr_handle,
+ const void *buf, size_t len, off_t offs, int bar_idx)
+{
+ if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX
+ || bar_idx > VFIO_PCI_BAR5_REGION_INDEX) {
+ RTE_LOG(ERR, EAL, "invalid bar_idx!\n");
+ return -1;
+ }
+
+ return pwrite64(intr_handle->vfio_dev_fd, buf, len,
+ VFIO_GET_REGION_ADDR(bar_idx) + offs);
+}
+
/* get PCI BAR number where MSI-X interrupts are */
static int
pci_vfio_get_msix_bar(int fd, int *msix_bar, uint32_t *msix_table_offset,