linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@intel.com>
To: Edward Cree <ecree@solarflare.com>, Don Dutile <ddutile@redhat.com>
Cc: linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH] PCI: handle pci_sriov_set_totalvfs(dev, 0)
Date: Mon, 04 Aug 2014 07:37:19 -0700	[thread overview]
Message-ID: <53DF9A9F.9060600@intel.com> (raw)
In-Reply-To: <53DF92B4.1040505@solarflare.com>

On 08/04/2014 07:03 AM, Edward Cree wrote:
> On 31/07/14 19:13, Edward Cree wrote:
>> On 31/07/14 18:53, Don Dutile wrote:
>>> Ideally, when the device is configured in different modes, the SRIOV
>>> cap structure
>>> should be modified so the pci sriov code doesn't try to act or reflect
>>> the
>>> non-reality the device is in.
>> That would be much nicer of course, and I'll ask our firmware team if
>> that's possible.  But I don't think it will be.
> As I feared, it's not practical for the device to do this.
> (Unfortunately, the IMEM (instruction memory) on the device is rather
> small and the firmware team are struggling to fit all the required
> features into it.)
> However, I also found out that in principle, the device _is_ capable of
> supporting VFs in PF-IOV mode, by attaching them directly to the
> underlying v-switch.  The firmware may or may not already do this; the
> person who's likely to know this isn't in today.  The driver at present
> doesn't support it because it assumes it always needs to attach VFs to
> its own v-switch, but it could (again, in principle) be taught otherwise.
> 
> So at this point it becomes a policy question of whether we want to
> support this, and our opinion is that this isn't a valid use-case (as
> the only time you'd want PF-IOV is if your system doesn't support VFs)
> and thus shouldn't be supported (as it creates extra testing and
> maintenance work).  The driver should probe the device, detect PF-IOV
> mode, recognise that it (the driver) doesn't have support for VFs in
> that mode, and set totalvfs to 0.
> Others may demur, of course, but that's likely to be decided when
> sending the driver patches to davem.  It's my belief that, uncertainty
> about my use case notwithstanding, pci_sriov_set_totalvfs(dev, 0) should
> be a valid operation with the semantics I've implemented in my patch.
> 
> -Edward

How is it you get yourself into the PF-IOV mode?  Is this something that
is stored in the NVM of the adapter, or is it something where the driver
has to issue a request for it?  Also does each PF advertise SR-IOV
support in PF-IOV mode or only function 0?

Modifying the total VFs and/or the initial VFs in the configuration
space is not practical.  You would likely run into issues as one of
these fields is read at probe time and the other is read at SR-IOV
enablement and if the two differ you cannot enable SR-IOV.

I'm not sure what davem has to do with any of this.  We are discussing
PCI at this point, not the network drivers.  As such Dave won't be the
one accepting the pci_sriov_set_totalvfs changes you proposed.  If you
are just wanting to get a patch through Dave for this you would be much
better off just returning an error on sriov_configure if the vswitch
isn't available rather than trying to change the SR-IOV API to match
your use case.

Thanks,

Alex



  reply	other threads:[~2014-08-04 14:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <53D9288B.5030302@solarflare.com>
2014-07-30 18:05 ` pci_sriov_set_totalvfs again Don Dutile
2014-07-30 18:24   ` Edward Cree
2014-07-30 21:14     ` Alexander Duyck
2014-07-31 12:07       ` Edward Cree
2014-07-31 14:24         ` [PATCH] PCI: handle pci_sriov_set_totalvfs(dev, 0) Edward Cree
2014-07-31 15:21           ` Alexander Duyck
2014-07-31 15:56             ` Edward Cree
2014-07-31 16:40               ` Alexander Duyck
2014-07-31 16:57                 ` Edward Cree
2014-07-31 17:53                   ` Don Dutile
2014-07-31 18:13                     ` Edward Cree
2014-08-04 14:03                       ` Edward Cree
2014-08-04 14:37                         ` Alexander Duyck [this message]
2014-08-04 15:22                           ` Edward Cree
2014-08-06  9:38                           ` Don Dutile
2014-07-31 17:55                   ` Alexander Duyck
2014-07-31 18:24                     ` Edward Cree
2014-08-01  3:18               ` Ethan Zhao
2014-08-01 11:51                 ` Edward Cree
2014-08-02  0:34                   ` Ethan Zhao
2014-08-01  3:51           ` Ethan Zhao
2014-08-01 12:15             ` Edward Cree
2014-08-02  0:25               ` Ethan Zhao
2014-08-04 15:45                 ` Edward Cree
2014-08-04 16:40                   ` Alexander Duyck
2014-08-04 17:08                     ` Edward Cree
2014-08-04  6:53               ` Sathya Perla

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=53DF9A9F.9060600@intel.com \
    --to=alexander.h.duyck@intel.com \
    --cc=bhelgaas@google.com \
    --cc=ddutile@redhat.com \
    --cc=ecree@solarflare.com \
    --cc=linux-pci@vger.kernel.org \
    /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).