Message ID | 20200418173035.8000-1-haiyue.wang@intel.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E393DA0598; Sat, 18 Apr 2020 19:36:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3A2131D560; Sat, 18 Apr 2020 19:36:10 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 5F8CC1D52F for <dev@dpdk.org>; Sat, 18 Apr 2020 19:36:09 +0200 (CEST) IronPort-SDR: i7rsaNxEHkNvtqq3E0pJgEjD3P7owaHpxR8qKVWvBFw9wfUhA2VaovePvNp6H/sCO067sVSTPR q7sLKj59NiXA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Apr 2020 10:36:08 -0700 IronPort-SDR: h1PXNfhGMWWFSQqxgWVn7XM6235chNDNNG4q/mz33VvqagMxcK5nDCQSxQFdlIcG4fYo4R5/Uq XLor3haNLGuQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,399,1580803200"; d="scan'208";a="455066802" Received: from npg-dpdk-haiyue-3.sh.intel.com ([10.67.119.46]) by fmsmga005.fm.intel.com with ESMTP; 18 Apr 2020 10:36:06 -0700 From: Haiyue Wang <haiyue.wang@intel.com> To: dev@dpdk.org, anatoly.burakov@intel.com, thomas@monjalon.net, vattunuru@marvell.com, jerinj@marvell.com, alex.williamson@redhat.com, david.marchand@redhat.com Cc: Haiyue Wang <haiyue.wang@intel.com> Date: Sun, 19 Apr 2020 01:30:33 +0800 Message-Id: <20200418173035.8000-1-haiyue.wang@intel.com> X-Mailer: git-send-email 2.26.1 In-Reply-To: <20200305043311.17065-1-vattunuru@marvell.com> References: <20200305043311.17065-1-vattunuru@marvell.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v8 0/2] support for VFIO-PCI VF token interface X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
support for VFIO-PCI VF token interface
|
|
Message
Wang, Haiyue
April 18, 2020, 5:30 p.m. UTC
v8: Update the document. v7: Add the Fixes tag in uuid, the release note and help document. https://patchwork.dpdk.org/cover/68845/ v6: Drop the Fixes tag in uuid, since the file has been moved to another place, not suitable to apply on stable. And this is not a bug, just some kind of enhancement. https://patchwork.dpdk.org/cover/68367/ v5: 1. Add the VF token parse error handling. 2. Split into two patches for different logic module. 3. Add more comments into the code for explaining the design. 4. Drop the ABI change workaround, this patch set focuses on code review. https://patchwork.dpdk.org/cover/68364/ v4: 1. Ignore rte_vfio_setup_device ABI check since it is for Linux driver use. https://patchwork.dpdk.org/patch/68255/ v3: Fix the Travis build failed: (1). rte_uuid.h:97:55: error: unknown type name ‘size_t’ (2). rte_uuid.h:58:2: error: implicit declaration of function ‘memcpy’ https://patchwork.dpdk.org/patch/68254/ v2: Fix the FreeBSD build error. https://patchwork.dpdk.org/patch/68240/ v1: Update the commit message. https://patchwork.dpdk.org/patch/68237/ RFC v2: https://patchwork.dpdk.org/patch/68114/ Based on Vamsi's RFC v1, and Alex's patch for Qemu [https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home/]: Use the devarg to pass-down the VF token. RFC v1: https://patchwork.dpdk.org/patch/66281/ by Vamsi. Haiyue Wang (2): eal: add uuid dependent header files explicitly eal: support for VFIO-PCI VF token doc/guides/linux_gsg/linux_drivers.rst | 38 ++++++++++++- doc/guides/rel_notes/release_20_05.rst | 5 ++ drivers/bus/pci/linux/pci_vfio.c | 74 +++++++++++++++++++++++++- lib/librte_eal/freebsd/eal.c | 3 +- lib/librte_eal/include/rte_uuid.h | 2 + lib/librte_eal/include/rte_vfio.h | 21 +++++++- lib/librte_eal/linux/eal_vfio.c | 20 +++++-- 7 files changed, 155 insertions(+), 8 deletions(-)
Comments
Hello, On Sat, Apr 18, 2020 at 7:36 PM Haiyue Wang <haiyue.wang@intel.com> wrote: > > v8: Update the document. Thanks for the last version. I had a look at the CI, I can see we are still missing bits to handle the ABI failure on rte_vfio_setup_device.
> -----Original Message----- > From: David Marchand <david.marchand@redhat.com> > Sent: Tuesday, April 21, 2020 00:53 > To: Wang, Haiyue <haiyue.wang@intel.com> > Cc: dev <dev@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com>; Thomas Monjalon > <thomas@monjalon.net>; Vamsi Attunuru <vattunuru@marvell.com>; Jerin Jacob Kollanukkaran > <jerinj@marvell.com>; Alex Williamson <alex.williamson@redhat.com> > Subject: Re: [PATCH v8 0/2] support for VFIO-PCI VF token interface > > Hello, > > On Sat, Apr 18, 2020 at 7:36 PM Haiyue Wang <haiyue.wang@intel.com> wrote: > > > > v8: Update the document. > > Thanks for the last version. > I had a look at the CI, I can see we are still missing bits to handle > the ABI failure on rte_vfio_setup_device. Yes, not handle it now. If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal is the best way to handle ABI issue. > > > -- > David Marchand
20/04/2020 19:02, Wang, Haiyue: > From: David Marchand <david.marchand@redhat.com> > > > > Hello, > > > > On Sat, Apr 18, 2020 at 7:36 PM Haiyue Wang <haiyue.wang@intel.com> wrote: > > > > > > v8: Update the document. > > > > Thanks for the last version. > > I had a look at the CI, I can see we are still missing bits to handle > > the ABI failure on rte_vfio_setup_device. > > Yes, not handle it now. > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > is the best way to handle ABI issue. Please could you help finishing integration of __rte_internal? This patch is blocked for now.
+Neil, > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, April 21, 2020 01:13 > To: Wang, Haiyue <haiyue.wang@intel.com> > Cc: David Marchand <david.marchand@redhat.com>; dev <dev@dpdk.org>; Burakov, Anatoly > <anatoly.burakov@intel.com>; Vamsi Attunuru <vattunuru@marvell.com>; Jerin Jacob Kollanukkaran > <jerinj@marvell.com>; Alex Williamson <alex.williamson@redhat.com> > Subject: Re: [PATCH v8 0/2] support for VFIO-PCI VF token interface > > 20/04/2020 19:02, Wang, Haiyue: > > From: David Marchand <david.marchand@redhat.com> > > > > > > Hello, > > > > > > On Sat, Apr 18, 2020 at 7:36 PM Haiyue Wang <haiyue.wang@intel.com> wrote: > > > > > > > > v8: Update the document. > > > > > > Thanks for the last version. > > > I had a look at the CI, I can see we are still missing bits to handle > > > the ABI failure on rte_vfio_setup_device. > > > > Yes, not handle it now. > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > is the best way to handle ABI issue. > > Please could you help finishing integration of __rte_internal? > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ > This patch is blocked for now. >
20/04/2020 19:37, Wang, Haiyue: > From: Thomas Monjalon <thomas@monjalon.net> > > 20/04/2020 19:02, Wang, Haiyue: > > > From: David Marchand <david.marchand@redhat.com> > > > > I had a look at the CI, I can see we are still missing bits to handle > > > > the ABI failure on rte_vfio_setup_device. > > > > > > Yes, not handle it now. > > > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > > is the best way to handle ABI issue. > > > > Please could you help finishing integration of __rte_internal? > > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) > http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ It did not happen after several months. If you want to unblock your patches, I think it is safer to finish yourself.
Hi Thomas & David, > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, April 21, 2020 01:43 > To: Wang, Haiyue <haiyue.wang@intel.com> > Cc: Neil Horman <nhorman@tuxdriver.com>; David Marchand <david.marchand@redhat.com>; dev > <dev@dpdk.org>; Burakov, Anatoly <anatoly.burakov@intel.com>; Vamsi Attunuru <vattunuru@marvell.com>; > Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Alex Williamson <alex.williamson@redhat.com> > Subject: Re: [PATCH v8 0/2] support for VFIO-PCI VF token interface > > 20/04/2020 19:37, Wang, Haiyue: > > From: Thomas Monjalon <thomas@monjalon.net> > > > 20/04/2020 19:02, Wang, Haiyue: > > > > From: David Marchand <david.marchand@redhat.com> > > > > > I had a look at the CI, I can see we are still missing bits to handle > > > > > the ABI failure on rte_vfio_setup_device. > > > > > > > > Yes, not handle it now. > > > > > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > > > is the best way to handle ABI issue. > > > > > > Please could you help finishing integration of __rte_internal? > > > > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) > > http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ > > It did not happen after several months. > If you want to unblock your patches, I think it is safer to finish yourself. > Unfortunately, this __rte_internal will still make the ci fail to run when move the function to INTERNAL session: --- a/devtools/libabigail.abignore +++ b/devtools/libabigail.abignore @@ -3,6 +3,11 @@ [suppress_variable] symbol_version = EXPERIMENTAL +[suppress_function] + symbol_version = INTERNAL +[suppress_variable] + symbol_version = INTERNAL + --- a/lib/librte_eal/rte_eal_version.map +++ b/lib/librte_eal/rte_eal_version.map @@ -213,7 +213,6 @@ DPDK_20.0 { rte_vfio_is_enabled; rte_vfio_noiommu_is_enabled; rte_vfio_release_device; - rte_vfio_setup_device; rte_vlog; rte_zmalloc; rte_zmalloc_socket; @@ -339,3 +338,10 @@ EXPERIMENTAL { # added in 20.05 rte_log_can_log; }; + +INTERNAL { + global: + + # added in 20.05 + rte_vfio_setup_device; +}; Functions changes summary: 1 Removed, 0 Changed, 0 Added function Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 Removed function: [D] 'function int rte_vfio_setup_device(const char*, const char*, int*, vfio_device_info*)' {rte_vfio_setup_device@@DPDK_20.0} Error: ABI issue reported for 'abidiff --suppr ./devtools/libabigail.abignore --no-added-syms --headers-dir1 old_abi/include --headers-dir2 new_abi/include old_abi/dump/librte_eal.dump new_abi/dump/librte_eal.dump'
21/04/2020 03:38, Wang, Haiyue: > From: Thomas Monjalon <thomas@monjalon.net> > > 20/04/2020 19:37, Wang, Haiyue: > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > 20/04/2020 19:02, Wang, Haiyue: > > > > > From: David Marchand <david.marchand@redhat.com> > > > > > > I had a look at the CI, I can see we are still missing bits to handle > > > > > > the ABI failure on rte_vfio_setup_device. > > > > > > > > > > Yes, not handle it now. > > > > > > > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > > > > is the best way to handle ABI issue. > > > > > > > > Please could you help finishing integration of __rte_internal? > > > > > > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) > > > http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ > > > > It did not happen after several months. > > If you want to unblock your patches, I think it is safer to finish yourself. > > > > Unfortunately, this __rte_internal will still make the ci fail to run when move the function > to INTERNAL session: [...] > +INTERNAL { > + global: > + > + # added in 20.05 > + rte_vfio_setup_device; > +}; Why do you need to move the symbol explicitly in .map? The tool should ignore symbols moving to internal, as an exception until 20.11.
> -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, April 21, 2020 10:13 > To: David Marchand <david.marchand@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com> > Cc: Neil Horman <nhorman@tuxdriver.com>; dev <dev@dpdk.org>; Burakov, Anatoly > <anatoly.burakov@intel.com>; Vamsi Attunuru <vattunuru@marvell.com>; Jerin Jacob Kollanukkaran > <jerinj@marvell.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Kinsella, Ray <ray.kinsella@intel.com> > Subject: Re: [PATCH v8 0/2] support for VFIO-PCI VF token interface > > 21/04/2020 03:38, Wang, Haiyue: > > From: Thomas Monjalon <thomas@monjalon.net> > > > 20/04/2020 19:37, Wang, Haiyue: > > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > > 20/04/2020 19:02, Wang, Haiyue: > > > > > > From: David Marchand <david.marchand@redhat.com> > > > > > > > I had a look at the CI, I can see we are still missing bits to handle > > > > > > > the ABI failure on rte_vfio_setup_device. > > > > > > > > > > > > Yes, not handle it now. > > > > > > > > > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > > > > > is the best way to handle ABI issue. > > > > > > > > > > Please could you help finishing integration of __rte_internal? > > > > > > > > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) > > > > http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ > > > > > > It did not happen after several months. > > > If you want to unblock your patches, I think it is safer to finish yourself. > > > > > > > Unfortunately, this __rte_internal will still make the ci fail to run when move the function > > to INTERNAL session: > [...] > > +INTERNAL { > > + global: > > + > > + # added in 20.05 > > + rte_vfio_setup_device; > > +}; > > Why do you need to move the symbol explicitly in .map? > The tool should ignore symbols moving to internal, as an exception until 20.11. > If not move the symbol explicitly in .map, another kind of error happened. ./devtools/check-abi.sh old_abi new_abi Functions changes summary: 0 Removed, 1 Changed, 0 Added function Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 function with some indirect sub-type change: [C] 'function int rte_vfio_setup_device(const char*, const char*, int*, vfio_device_info*)' at eal_vfio.c:704:1 has some indirect sub-type changes: parameter 5 of type 'unsigned char*' was added Error: ABI issue reported for 'abidiff --suppr ./devtools/libabigail.abignore --no-added-syms --headers-dir1 old_abi/include --headers-dir2 new_abi/include old_abi/dump/librte_eal.dump new_abi/dump/librte_eal.dump'
21/04/2020 04:52, Wang, Haiyue: > From: Thomas Monjalon <thomas@monjalon.net> > > 21/04/2020 03:38, Wang, Haiyue: > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > 20/04/2020 19:37, Wang, Haiyue: > > > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > > > 20/04/2020 19:02, Wang, Haiyue: > > > > > > > From: David Marchand <david.marchand@redhat.com> > > > > > > > > I had a look at the CI, I can see we are still missing bits to handle > > > > > > > > the ABI failure on rte_vfio_setup_device. > > > > > > > > > > > > > > Yes, not handle it now. > > > > > > > > > > > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > > > > > > is the best way to handle ABI issue. > > > > > > > > > > > > Please could you help finishing integration of __rte_internal? > > > > > > > > > > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) > > > > > http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ > > > > > > > > It did not happen after several months. > > > > If you want to unblock your patches, I think it is safer to finish yourself. > > > > > > > > > > Unfortunately, this __rte_internal will still make the ci fail to run when move the function > > > to INTERNAL session: > > [...] > > > +INTERNAL { > > > + global: > > > + > > > + # added in 20.05 > > > + rte_vfio_setup_device; > > > +}; > > > > Why do you need to move the symbol explicitly in .map? > > The tool should ignore symbols moving to internal, as an exception until 20.11. > > If not move the symbol explicitly in .map, another kind of error happened. > > ./devtools/check-abi.sh old_abi new_abi > Functions changes summary: 0 Removed, 1 Changed, 0 Added function > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable > > 1 function with some indirect sub-type change: > > [C] 'function int rte_vfio_setup_device(const char*, const char*, int*, vfio_device_info*)' at eal_vfio.c:704:1 has some indirect sub-type changes: > parameter 5 of type 'unsigned char*' was added > > Error: ABI issue reported for 'abidiff --suppr ./devtools/libabigail.abignore --no-added-syms --headers-dir1 old_abi/include --headers-dir2 new_abi/include old_abi/dump/librte_eal.dump new_abi/dump/librte_eal.dump' This is what I said: you need to add a rule to ignore internal symbols.
> -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, April 21, 2020 16:48 > To: David Marchand <david.marchand@redhat.com>; Wang, Haiyue <haiyue.wang@intel.com> > Cc: Neil Horman <nhorman@tuxdriver.com>; dev <dev@dpdk.org>; Burakov, Anatoly > <anatoly.burakov@intel.com>; Vamsi Attunuru <vattunuru@marvell.com>; Jerin Jacob Kollanukkaran > <jerinj@marvell.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Kinsella, Ray <ray.kinsella@intel.com> > Subject: Re: [PATCH v8 0/2] support for VFIO-PCI VF token interface > > 21/04/2020 04:52, Wang, Haiyue: > > From: Thomas Monjalon <thomas@monjalon.net> > > > 21/04/2020 03:38, Wang, Haiyue: > > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > > 20/04/2020 19:37, Wang, Haiyue: > > > > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > > > > 20/04/2020 19:02, Wang, Haiyue: > > > > > > > > From: David Marchand <david.marchand@redhat.com> > > > > > > > > > I had a look at the CI, I can see we are still missing bits to handle > > > > > > > > > the ABI failure on rte_vfio_setup_device. > > > > > > > > > > > > > > > > Yes, not handle it now. > > > > > > > > > > > > > > > > If 'rte_vfio_setup_device' can be internal, not official DPDK API, then __rte_internal > > > > > > > > is the best way to handle ABI issue. > > > > > > > > > > > > > > Please could you help finishing integration of __rte_internal? > > > > > > > > > > > > I thought it should be Neil ? "Yes, I'll get back to this today" ;-) > > > > > > > http://inbox.dpdk.org/dev/CAJFAV8ydLkV0afEHqbh6KeA3Box0Yxb3N0MNGtMD4S9bmSgT0A@mail.gmail.com/ > > > > > > > > > > It did not happen after several months. > > > > > If you want to unblock your patches, I think it is safer to finish yourself. > > > > > > > > > > > > > Unfortunately, this __rte_internal will still make the ci fail to run when move the function > > > > to INTERNAL session: > > > [...] > > > > +INTERNAL { > > > > + global: > > > > + > > > > + # added in 20.05 > > > > + rte_vfio_setup_device; > > > > +}; > > > > > > Why do you need to move the symbol explicitly in .map? > > > The tool should ignore symbols moving to internal, as an exception until 20.11. > > > > If not move the symbol explicitly in .map, another kind of error happened. > > > > ./devtools/check-abi.sh old_abi new_abi > > Functions changes summary: 0 Removed, 1 Changed, 0 Added function > > Variables changes summary: 0 Removed, 0 Changed, 0 Added variable > > > > 1 function with some indirect sub-type change: > > > > [C] 'function int rte_vfio_setup_device(const char*, const char*, int*, vfio_device_info*)' at > eal_vfio.c:704:1 has some indirect sub-type changes: > > parameter 5 of type 'unsigned char*' was added > > > > Error: ABI issue reported for 'abidiff --suppr ./devtools/libabigail.abignore --no-added-syms -- > headers-dir1 old_abi/include --headers-dir2 new_abi/include old_abi/dump/librte_eal.dump > new_abi/dump/librte_eal.dump' > > This is what I said: you need to add a rule to ignore internal symbols. > Got the kind reply from Dodji, the maintainer of libabigail, his suggestion meets what we have done for 'EXPERIMENTAL'. So it seems that have to mark all needed functions as INTERNAL firstly, which obviously will break the CI as the public functions are removed.... " __PROJECT_INTERNAL_USE_ONLY_VERSION__ { global: funA }; ... Then, once you have all your internal functions marked in the ELF binary with the proper ELF version string, you can tell libabigail to suppress all functions that have that version by writing a suppression specification file that has this content: [suppress_function] symbol_version = __PROJECT_INTERNAL_USE_ONLY_VERSION__ "