linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
	Alexander Duyck <aduyck@mirantis.com>
Cc: bhelgaas@google.com, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] iov: Update virtfn_max_buses to validate offset and stride
Date: Wed, 28 Oct 2015 10:57:12 -0700	[thread overview]
Message-ID: <56310C78.6030703@gmail.com> (raw)
In-Reply-To: <20151028163216.GA2927@localhost>

On 10/28/2015 09:32 AM, Bjorn Helgaas wrote:
> Hi Alex,
>
> Thanks a lot for cleaning this up.  I think this is a great
> improvement over what I did.
>
> On Tue, Oct 27, 2015 at 01:52:15PM -0700, Alexander Duyck wrote:
>> This patch pulls the validation of offset and stride into virtfn_max_buses.
>> The general idea is to validate offset and stride for each possible value
>> of numvfs in addition to still determining the maximum bus value for the
>> VFs.
>>
>> I also reversed the loop as the most likely maximum will be when numvfs is
>> set to total_VFs.  In addition this makes it so that we loop down to a
>> value of 0 for numvfs which should be the resting state for the register.
>>
>> Fixes: 8e20e89658f2 ("PCI: Set SR-IOV NumVFs to zero after enumeration")
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>
> I'd like to squash this together with my patch instead of having fixes
> on top of fixes.  What do you think of the following?  (This applies
> on top of 70675e0b6a1a ("PCI: Don't try to restore VF BARs")).
>
>
> commit c20e11b572c5d4e4f01c86580a133122fbd13cfa
> Author: Alexander Duyck <aduyck@mirantis.com>
> Date:   Wed Oct 28 10:54:32 2015 -0500
>
>      PCI: Set SR-IOV NumVFs to zero after enumeration
>
>      The enumeration path should leave NumVFs set to zero.  But after
>      4449f079722c ("PCI: Calculate maximum number of buses required for VFs"),
>      we call virtfn_max_buses() in the enumeration path, which changes NumVFs.
>      This NumVFs change is visible via lspci and sysfs until a driver enables
>      SR-IOV.
>
>      Iterate from TotalVFs down to zero so NumVFs is zero when we're finished
>      computing the maximum number of buses.  Validate offset and stride in
>      the loop, so we can test it at every possible NumVFs setting.  Rename
>      virtfn_max_buses() to compute_max_vf_buses() to hint that it does have a
>      side effect of updating iov->max_VF_buses.
>
>      [bhelgaas: changelog, rename, reverse sense of error path]
>      Fixes: 4449f079722c ("PCI: Calculate maximum number of buses required for VFs")
>      Based-on-patch-by: Ethan Zhao <ethan.zhao@oracle.com>
>      Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>      Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>

This looks fine to me.

Acked-by: Alexander Duyck <aduyck@mirantis.com>


  reply	other threads:[~2015-10-28 17:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-27 20:52 [PATCH 0/5] Various of SR-IOV fixes and cleanup Alexander Duyck
2015-10-27 20:52 ` [PATCH 1/5] iov: Update virtfn_max_buses to validate offset and stride Alexander Duyck
2015-10-28 16:32   ` Bjorn Helgaas
2015-10-28 17:57     ` Alexander Duyck [this message]
2015-10-28 18:43     ` Bjorn Helgaas
2015-10-28 21:46       ` Alexander Duyck
2015-10-29 19:50         ` Bjorn Helgaas
2015-10-27 20:52 ` [PATCH 2/5] iov: Reset resources to 0 if totalVFs increases after enabling ARI Alexander Duyck
2015-10-28 16:37   ` Bjorn Helgaas
2015-10-28 18:32     ` Alexander Duyck
2015-10-28 19:52       ` Bjorn Helgaas
2015-10-28 21:37         ` Alexander Duyck
2015-10-27 20:52 ` [PATCH 3/5] iov: Fix sriov_enable exception handling path Alexander Duyck
2015-10-29 16:32   ` Bjorn Helgaas
2015-10-29 16:54     ` Alex Duyck
2015-10-29 20:41       ` Bjorn Helgaas
2015-10-27 20:52 ` [PATCH 4/5] iov: Variable and loop cleanup for sriov_disable and sriov_enable Alexander Duyck
2015-10-29 21:43   ` Bjorn Helgaas
2015-10-29 23:19     ` Alexander Duyck
2015-10-27 20:52 ` [PATCH 5/5] iov: Update sriov_enable to correctly handle offset and stride Alexander Duyck

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=56310C78.6030703@gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=aduyck@mirantis.com \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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).