All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
	Ben Walker <benjamin.walker@intel.com>,
	"Burakov, Anatoly" <anatoly.burakov@intel.com>
Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] bus/pci: fix IOVA as VA mode selection
Date: Mon, 8 Jul 2019 19:13:28 +0000	[thread overview]
Message-ID: <BYAPR18MB2424D794776AD36FF9FAABADC8F60@BYAPR18MB2424.namprd18.prod.outlook.com> (raw)
In-Reply-To: <CAJFAV8x_Qw6pcc+X1jQuk-LGPkMGosdycu25jjestKWoAi5ArA@mail.gmail.com>

See below,

Please send the email as text to avoid formatting issue.(No HTML)

From: David Marchand <david.marchand@redhat.com> 
Sent: Tuesday, July 9, 2019 12:09 AM
To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Cc: dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>; Ben Walker <benjamin.walker@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
Subject: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: fix IOVA as VA mode selection

________________________________________

On Mon, Jul 8, 2019 at 4:25 PM <mailto:jerinj@marvell.com> wrote:
From: Jerin Jacob <mailto:jerinj@marvell.com>

Existing logic fails to select IOVA mode as VA
if driver request to enable IOVA as VA.

IOVA as VA has more strict requirement than other modes,
so enabling positive logic for IOVA as VA selection.

This patch also updates the default IOVA mode as PA
for PCI devices as it has to deal with DMA engines unlike
the virtual devices that may need only IOVA as DC.

We have three cases:
- driver/hw supports IOVA as PA only

[Jerin] It is not driver cap, it is more of system cap(IOMMU vs non  IOMMU). We are already addressing that case

- driver/hw supports IOVA as both VA and PA

- driver/hw supports IOVA as VA only

[Jerin] We left with only two options, So one bit from driver is enough to advertise the capability.

Looking at the header:
/** Device driver supports IOVA as VA */
#define RTE_PCI_DRV_IOVA_AS_VA 0X0040

[Jerin] I agree, The name can be RTE_PCI_DRV_NEED_IOVA_AS_VA if there is confusion.

It clearly states that the driver supports IOVA as VA, and not requires.
With only one flag, we can't decide on those three cases and we might want to introduce a new flag for your usecase.

[Jerin] See above.

But, in any case, this is too short for -rc1.
I prefer to nak this patch for now.

[Jerin] Anyway the current patch is wrong with existing definition also i.e If drivers support IOVA as VA it is switching to DC?
Then why do we need IOVA_AS_VA flag from driver. Moreover, from PCI device point of view, Setting default to PA is
More appropriate than DC as Typical PCIe devices we deal in DPDK are bus master which deal with PA or VA.

You can still set --iova-mode=va and we can try to find a solution for -rc2.

[Jerin] The automatic scripts etc will fail on that case. More over, I failed to understand the issue with this patch.
I agree that we can update the logic etc in rc2. But why to break the stuff for rc1 for no reason.



-- 
David Marchand

  reply	other threads:[~2019-07-08 19:13 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-08 14:24 [dpdk-dev] [PATCH] bus/pci: fix IOVA as VA mode selection jerinj
2019-07-08 18:39 ` David Marchand
2019-07-08 19:13   ` Jerin Jacob Kollanukkaran [this message]
2019-07-09  8:39     ` [dpdk-dev] [EXT] " Bruce Richardson
2019-07-09  9:05       ` Jerin Jacob Kollanukkaran
2019-07-09  9:32         ` Bruce Richardson
2019-07-09  9:44     ` Burakov, Anatoly
2019-07-09 11:13       ` Jerin Jacob Kollanukkaran
2019-07-09 11:40         ` Burakov, Anatoly
2019-07-09 12:11           ` Jerin Jacob Kollanukkaran
2019-07-09 13:30             ` Burakov, Anatoly
2019-07-09 13:50               ` Burakov, Anatoly
2019-07-09 14:19                 ` Jerin Jacob Kollanukkaran
2019-07-09 14:00               ` Jerin Jacob Kollanukkaran
2019-07-09 14:37                 ` Burakov, Anatoly
2019-07-09 15:04                   ` Thomas Monjalon
2019-07-09 15:06                     ` Burakov, Anatoly
2019-07-09 17:50                   ` Jerin Jacob Kollanukkaran
2019-07-10  8:09                     ` David Marchand
2019-07-09 14:54                 ` Burakov, Anatoly
2019-07-09 14:58                   ` Jerin Jacob Kollanukkaran
2019-07-09 15:02                     ` Burakov, Anatoly
2019-07-09 15:12                       ` Thomas Monjalon
2019-07-09 15:18                         ` Burakov, Anatoly

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=BYAPR18MB2424D794776AD36FF9FAABADC8F60@BYAPR18MB2424.namprd18.prod.outlook.com \
    --to=jerinj@marvell.com \
    --cc=anatoly.burakov@intel.com \
    --cc=benjamin.walker@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.