From: Olivier Matz <olivier.matz@6wind.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>,
David Marchand <david.marchand@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
Chao Zhu <chaozhu@linux.vnet.ibm.com>,
"Xie, Huawei" <huawei.xie@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 5/7] eal/linux: mmap ioports on ppc64
Date: Mon, 23 May 2016 15:40:58 +0200 [thread overview]
Message-ID: <5743086A.2020103@6wind.com> (raw)
In-Reply-To: <20160523130749.GJ5641@yliu-dev.sh.intel.com>
Hi Yuanhan,
On 05/23/2016 03:07 PM, Yuanhan Liu wrote:
> On Tue, May 17, 2016 at 05:54:01PM +0200, David Marchand wrote:
>>> +pci_uio_ioport_map(struct rte_pci_device *dev, int bar,
>>> + struct rte_pci_ioport *p)
>>> +{
>>> + FILE *f;
>>> + char buf[BUFSIZ];
>>> + char filename[PATH_MAX];
>>> + uint64_t phys_addr, end_addr, flags;
>>> + int fd, i;
>>> + void *addr;
>>> +
>>> + /* open and read addresses of the corresponding resource in sysfs */
>>> + snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT "/resource",
>>> + SYSFS_PCI_DEVICES, dev->addr.domain, dev->addr.bus,
>>> + dev->addr.devid, dev->addr.function);
>>> + f = fopen(filename, "r");
>>> + if (f == NULL) {
>>> + RTE_LOG(ERR, EAL, "Cannot open sysfs resource: %s\n",
>>> + strerror(errno));
>>> + return -1;
>>> + }
>>> + for (i = 0; i < bar + 1; i++) {
>>> + if (fgets(buf, sizeof(buf), f) == NULL) {
>>> + RTE_LOG(ERR, EAL, "Cannot read sysfs resource\n");
>>> + goto error;
>>> + }
>>> + }
>>> + if (pci_parse_one_sysfs_resource(buf, sizeof(buf), &phys_addr,
>>> + &end_addr, &flags) < 0)
>>> + goto error;
>>> + if ((flags & IORESOURCE_IO) == 0) {
>>> + RTE_LOG(ERR, EAL, "BAR %d is not an IO resource\n", bar);
>>> + goto error;
>>> + }
>>> + snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT "/resource%d",
>>> + SYSFS_PCI_DEVICES, dev->addr.domain, dev->addr.bus,
>>> + dev->addr.devid, dev->addr.function, bar);
>>> +
>>> + /* mmap the pci resource */
>>> + fd = open(filename, O_RDWR);
>>> + if (fd < 0) {
>>> + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", filename,
>>> + strerror(errno));
>>> + goto error;
>>> + }
>>> + addr = mmap(NULL, end_addr + 1, PROT_READ | PROT_WRITE,
>>> + MAP_SHARED, fd, 0);
>>
>> Sorry, did not catch it in v1, but a close(fd) is missing here.
>> With this, I think the patchset looks good.
>>
>> Just missing some opinion from the virtio maintainers ?
>
> Apologize for being late for review. Assuming you have done proper
> test, this patch set looks good to me. (well, I don't quite like
> the tons of "#ifdef ... #else ..#end" block though)
>
> A side note is that I noticed an ABI breakage introduced in this
> patch, so, this release is not a good fit?
Thank you for the review.
For reference, here is the report of the ABI checker for EAL:
[−] struct rte_pci_ioport (2)
1 Field len has been added to this type.
1) This field will not be initialized by old clients.
2) Size of the inclusive type has been changed.
NOTE: this field should be accessed only from the new library
functions, otherwise it may result in crash or incorrect behavior
of applications.
2 Size of this type has been changed from 16 bytes to 24 bytes.
The fields or parameters of such data type may be incorrectly
initialized or accessed by old client applications.
[−] affected symbols (4)
rte_eal_pci_ioport_map ( struct rte_pci_device* dev, int bar,
struct rte_pci_ioport* p ) @@ DPDK_16.04
3rd parameter 'p' (pointer) has base type 'struct rte_pci_ioport'.
rte_eal_pci_ioport_read ( struct rte_pci_ioport* p, void* data,
size_t len, off_t offset ) @@ DPDK_16.04
1st parameter 'p' (pointer) has base type 'struct rte_pci_ioport'.
rte_eal_pci_ioport_unmap ( struct rte_pci_ioport* p ) @@ DPDK_16.04
1st parameter 'p' (pointer) has base type 'struct rte_pci_ioport'.
rte_eal_pci_ioport_write ( struct rte_pci_ioport* p, void const* data,
size_t len, off_t offset ) @@ DPDK_16.04
1st parameter 'p' (pointer) has base type 'struct rte_pci_ioport'.
My understanding of the comment for this structure is that it's
internal to EAL:
/**
* A structure used to access io resources for a pci device.
* rte_pci_ioport is arch, os, driver specific, and should not be used
outside
* of pci ioport api.
*/
struct rte_pci_ioport {
...
}
So I'd say it's ok to have it integrated for 16.07.
Regards,
Olivier
next prev parent reply other threads:[~2016-05-23 13:41 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-13 12:50 [dpdk-dev] [PATCH 0/7] virtio-net support " Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 1/7] eal: fix typos in ioport API doxygen comments Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 2/7] eal/linux: only call iopl on x86 Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 3/7] eal/linux: remove invalid comment Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 4/7] eal/linux: split function parsing pci resources in sysfs Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 5/7] eal/linux: mmap ioports on ppc64 Olivier Matz
2016-05-13 14:33 ` David Marchand
2016-05-13 16:36 ` Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 6/7] virtio: fix pci accesses for ppc64 in legacy mode Olivier Matz
2016-05-13 14:28 ` David Marchand
2016-05-13 16:34 ` Olivier Matz
2016-05-13 12:50 ` [dpdk-dev] [PATCH 7/7] config: enable virtio-net pmd for ppc64 Olivier Matz
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 0/7] virtio-net support on ppc64 Olivier Matz
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 1/7] eal: fix typos in ioport API doxygen comments Olivier Matz
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 2/7] eal/linux: only call iopl on x86 Olivier Matz
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 3/7] eal/linux: remove invalid comment Olivier Matz
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 4/7] eal/linux: split function parsing pci resources in sysfs Olivier Matz
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 5/7] eal/linux: mmap ioports on ppc64 Olivier Matz
2016-05-17 15:54 ` David Marchand
2016-05-18 11:17 ` Olivier Matz
2016-05-23 13:07 ` Yuanhan Liu
2016-05-23 13:40 ` Olivier Matz [this message]
2016-05-24 5:15 ` Yuanhan Liu
2016-05-30 8:45 ` Olivier Matz
2016-06-15 16:13 ` Thomas Monjalon
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 6/7] virtio: fix pci accesses for ppc64 in legacy mode Olivier Matz
2016-05-19 9:13 ` Chao Zhu
2016-05-20 12:11 ` Olivier Matz
2016-05-20 12:18 ` Olivier Matz
2016-05-24 6:28 ` Chao Zhu
2016-05-17 9:59 ` [dpdk-dev] [PATCH v2 7/7] config: enable virtio-net pmd for ppc64 Olivier Matz
2016-06-15 17:08 ` [dpdk-dev] [PATCH v2 0/7] virtio-net support on ppc64 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=5743086A.2020103@6wind.com \
--to=olivier.matz@6wind.com \
--cc=chaozhu@linux.vnet.ibm.com \
--cc=david.marchand@6wind.com \
--cc=dev@dpdk.org \
--cc=huawei.xie@intel.com \
--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).